diff --git a/.gitignore b/.gitignore index 84893ba..93caf15 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ /.idea/ /infection.log /.superpowers/ +.phptramp.cache/ diff --git a/README.md b/README.md index 0ea630b..eee454c 100644 --- a/README.md +++ b/README.md @@ -4,15 +4,16 @@ > methods that never use them — "this parameter was passed through 4 classes / 5 methods > before being used." -**Status: Phase 5.** Cross-file chain reporting works: `phptramp --folder src` -stitches forwarding chains across files and prints findings in any of six -formats, exiting `0`/`1`/`2`. A `phptramp.json` config file, `#[TrampIgnore]`/ -`// phptramp-ignore` suppression, and a `--warn-limit` warning tier are all wired -up. Diff-aware mode (`--changed-only`/`--git-base`/`--diff`) and baselining +**Status: v0.1.0** — the first release. phptramp detects tramp data across +file boundaries: `phptramp --folder src` stitches forwarding chains across files +and prints findings in any of six formats, exiting `0`/`1`/`2`. A +`phptramp.json` config file, `#[TrampIgnore]`/`// phptramp-ignore` suppression, +and a `--warn-limit` warning tier are all wired up. Diff-aware mode +(`--changed-only`/`--git-base`/`--diff`) and baselining (`--generate-baseline`/`--baseline`/`--fail-on-stale`) are shipped too — see -below. The classifier and whole-project index remain inspectable via -`--dump-index`. See [docs/plan.md](docs/plan.md) for the full implementation plan -and current phase. +below. A per-file index cache (default-on, `--no-cache` to disable) makes warm +re-runs fast enough for per-save IDE use. The classifier and whole-project +index remain inspectable via `--dump-index`. ## What it does @@ -108,6 +109,7 @@ phptramp [options] --baseline Ignore findings recorded in the baseline file --generate-baseline Write current findings to a baseline file --fail-on-stale Exit 1 when stale baseline entries or stale suppressions are found + --no-cache Disable the per-file index cache (re-parse every file) ``` Exit codes: `0` no finding at error severity, `1` at least one finding at/over @@ -137,6 +139,42 @@ key, or a value of the wrong type, is a config error rather than a silently igno | `warnLimit` | integer | `--warn-limit` | | `format` | string | `--format` | | `baseline` | string | `--baseline` | +| `cache` | string | directory for the per-file index cache (default: `.phptramp.cache/`, resolved relative to the config file's directory) | + +## Index cache + +phptramp caches each source file's parsed index under `.phptramp.cache/` (a +per-file cache, default-on). The cache is what makes warm re-runs — including +per-save IDE invocations — cheap: the whole-project index (the part call +resolution needs) rebuilds from cache instead of re-parsing every file. + +- **Disable** with `--no-cache`, or **move the directory** with the `cache` + config key (see [Configuration](#configuration)). +- **Identity-validated and version-keyed.** Each entry stores the source + file's path, mtime, and size, plus the tool and payload-format versions. Any + mismatch — an edited file, a bumped version, a corrupt or stale entry — is + a silent cache miss followed by a fresh parse. A stale or corrupt cache + **never changes findings**. +- **Safe to wipe.** Delete `.phptramp.cache/` any time; the next run simply + re-parses and repopulates it. Add `/.phptramp.cache/` to your own + `.gitignore` so the cache is not committed. + +### Performance + +Recorded 2026-08-11 on the maintainer's machine (Apple Silicon, PHP 8.4.23): + +| Run | Wall-clock | Notes | +|---|---|---| +| `--folder vendor` cold | ≈ 19.6s | 4,610 files / 601,867 lines parsed | +| `--folder vendor` warm | ≈ 4.5s | cache hot — indexing ≈ 0; the residual is chain resolution + reporting | +| `--folder src` cold (36 files) | ≈ 0.24s | small project baseline | +| Extrapolation | ≈ 1.6s cold per 50k LOC | linear in parsed lines | + +The cache eliminates the parsing portion of a run (≈ 15s saved on the +`--folder vendor` measurement); chain resolution and reporting are unchanged. +On a typical `src/` the warm per-save run is effectively instant after the +first cold run. See [docs/phpstorm.md](docs/phpstorm.md) for the IDE use this +enables. ## Output formats @@ -220,9 +258,9 @@ schedule or nightly to surface stale entries and prune the file. See ## IDE integration PhpStorm (and any IDE with external-tool support) can run phptramp per file via -`--file`; a documented External Tool / File Watcher recipe ships in Phase 6. -`--format=checkstyle` and `--format=sarif` cover CI annotation ecosystems, -including GitHub Code Scanning. +`--file`; a documented External Tool + File Watcher recipe is in +[docs/phpstorm.md](docs/phpstorm.md). `--format=checkstyle` and `--format=sarif` +cover CI annotation ecosystems, including GitHub Code Scanning. ## Non-goals diff --git a/docs/ci.md b/docs/ci.md index 0bade24..43c572b 100644 --- a/docs/ci.md +++ b/docs/ci.md @@ -5,6 +5,8 @@ the chains your change touches, and marks exactly which hops are yours. Wire it into a PR pipeline with `--changed-only --git-base `; pipe an arbitrary diff with `--diff -`; or run it as a full-repo gate the way this project gates itself. All four patterns below are copy-paste starting points. +For the IDE side (per-save runs in PhpStorm and friends) see +[docs/phpstorm.md](phpstorm.md). ## GitHub Actions — PR job diff --git a/docs/phpstorm.md b/docs/phpstorm.md new file mode 100644 index 0000000..eadbb10 --- /dev/null +++ b/docs/phpstorm.md @@ -0,0 +1,119 @@ +# PhpStorm recipe + +phptramp runs per file from any IDE with external-tool support; PhpStorm is the +common case. Two variants are documented below — a manual **External Tool** for +"analyze this file now" and a **File Watcher** for "analyze on save" — plus +one-liners for keyboard-shortcut and before-commit use. The cache is what +makes per-save viable: the whole-project index (the part call resolution needs) +rebuilds from cache, so an on-save run pays only the per-file parse plus the +unchanged chain-resolution and reporting pipeline. See the measured numbers at +the bottom of this file. + +phptramp always indexes the whole configured project; `--file` is a +*reporting* filter, not an analysis shortcut (chains cross files). The +`paths`/`--folder` configured in `phptramp.json` (or passed on the CLI) supply +the project index, and `--file $FilePath$` restricts the reported findings to +chains touching that one file. + +## External Tool + +**Settings → Tools → External Tools → +** + +| Field | Value | +|---|---| +| Name | `phptramp` | +| Program | `$ProjectFileDir$/vendor/bin/phptramp` | +| Arguments | `--file $FilePath$` | +| Working directory | `$ProjectFileDir$` | +| Output filters | `$FILE_PATH$:$LINE$` | + +Run it via **Code → Run External Tool → phptramp** (or a shortcut — see below). +Findings print to the tool console; the `$FILE_PATH$:$LINE$` output filter +turns every `path:line` occurrence into a clickable link back into the editor. + +The working directory is `$ProjectFileDir$` so `phptramp.json` (or +`phptramp.dist.json`) and the `.phptramp.cache/` directory are picked up from the +project root. If your config lives elsewhere, set the working directory +accordingly — phptramp resolves config and the cache relative to the current +working directory. + +The first run on a project is cold: every file parses once and the cache is +written. Subsequent runs — External Tool, File Watcher, CLI, CI — read the +warm cache and skip parsing, so the per-save invocation cost drops to the +per-file parse of the one edited file plus chain resolution. On a typical +`src/` the warm per-save run is effectively instant; see the numbers below. + +## File Watcher (on-save) + +**Settings → Tools → File Watchers → +** (the built-in "phptramp" template is +not required — add a custom watcher with these fields): + +| Field | Value | +|---|---| +| Name | `phptramp` | +| Scope | `Project PHP Files` (or a custom scope) | +| Program | `$ProjectFileDir$/vendor/bin/phptramp` | +| Arguments | `--file $FilePath$` | +| Working directory | `$ProjectFileDir$` | +| Output filters | `$FILE_PATH$:$LINE$` | + +Set **Scope** to **Project PHP Files** (**Settings → Scopes → Project PHP +Files**, or a custom scope) so the watcher fires only on the project's PHP +sources. Program and arguments are identical to the External Tool. + +Leave **"Auto-save edited files to trigger the watcher"** **OFF**. With it on, +the watcher fires phptramp, phptramp may report and the editor re-saves, which +re-triggers the watcher — a feedback loop. OFF means the watcher runs once per +explicit save. + +## Keyboard shortcut / before-commit + +Bind the External Tool to a keymap entry (**Settings → Keymap → External Tools +→ phptramp**) for an "analyze this file" shortcut. For a pre-commit pass, run +it against staged files from a terminal: + +```bash +git diff --cached --name-only --diff-filter=d -- '*.php' \ + | xargs -r -I{} vendor/bin/phptramp --file {} +``` + +Or, if you keep it simple, a single `--folder src` run in a pre-commit hook is +the same full-scan gate this repo uses on itself — see [docs/ci.md](ci.md) for +the GitHub Actions and GitLab equivalents. + +## Qodana / Code Scanning (checkstyle & SARIF) + +phptramp's `--format checkstyle` and `--format sarif` renderers feed the +standard CI annotation ecosystems: + +- **`--format checkstyle`** — for Qodana/JetBrains and any tool that consumes + Checkstyle XML. Pipe the output to a file the IDE's inspection viewer reads, + or surface it in the PhpStorm **Problems** tool window via a Qodana + configuration that invokes phptramp. +- **`--format sarif`** — for GitHub Code Scanning and any SARIF consumer. + Upload the SARIF file with the `github/codeql-action/upload-sarif` action; + see [docs/ci.md](ci.md) for the annotation-recipe patterns (`--format github` + is the lighter-weight in-PR alternative). + +## Measured performance + +Recorded 2026-08-11 on the maintainer's machine (Apple Silicon, PHP 8.4.23): + +| Run | Wall-clock | Notes | +|---|---|---| +| `--folder vendor` cold | ≈ 19.6s | 4,610 files / 601,867 lines parsed | +| `--folder vendor` warm | ≈ 4.5s | cache hot — indexing ≈ 0 (all cache hits); the residual is chain resolution + reporting | +| `--folder src` cold (36 files) | ≈ 0.24s | small project baseline | +| Extrapolation | ≈ 1.6s cold per 50k LOC | linear in parsed lines | + +The cache eliminates the parsing portion of a run. On `--folder vendor` that +is ≈ 15s saved (≈ 19.6s cold → ≈ 4.5s warm). Chain resolution and reporting +are unchanged by the cache — they run on the (now cache-built) index the same +way cold or warm. + +What this means for IDE use: a `--file` invocation still builds the whole +project index (call resolution needs it), but warm it does so from cache. The +per-save cost is therefore the per-file parse of the one edited file plus the +unchanged resolution/reporting pipeline — on a typical `src/` that is +effectively instant after the first cold run. The cache is what makes per-save +viable; it does **not** speed up the resolution/reporting half of a full scan. diff --git a/docs/plan.md b/docs/plan.md index fcab1d5..4ff95e4 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -770,19 +770,33 @@ the detailed plan wins. --- -## Phase 6 — Performance & release - -- **6.1 Index cache:** per-file serialized `MethodInfo[]` keyed by (path, mtime, size, - tool version) under `.phptramp.cache/`; `--no-cache` flag. Target: warm re-run on a - 50k-LOC codebase < 1s (this is what makes IDE per-save invocation viable). -- **6.2 Parallel indexing:** worker pool via `proc_open` of self with a hidden - `--worker` mode over file shards (no pcntl dependency — works on Windows CI). Decide - worker count from `nproc`, flag `--jobs`. -- **6.3 Docs:** PhpStorm External Tool + File Watcher recipe (`docs/phpstorm.md`), - finalize `docs/ci.md`, README rewrite against the real tool output. +## Phase 6 — Performance & release (Tasks 1–5 done on this branch; release follows merge — issue #11) + +**Detailed step-by-step plan: [docs/plans/phase-6.md](plans/phase-6.md)** — written +against the shipped Phase 1–5 interfaces; where it refines the sketches below, the +detailed plan wins. Tasks 1–5 (index cache, `--no-cache` + `cache` config key, +PhpStorm recipe, docs release pass, this roadmap close-out) land on +`feature/11-cache-and-release`; the v0.1.0 release (Task 6) follows the merge via the +git-flow release process described in `CLAUDE.md` — not on this branch. + +- [x] **6.1 Index cache:** per-file serialized `MethodInfo[]` keyed by (path, mtime, + size, tool version) under `.phptramp.cache/`; `--no-cache` flag and `cache` config key. + Measured on `--folder vendor` (4,610 files / 601,867 lines): cold ≈ 19.6s → warm total + ≈ 4.5s with indexing ≈ 0 (all cache hits; the residual is chain-resolution + reporting). + Target was warm re-run on a 50k-LOC codebase < 1s — met, and IDE per-save invocation is + viable on the warm path. +- **6.2 Parallel indexing:** deferred — **the data said no, not yet.** Measured cold + indexing is ≈ 19.6s for the 600k-LOC vendor tree (≈ 1.6s per 50k LOC), and warm + indexing with the 6.1 cache is ≈ 0 (all hits), so the per-file cache alone meets the + stated warm-run target; the `proc_open`-of-self worker pool (no pcntl dependency, + Windows-CI-friendly) is moved to the post-v0.1 roadmap with the measurement recorded. +- [x] **6.3 Docs:** PhpStorm External Tool + File Watcher recipe shipped at + `docs/phpstorm.md`; `docs/ci.md` finalized and README release pass done against the + real tool output. - **6.4 Release:** (dogfood gating shipped early, in Phase 4) Packagist submission, tag - `v0.1.0`. Roadmap notes (explicit maybe-laters): `--follow-all-implementations`, - field/property tramping, native PhpStorm plugin, composer-plugin command package. + `v0.1.0` — follows the merge of this branch into `develop` via the git-flow release + process (see `CLAUDE.md`); not performed on this branch. The explicit maybe-laters + (roadmap notes) move to the `## Post-v0.1 roadmap` section below. --- @@ -795,6 +809,31 @@ the detailed plan wins. --- +## Post-v0.1 roadmap + +Explicit maybe-laters recorded during Phases 0–6; each waits on a concrete consumer +ask unless the measurement already says "not yet". + +- **`--follow-all-implementations`:** follow all N implementations of an interface + instead of the single-implementation default — fan-out counts are already recorded + since Phase 2 (see the single-implementation follow-through bullet in the Appendix), + so this is a small flag, not a rewrite. Origin: Phase 2 frozen-semantics decision. +- **Field/property tramping:** constructor-stored values re-forwarded via properties + (the parameter is captured, not just passed). Today the classifier tracks parameter + forwarding, not property-mediated re-forwarding. Origin: Phase 0 "future work" note. +- **Native PhpStorm plugin:** inline squiggles in the editor. The External Tool / File + Watcher recipe in `docs/phpstorm.md` is the stopgap. Origin: Phase 6.3 docs stopgap. +- **Composer-plugin command package:** `composer tramp` without the per-project + script snippet. Origin: Option B from the Phase 0 design review. +- **Parallel indexing:** the deferred 6.2 worker pool (`proc_open`-of-self over file + shards, `--jobs`). The measurement — cold ≈ 19.6s for the 600k-LOC vendor tree, + warm indexing ≈ 0 with the 6.1 cache — shows the cache alone meets the target, so + this is "the data said no, not yet", not descoped. Origin: Phase 6.2 deferral. +- **`--fail-on-stale` config key:** CLI-only since Phase 5; add a `failOnStale` config + key on demand when a consumer asks for it. Origin: Phase 5.3 stale-detection design. + +--- + ## Appendix: design rationale (why the frozen decisions are what they are) Recorded so a future implementer can make *aligned* trade-offs when hitting a wall, diff --git a/docs/plans/phase-6.md b/docs/plans/phase-6.md new file mode 100644 index 0000000..b7c50e2 --- /dev/null +++ b/docs/plans/phase-6.md @@ -0,0 +1,371 @@ +# Phase 6 Implementation Plan — Index cache, IDE recipe, v0.1.0 release + +> **For agentic workers:** Self-contained; assumes no conversation context. Read +> `CLAUDE.md` (toolchain, Clean Code house style, git-flow workflow) and the +> "Frozen core semantics" + Phase 6 sections of `docs/plan.md` first — this plan +> expands them into steps and repeats what it relies on. Work task-by-task with +> checkboxes; strict TDD; one task per commit. If your environment provides +> plan-execution skills (superpowers' subagent-driven-development or +> executing-plans), use them; otherwise execute task-by-task in order. + +**Goal:** warm re-runs become fast enough for per-save IDE use (per-file index +cache), PhpStorm integration is documented, the docs get their release pass, the +post-v0.1 roadmap is written down — and v0.1.0 ships through the repo's +tag-triggered release workflow onto Packagist. + +**Architecture:** `Indexer` splits into a per-file unit (`FileIndexer` → +`FileIndex`) and a merging orchestrator — a pure refactor that makes each file's +index independently cacheable. `src/Cache/FileIndexCache` stores one serialized +`FileIndex` per source file, keyed by content identity (path, mtime, size) plus +tool/format version; any validation failure is a silent miss, never an error. +The cache is on by default at `.phptramp.cache/` with `--no-cache` and a `cache` +config key. No analysis semantics change anywhere in this phase. + +**Tech Stack:** unchanged — PHP ≥ 8.2, nikic/php-parser ^5 (only prod +dependency). Serialization via PHP `serialize()`/`unserialize()` with an +explicit `allowed_classes` whitelist. + +## Global Constraints + +- Everything in `CLAUDE.md` applies: `composer check` green before every commit, + PHPMD-clean for every touched `src/` file, Clean Code house style, strict TDD, + one task per commit. +- **GitHub issue for this phase: #11.** Branch `feature/11-cache-and-release` + off `develop` (already created, carries this plan). Commits `type(#11): …`. + PR into `develop` with `Closes #11`; diff-scoped Infection gate (minMsi 80) + applies. The release itself (Task 6) happens AFTER that PR merges, on + git-flow release branches, per `CLAUDE.md`'s workflow — not on this branch. +- **Cache is transparency-critical:** a stale or corrupt cache must never + change findings. Every failure path (unreadable file, bad payload, version + mismatch, class-whitelist rejection) is a miss followed by a fresh parse. + No exception may escape the cache layer. +- **Measured baseline, recorded 2026-08-11 on the maintainer's machine + (Apple Silicon, PHP 8.4.23):** cold `--folder vendor` = 4,610 files / + 601,867 lines in **≈ 19.6s**; cold `--folder src` (36 files) ≈ 0.24s. + Extrapolation: ≈ 1.6s cold per 50k LOC. This is why parallel indexing + (master-plan sketch 6.2) is **deferred to the roadmap, not built**: the + master plan's own rule is "performance last, when profiling has real data," + and the data says the cache alone meets the stated warm-run target while a + worker pool would add a `proc_open` protocol + serialization boundary for a + goal already met. Record this deferral in `docs/plan.md` (Task 5). + +## Existing interfaces this phase consumes (Phase 1–5, verbatim) + +- `Indexer::index(list $files): MethodIndex` — builds one + `IndexingVisitor` + traverser (NameResolver, ParentConnectingVisitor, + visitor), loops files via `indexFile()` collecting error strings, throws + `ParseException` with all errors joined, then assembles + `new MethodIndex($this->classifyPending($visitor->pending()), $visitor->classes(), $visitor->suppressions())`. +- `IndexingVisitor` accumulates across files (`setFile()` between traversals); + exposes `pending(): list`, `classes(): array`, + `suppressions(): SuppressionIndex`. +- `MethodIndex::__construct(array $methods, array $classes = [], SuppressionIndex $suppressions = new SuppressionIndex([], [], []))`. +- Value objects that end up inside a cached payload — all `final` with + `readonly` props, safely `serialize()`-able: + `MethodInfo{ fqmn, file, line, params, class }`, + `ParamInfo{ name, position, fate, forwards, byRef, variadic, type, storedOnly }`, + `ParamFate` (pure enum), `ForwardSite{ callee, argKey, line }`, + `CalleeRef{ kind, name, receiverHint }`, `CalleeKind` (string-backed enum), + `ClassInfo{ name, kind, parent, interfaces, traits }`, `ClassKind` (pure enum). +- `SuppressionIndex::__construct(list $methods, list $params, array> $lines)` + plus `keys()` and the static key builders (Phase 5). +- `Options` (readonly value object) + `ArgvParser` (VALUE_FLAGS / BOOL_FLAGS + consts, `applyValueFlag`/`applyBoolFlag` matches) + `ConfigLoader` with + strict unknown-key errors — the pattern for adding `--no-cache` / `cache`. +- `Application::VERSION = '0.1.0-dev'` at `src/Console/Application.php:34`. +- Fixture harness folder mode runs from the repo root with `--no-config`; + `ApplicationTest` uses temp dirs + injected `php://memory` streams. +- Release process (from `CLAUDE.md`, quoted because Task 6 follows it + literally): git-flow AVH, per-clone config via `git flow init -d` with tag + prefix `v`; pushing a `vX.Y.Z` tag on a `main` commit runs + `.github/workflows/release.yml`, which guards CI was green on that SHA, + publishes the GitHub Release with notes from merged PRs, writes + `CHANGELOG.md`, and carries the commit back onto `develop`. + +## File structure (this phase) + +``` +src/Index/FileIndex.php NEW one file's methods/classes/suppression parts (Task 1) +src/Index/FileIndexer.php NEW path -> FileIndex (single-file parse) (Task 1) +src/Index/Indexer.php MOD merge orchestrator; optional cache (Tasks 1, 3) +src/Index/IndexingVisitor.php MOD + suppressionParts() raw accessor (Task 1) +src/Cache/FileIndexCache.php NEW get/put with identity+version validation (Task 2) +src/Console/ArgvParser.php MOD --no-cache (Task 3) +src/Console/Options.php MOD + noCache, + cacheDir (Task 3) +src/Config/ConfigLoader.php MOD `cache` key (string dir) (Task 3) +src/Console/Application.php MOD cache wiring; later VERSION bumps (Tasks 3, 6) +.gitignore MOD /.phptramp.cache/ (Task 3) +docs/phpstorm.md NEW External Tool + File Watcher recipe (Task 4) +docs/ci.md, README.md MOD cross-links, cache section, release pass (Task 4) +docs/plan.md MOD roadmap + deferral notes + phase status (Task 5) +tests/Index/FileIndexerTest.php NEW (Task 1) +tests/Cache/FileIndexCacheTest.php NEW (Task 2) +tests/… extended (Tasks 1–3) +``` + +--- + +### Task 1: `FileIndexer` + `FileIndex` (pure refactor, no cache yet) + +Per-file caching needs per-file indexing units. Today one visitor accumulates +across all files; afterwards each file parses independently and `Indexer` +merges. **No behavior change** — the whole existing suite is the safety net, +and per-file classification is safe because `UsageClassifier` only ever reads +one method's own body. + +**Files:** +- Create: `src/Index/FileIndex.php`, `src/Index/FileIndexer.php` +- Modify: `src/Index/Indexer.php`, `src/Index/IndexingVisitor.php` +- Test: `tests/Index/FileIndexerTest.php`; existing `IndexerTest`, + `UsageClassifierTest`, fixture harness must pass untouched + +**Interfaces:** +- Consumes: `IndexingVisitor`, `UsageClassifier`, the value objects listed + above. +- Produces: + +```php +/** Everything the index learns from one source file. Serializable as a unit. */ +final class FileIndex +{ + /** + * @param array $methods keyed by FQMN + * @param array $classes keyed by FQCN + * @param list $suppressedMethods + * @param list $suppressedParams + * @param array> $suppressedLines + */ + public function __construct( + public readonly array $methods, + public readonly array $classes, + public readonly array $suppressedMethods, + public readonly array $suppressedParams, + public readonly array $suppressedLines, + ) {} +} + +final class FileIndexer +{ + /** Parses and classifies ONE file. @throws ParseException on read/parse failure (single file's message) */ + public function index(string $file): FileIndex; +} + +// IndexingVisitor addition — the raw parts suppressions() already aggregates: +/** @return array{methods: list, params: list, lines: array>} */ +public function suppressionParts(): array; +``` + +`Indexer::index()` becomes: for each file call `FileIndexer` (collecting +per-file `ParseException` messages, still throwing them joined at the end — +same aggregate message format), then merge in input order: later files win on +duplicate FQMN/FQCN keys (`$merged[$key] = $value` in file order — this is the +current last-wins behavior of the accumulating visitor, preserved), suppression +parts concatenated then built into one `SuppressionIndex`. Input order comes +from `FileLocator` (sorted), so `MethodIndex` insertion order — and therefore +chain-walk determinism — is unchanged. + +- [ ] **Step 1: Write the failing test** (`FileIndexerTest`, temp-file helper + pattern): one file with a namespaced class + method + `#[TrampIgnore]` param + + `// phptramp-ignore` comment → `FileIndex` carries the method (classified), + the class, and all three suppression part kinds; unreadable path throws + `ParseException` naming the file; syntax error throws naming the file. +- [ ] **Step 2:** `vendor/bin/phpunit --filter FileIndexerTest` → FAIL (class + not found). +- [ ] **Step 3:** implement `FileIndex`, `FileIndexer` (per-file visitor + + traverser instances), `suppressionParts()`; rewrite `Indexer::index()` as the + merge loop. +- [ ] **Step 4:** `vendor/bin/phpunit` — the WHOLE suite, not just the new + test; then `composer check`; then `composer tramp` (self-gate must be + byte-identical). +- [ ] **Step 5:** commit `refactor(#11): per-file indexing units behind the Indexer`. + +### Task 2: `FileIndexCache` + +**Files:** +- Create: `src/Cache/FileIndexCache.php` +- Test: `tests/Cache/FileIndexCacheTest.php` + +**Interfaces:** +- Consumes: `FileIndex` (Task 1), `Application::VERSION`. +- Produces: + +```php +final class FileIndexCache +{ + /** Bump when FileIndex or any nested value object changes shape. */ + private const FORMAT = 1; + + public function __construct(private readonly string $directory) {} + + /** Null on any miss: absent, unreadable, version/format/identity mismatch, corrupt payload. */ + public function get(string $file): ?FileIndex; + + /** Writes best-effort; a failed write (unwritable dir, full disk) is silently ignored. */ + public function put(string $file, FileIndex $index): void; +} +``` + +Mechanics, pinned: + +- Cache entry path: `/.cache`. +- Payload: `serialize(['format' => self::FORMAT, 'version' => Application::VERSION, 'path' => $realpath, 'mtime' => $mtime, 'size' => $size, 'index' => $fileIndex])`. +- `get()` validates format, version, path, and the file's *current* mtime+size + against the stored ones; `unserialize(..., ['allowed_classes' => [FileIndex::class, MethodInfo::class, ParamInfo::class, ParamFate::class, ForwardSite::class, CalleeRef::class, CalleeKind::class, ClassInfo::class, ClassKind::class]])`; + `false`/wrong-type results are misses. Directory is created lazily on first + `put()` (`mkdir` recursive, `@`-suppressed — creation failure just means no + caching). + +- [ ] **Step 1: Write the failing test:** put→get round-trips an equal + `FileIndex` (use `assertEquals`); get on a never-put file → null; touching + the source file (`touch($file, time() + 10)`) → null; content change + (size) → null; garbage bytes in the entry → null; entry written by a + different `FORMAT` (write the array by hand with `'format' => 0`) → null; + `put()` into a read-only directory does not throw. +- [ ] **Step 2:** red. **Step 3:** implement. **Step 4:** green + + `composer check`. +- [ ] **Step 5:** commit `feat(#11): per-file index cache`. + +### Task 3: Wiring — default-on cache, `--no-cache`, `cache` config key + +**Files:** +- Modify: `src/Console/ArgvParser.php`, `src/Console/Options.php`, + `src/Config/ConfigLoader.php`, `src/Index/Indexer.php`, + `src/Console/Application.php`, `.gitignore` +- Test: extend `ArgvParserTest`, `ConfigLoaderTest`, `ApplicationTest` + +**Interfaces:** +- Consumes: `FileIndexCache` (Task 2). +- Produces: `Options` gains `public readonly bool $noCache = false` and + `public readonly string $cacheDir = '.phptramp.cache'`; + `Indexer::__construct(private readonly ?FileIndexCache $cache = null)` — + every existing `new Indexer()` call site (tests included) keeps working, + uncached. + +Behavior, pinned: + +- `--no-cache` (BOOL_FLAGS) disables entirely; config key `cache` (string) + overrides the directory, resolved relative to the config file's directory + (= cwd), same as `exclude`. CLI-over-config precedence via the existing + defaults-seeding mechanic. +- `Application` constructs `new Indexer($options->noCache ? null : new FileIndexCache($options->cacheDir))` + in both `analyze()` and `dumpIndex()`. +- Cache hits skip `FileIndexer` per file; misses parse then `put()`. The + aggregate-`ParseException` contract is unchanged (a cached file cannot be a + parse error — it parsed when cached; identity checks force re-parse after + edits). +- Repo's own `.gitignore` gains `/.phptramp.cache/`; README tells consumers to + do the same. + +- [ ] **Step 1: Write the failing tests:** parser: `--no-cache` sets the flag. + ConfigLoader: `"cache": "build/tramp-cache"` lands in `cacheDir`; + `"cache": 5` throws. Application (temp dir, temp cache dir): run twice with + the same cache dir → second run's stdout byte-identical to the first and the + cache dir contains one `.cache` entry per source file; edit a source file + between runs → new finding appears (identity invalidation end-to-end); + `--no-cache` leaves the cache dir absent. +- [ ] **Step 2:** red. **Step 3:** implement. **Step 4:** full suite + + `composer check` + `composer tramp`; manual sanity: time + `bin/phptramp --folder vendor --limit 99 --format summary` cold vs warm and + record both numbers in the commit body (expect ≈ 19.6s cold → far under 1s + of indexing warm; report actual). +- [ ] **Step 5:** commit `feat(#11): default-on index cache with --no-cache and cache config key`. + +### Task 4: PhpStorm recipe + docs release pass + +**Files:** +- Create: `docs/phpstorm.md` +- Modify: `README.md`, `docs/ci.md` + +`docs/phpstorm.md` content (write it fully, not as an outline): + +- **External Tool** setup, field by field: Program + `$ProjectFileDir$/vendor/bin/phptramp`, Arguments `--file $FilePath$`, + Working directory `$ProjectFileDir$` (so `phptramp.json` and the cache are + found); Output Filters `$FILE_PATH$:$LINE$` so findings are clickable; note + that the whole-project index still builds (config `paths` supplies it) and + that the warm cache is what makes per-save invocation viable (cite the + measured numbers from the Global Constraints). +- **File Watcher** variant for on-save runs (scope: project PHP files; program + and arguments identical; "Auto-save edited files" off to avoid feedback + loops). +- **Keyboard shortcut / before-commit** one-liners; `--format checkstyle` + + SARIF pointers for Qodana/Code-Scanning users; link back to `docs/ci.md`. + +README release pass: status paragraph loses phase numbering ("v0.1.0"), cache +section added ("delete `.phptramp.cache/` any time; it is identity-validated +and version-keyed"), gitignore note, performance numbers, PhpStorm link. +`docs/ci.md`: link `docs/phpstorm.md`, verify every recipe still matches the +real flags (they changed in no task of this phase — this is a read-through, +fix only drift found). + +- [ ] Steps: write `docs/phpstorm.md` → README + ci.md edits → `composer check` + (docs-only, still run it) → commit `docs(#11): phpstorm recipe and docs release pass`. + +### Task 5: Roadmap + master-plan close-out + +**Files:** +- Modify: `docs/plan.md` + +- Mark Phase 6 tasks/sections done as they land (this task can trail Task 6's + release by a commit — see ordering note there). Add to the Phase 6 section: + parallel indexing (sketch 6.2) **deferred with the measurement** from Global + Constraints — the honest sentence is "the data said no, not yet", not + "descoped". +- New final section `## Post-v0.1 roadmap` listing, each with one sentence of + rationale and its origin: `--follow-all-implementations` (fan-out counts + already recorded since Phase 2), field/property tramping (constructor-stored + values re-forwarded via properties — the Phase 0 "future work" note), native + PhpStorm plugin (inline squiggles; External Tool recipe is the stopgap), + composer-plugin command package (`composer tramp` without the script + snippet — Option B from the Phase 0 design review), parallel indexing (the + deferral above, with numbers), `--fail-on-stale` config key (CLI-only since + Phase 5, add on demand). + +- [ ] Steps: edit → commit `docs(#11): post-v0.1 roadmap and phase 6 status`. + +### Task 6: Release v0.1.0 + +**Prerequisite:** the `feature/11-cache-and-release` PR (Tasks 1–5) is merged +into `develop` and CI is green there. The release follows `CLAUDE.md`'s +workflow **exactly**; commands below are that workflow spelled out. Any +deviation the repo's git-flow config forces (e.g. missing `git flow init`) is +fixed by `git flow init -d` (tag prefix `v`), never improvised around. + +- [ ] **Step 1:** preflight — on `develop`: `git pull --ff-only`, + `gh run list --limit 1` shows the merge commit green; `composer check` and + `composer tramp` green locally; `composer infection` (full sweep, not diff) + run once and surviving-mutant count noted in the release PR/notes if any. +- [ ] **Step 2:** `git flow release start 0.1.0`. +- [ ] **Step 3:** bump `Application::VERSION` to `'0.1.0'`; adjust the version + test expectation; `composer check`; commit + `chore: version 0.1.0` (release-branch commit, no issue number — the plain + form per `CLAUDE.md`). +- [ ] **Step 4:** `git flow release finish -m "v0.1.0" 0.1.0` (merges to + `main`, tags `v0.1.0`, back-merges `develop`). +- [ ] **Step 5:** on `develop`: bump `Application::VERSION` to `'0.2.0-dev'` + (+ test), `composer check`, commit `chore: open 0.2.0-dev`. +- [ ] **Step 6:** `git push origin develop main v0.1.0` — the tag on the + `main` commit triggers `release.yml`: it guards CI was green on that SHA, + publishes the GitHub Release with generated notes, writes `CHANGELOG.md`, + and carries the changelog commit back onto `develop`. Watch it: + `gh run watch` / `gh release view v0.1.0`. Then `git pull --ff-only` on + `develop` to pick up the carried-back changelog commit. +- [ ] **Step 7 — MAINTAINER ACTION (cannot be done by an agent):** submit + `https://github.com/larspohlmann/phptramp` on packagist.org (requires the + maintainer's Packagist login) and enable the GitHub hook for auto-updates. +- [ ] **Step 8:** verify from the outside: in a scratch directory, + `composer require --dev larspohlmann/phptramp:^0.1` succeeds and + `vendor/bin/phptramp --version` prints `phptramp 0.1.0`. + +--- + +## Done when + +1. Tasks 1–5 merged into `develop` via a PR with `Closes #11`; CI (matrix + + static + dogfood + mutation) green; `composer tramp` output identical before + and after the cache refactor. +2. Warm-run indexing measured and recorded; parallel-indexing deferral and the + roadmap are in `docs/plan.md`. +3. `v0.1.0` exists as a GitHub Release with generated notes; `CHANGELOG.md` + updated; `develop` carries the back-merge; the package installs from + Packagist (after the maintainer's submission) and prints `phptramp 0.1.0`. diff --git a/src/Cache/FileIndexCache.php b/src/Cache/FileIndexCache.php new file mode 100644 index 0000000..a3730fd --- /dev/null +++ b/src/Cache/FileIndexCache.php @@ -0,0 +1,154 @@ + */ + private const ALLOWED_CLASSES = [ + FileIndex::class, + SuppressionParts::class, + MethodInfo::class, + ParamInfo::class, + ParamFate::class, + ForwardSite::class, + CalleeRef::class, + CalleeKind::class, + ClassInfo::class, + ClassKind::class, + ]; + + public function __construct(private readonly string $directory) + { + } + + /** + * Returns the cached index for the file, or null on any miss — absent + * entry, unreadable file, version/format/identity mismatch, or corrupt + * payload. Never throws. + */ + public function get(string $file): ?FileIndex + { + $realpath = realpath($file); + if ($realpath === false) { + return null; + } + + $entryPath = $this->entryPath($realpath); + $payload = @file_get_contents($entryPath); + if ($payload === false) { + return null; + } + + $data = @unserialize($payload, ['allowed_classes' => self::ALLOWED_CLASSES]); + if (!is_array($data)) { + return null; + } + + $index = $data['index'] ?? null; + if (!$index instanceof FileIndex) { + return null; + } + + $stat = @stat($realpath); + if ($stat === false) { + return null; + } + + if (!$this->payloadIsCurrent($data, $realpath, $stat)) { + return null; + } + + return $index; + } + + /** + * A payload is current only when format, version, source path, and the + * file's mtime and size all match what was recorded at write time. + * + * @param array $data + * @phpstan-param array $stat + */ + private function payloadIsCurrent(array $data, string $realpath, array $stat): bool + { + return ($data['format'] ?? null) === self::FORMAT + && ($data['version'] ?? null) === Application::VERSION + && ($data['path'] ?? null) === $realpath + && ($data['mtime'] ?? null) === $stat['mtime'] + && ($data['size'] ?? null) === $stat['size']; + } + + /** + * Writes the index for the file, best-effort. A failed write (unwritable + * directory, full disk) is silently ignored — the next run will simply + * re-parse. Never throws. + */ + public function put(string $file, FileIndex $index): void + { + $realpath = realpath($file); + if ($realpath === false) { + return; + } + + $stat = @stat($realpath); + if ($stat === false) { + return; + } + + if (!is_dir($this->directory)) { + @mkdir($this->directory, self::DIRECTORY_MODE, true); + if (!is_dir($this->directory)) { + return; + } + } + + $payload = serialize([ + 'format' => self::FORMAT, + 'version' => Application::VERSION, + 'path' => $realpath, + 'mtime' => $stat['mtime'], + 'size' => $stat['size'], + 'index' => $index, + ]); + + @file_put_contents($this->entryPath($realpath), $payload); + } + + private function entryPath(string $realpath): string + { + return $this->directory . '/' . sha1($realpath) . '.cache'; + } +} diff --git a/src/Config/ConfigLoader.php b/src/Config/ConfigLoader.php index ab93896..165ee22 100644 --- a/src/Config/ConfigLoader.php +++ b/src/Config/ConfigLoader.php @@ -89,6 +89,7 @@ private function toOptions(array $config): Options $warnLimit = $defaults->warnLimit; $format = $defaults->format; $baseline = $defaults->baseline; + $cacheDir = $defaults->cacheDir; foreach ($config as $key => $value) { match ($key) { @@ -98,6 +99,7 @@ private function toOptions(array $config): Options 'warnLimit' => $warnLimit = $this->requireInt('warnLimit', $value), 'format' => $format = $this->requireString('format', $value), 'baseline' => $baseline = $this->requireString('baseline', $value), + 'cache' => $cacheDir = $this->requireString('cache', $value), default => throw new ConfigException("unknown config key: {$key}"), }; } @@ -110,6 +112,7 @@ private function toOptions(array $config): Options format: $format, baseline: $baseline, exclude: $exclude, + cacheDir: $cacheDir, ); } diff --git a/src/Console/Application.php b/src/Console/Application.php index 4eeb684..b03dd62 100644 --- a/src/Console/Application.php +++ b/src/Console/Application.php @@ -7,6 +7,7 @@ use PhpTramp\Baseline\Baseline; use PhpTramp\Baseline\BaselineException; use PhpTramp\Baseline\BaselineFilter; +use PhpTramp\Cache\FileIndexCache; use PhpTramp\Chain\ChainBuilder; use PhpTramp\Chain\Finding; use PhpTramp\Config\ConfigException; @@ -300,8 +301,9 @@ private function hasError(array $findings, Thresholds $thresholds): bool private function buildIndex(Options $options): MethodIndex { $files = (new FileLocator($this->workingDirectory()))->locate($options); + $cache = $options->noCache ? null : new FileIndexCache($options->cacheDir); - return (new Indexer())->index($files); + return (new Indexer(cache: $cache))->index($files); } /** @@ -317,8 +319,7 @@ private function findChains(MethodIndex $index): array private function dumpIndex(Options $options): int { try { - $files = (new FileLocator($this->workingDirectory()))->locate($options); - $index = (new Indexer())->index($files); + $index = $this->buildIndex($options); } catch (InvalidArgsException | ParseException $e) { fwrite($this->stderr, 'phptramp: ' . $e->getMessage() . "\n"); @@ -394,6 +395,9 @@ private static function helpText(): string --generate-baseline Write current findings to a baseline file --fail-on-stale Exit 1 when stale baseline entries or stale suppressions are found + Cache: + --no-cache Disable the per-file index cache (default: on at .phptramp.cache/) + Debugging: --dump-index Print the classified method index as JSON and exit @@ -406,16 +410,12 @@ private static function helpText(): string 1 at least one finding at or over --limit 2 tool error (bad arguments, parse failure, ...) - Status: Phase 5 - baseline is shipped: --generate-baseline / - --baseline ignore refactor-stable findings recorded in the file, - and --fail-on-stale exits 1 when a baseline entry or suppression - matches nothing (full runs only — stale detection is skipped - under --changed-only). Phase 4 - diff-aware mode - (--changed-only / --git-base / --diff) reports only chains - touching the diff and marks which hops are yours, across all six - formats (text/json/github/checkstyle/sarif/summary). - phptramp.json config, #[TrampIgnore]/phptramp-ignore suppression, - and --warn-limit are wired up. See docs/plan.md. + Status: v0.1.0 — cross-file chain reporting (six formats), diff-aware + mode (--changed-only / --git-base / --diff), baselining + (--generate-baseline / --baseline / --fail-on-stale), + #[TrampIgnore] / // phptramp-ignore suppression, the phptramp.json + config, --warn-limit, and a per-file index cache (--no-cache / the + `cache` config key) are all shipped. See docs/plan.md. TXT; } diff --git a/src/Console/ArgvParser.php b/src/Console/ArgvParser.php index 7f964f6..fe6154b 100644 --- a/src/Console/ArgvParser.php +++ b/src/Console/ArgvParser.php @@ -41,6 +41,7 @@ final class ArgvParser '--version', '-V', '--fail-on-stale', + '--no-cache', ]; private const VALID_FORMATS = ['text', 'json', 'github', 'checkstyle', 'sarif', 'summary']; @@ -59,6 +60,7 @@ final class ArgvParser private bool $clearedSeededPaths = false; /** @var list */ private array $exclude = []; + private string $cacheDir = '.phptramp.cache'; /** * @param list $args @@ -91,6 +93,8 @@ public function parse(array $args, Options $defaults = new Options()): Options version: $this->bools->version, failOnStale: $this->bools->failOnStale, exclude: $this->exclude, + noCache: $this->bools->noCache, + cacheDir: $this->cacheDir, ); } @@ -109,10 +113,12 @@ private function reset(Options $defaults): void help: $defaults->help, version: $defaults->version, failOnStale: $defaults->failOnStale, + noCache: $defaults->noCache, ); $this->baseline = $defaults->baseline; $this->generateBaseline = $defaults->generateBaseline; $this->exclude = $defaults->exclude; + $this->cacheDir = $defaults->cacheDir; $this->clearedSeededPaths = false; } @@ -176,6 +182,7 @@ private function applyBoolFlag(string $name): void '--help', '-h' => $this->bools->help = true, '--version', '-V' => $this->bools->version = true, '--fail-on-stale' => $this->bools->failOnStale = true, + '--no-cache' => $this->bools->noCache = true, default => throw new InvalidArgsException("unknown option: {$name}"), }; } diff --git a/src/Console/BoolFlags.php b/src/Console/BoolFlags.php index cc4bb6d..0b63869 100644 --- a/src/Console/BoolFlags.php +++ b/src/Console/BoolFlags.php @@ -19,6 +19,7 @@ public function __construct( public bool $help = false, public bool $version = false, public bool $failOnStale = false, + public bool $noCache = false, ) { } } diff --git a/src/Console/Options.php b/src/Console/Options.php index ca172fa..8032516 100644 --- a/src/Console/Options.php +++ b/src/Console/Options.php @@ -37,6 +37,8 @@ public function __construct( public readonly bool $version = false, public readonly bool $failOnStale = false, public readonly array $exclude = [], + public readonly bool $noCache = false, + public readonly string $cacheDir = '.phptramp.cache', ) { } } diff --git a/src/Ignore/SuppressionIndex.php b/src/Ignore/SuppressionIndex.php index 20207a5..2a4680e 100644 --- a/src/Ignore/SuppressionIndex.php +++ b/src/Ignore/SuppressionIndex.php @@ -8,8 +8,9 @@ * O(1) membership lookups for every suppression collected during indexing: * methods (and classes, already expanded to their member FQMNs) suppressed by * a `TrampIgnore` attribute, individual params suppressed by the same, and - * source lines carrying a `// phptramp-ignore` comment. Immutable; produced - * by {@see \PhpTramp\Index\IndexingVisitor::suppressions()}. + * source lines carrying a `// phptramp-ignore` comment. Immutable; produced by + * {@see \PhpTramp\Index\Indexer}, which merges per-file suppression parts + * into one index. * * The static `*Key()` builders and {@see keys()} expose every configured entry * as a stable string key, so the suppression filter can record which entries diff --git a/src/Index/FileIndex.php b/src/Index/FileIndex.php new file mode 100644 index 0000000..d8235ba --- /dev/null +++ b/src/Index/FileIndex.php @@ -0,0 +1,24 @@ + $methods keyed by FQMN + * @param array $classes keyed by FQCN + */ + public function __construct( + public readonly array $methods, + public readonly array $classes, + public readonly SuppressionParts $suppression, + ) { + } +} diff --git a/src/Index/FileIndexer.php b/src/Index/FileIndexer.php new file mode 100644 index 0000000..4221d53 --- /dev/null +++ b/src/Index/FileIndexer.php @@ -0,0 +1,91 @@ +parser = (new ParserFactory())->createForNewestSupportedVersion(); + } + + /** + * @throws ParseException if the file cannot be read or parsed (single file's message) + */ + public function index(string $file): FileIndex + { + $code = @file_get_contents($file); + if ($code === false) { + throw new ParseException("{$file}: could not read file"); + } + + try { + $ast = $this->parser->parse($code); + } catch (Error $e) { + throw new ParseException("{$file}: {$e->getMessage()}"); + } + + if ($ast === null) { + throw new ParseException("{$file}: could not parse file"); + } + + $visitor = new IndexingVisitor(); + $visitor->setFile($file); + $visitor->recordIgnoreComments($code); + + $traverser = new NodeTraverser(); + $traverser->addVisitor(new NameResolver()); + $traverser->addVisitor(new ParentConnectingVisitor()); + $traverser->addVisitor($visitor); + $traverser->traverse($ast); + + return new FileIndex( + $this->classifyPending($visitor->pending()), + $visitor->classes(), + $visitor->suppressionParts(), + ); + } + + /** + * @param list $pending + * + * @return array + */ + private function classifyPending(array $pending): array + { + $classifier = new UsageClassifier(); + $methods = []; + foreach ($pending as $entry) { + $node = $entry['node']; + $params = $classifier->classify($node->getParams(), $node->getStmts()); + $methods[$entry['fqmn']] = new MethodInfo( + $entry['fqmn'], + $entry['file'], + $entry['line'], + $params, + $entry['class'], + ); + } + + return $methods; + } +} diff --git a/src/Index/Indexer.php b/src/Index/Indexer.php index d11a9b4..8d50f5a 100644 --- a/src/Index/Indexer.php +++ b/src/Index/Indexer.php @@ -4,42 +4,61 @@ namespace PhpTramp\Index; -use PhpParser\Error; -use PhpParser\NodeTraverser; -use PhpParser\NodeVisitor\NameResolver; -use PhpParser\NodeVisitor\ParentConnectingVisitor; -use PhpParser\Parser; -use PhpParser\ParserFactory; +use PhpTramp\Cache\FileIndexCache; /** - * Parses the located files into a whole-project {@see MethodIndex}. Parameter - * usage classification is delegated to {@see UsageClassifier}. + * Orchestrates per-file indexing: each located file is parsed and classified + * by {@see FileIndexer} in isolation, then the per-file results are merged in + * input order into one whole-project {@see MethodIndex}. The merge preserves + * the prior single-visitor accumulation semantics — later files win on + * duplicate FQMN/FQCN keys, suppression parts are concatenated — so the + * resulting index is byte-identical to the pre-refactor output. + * + * When a {@see FileIndexCache} is injected, each file is served from the cache + * on a hit (skipping the parse) and written back on a miss. Cache transparency + * is total: the cache never changes the merged result, never throws, and a + * cached entry is invalidated by mtime/size so an edited file always re-parses. * * @phpstan-import-type PendingMethod from IndexingVisitor */ final class Indexer { + public function __construct( + private readonly FileIndexer $fileIndexer = new FileIndexer(), + private readonly ?FileIndexCache $cache = null, + ) { + } + /** * @param list $files * - * @throws ParseException if any file cannot be read or parsed + * @throws ParseException if any file cannot be read or parsed (per-file messages joined) */ public function index(array $files): MethodIndex { - $parser = (new ParserFactory())->createForNewestSupportedVersion(); - - $visitor = new IndexingVisitor(); - $traverser = new NodeTraverser(); - $traverser->addVisitor(new NameResolver()); - $traverser->addVisitor(new ParentConnectingVisitor()); - $traverser->addVisitor($visitor); - + /** @var array $mergedMethods */ + $mergedMethods = []; + /** @var array $mergedClasses */ + $mergedClasses = []; + /** @var list $suppressions */ + $suppressions = []; $errors = []; + foreach ($files as $file) { - $error = $this->indexFile($parser, $traverser, $visitor, $file); - if ($error !== null) { - $errors[] = $error; + try { + $fileIndex = $this->indexFile($file); + } catch (ParseException $e) { + $errors[] = $e->getMessage(); + continue; + } + + foreach ($fileIndex->methods as $fqmn => $method) { + $mergedMethods[$fqmn] = $method; } + foreach ($fileIndex->classes as $fqcn => $class) { + $mergedClasses[$fqcn] = $class; + } + $suppressions[] = $fileIndex->suppression; } if ($errors !== []) { @@ -47,61 +66,31 @@ public function index(array $files): MethodIndex } return new MethodIndex( - $this->classifyPending($visitor->pending()), - $visitor->classes(), - $visitor->suppressions(), + $mergedMethods, + $mergedClasses, + SuppressionParts::merge(...$suppressions)->toSuppressionIndex(), ); } /** - * @param list $pending - * - * @return array + * Returns the cached {@see FileIndex} on a hit, otherwise parses fresh + * and stores the result for next time. A null cache means uncached, which + * is the default so every existing `new Indexer()` call site is unaffected. */ - private function classifyPending(array $pending): array + private function indexFile(string $file): FileIndex { - $classifier = new UsageClassifier(); - $methods = []; - foreach ($pending as $entry) { - $node = $entry['node']; - $params = $classifier->classify($node->getParams(), $node->getStmts()); - $methods[$entry['fqmn']] = new MethodInfo( - $entry['fqmn'], - $entry['file'], - $entry['line'], - $params, - $entry['class'], - ); - } - - return $methods; - } - - private function indexFile( - Parser $parser, - NodeTraverser $traverser, - IndexingVisitor $visitor, - string $file - ): ?string { - $code = @file_get_contents($file); - if ($code === false) { - return "{$file}: could not read file"; - } - - try { - $ast = $parser->parse($code); - } catch (Error $e) { - return "{$file}: {$e->getMessage()}"; + if ($this->cache === null) { + return $this->fileIndexer->index($file); } - if ($ast === null) { - return "{$file}: could not parse file"; + $cached = $this->cache->get($file); + if ($cached !== null) { + return $cached; } - $visitor->setFile($file); - $visitor->recordIgnoreComments($code); - $traverser->traverse($ast); + $fileIndex = $this->fileIndexer->index($file); + $this->cache->put($file, $fileIndex); - return null; + return $fileIndex; } } diff --git a/src/Index/IndexingVisitor.php b/src/Index/IndexingVisitor.php index 4dcb866..dc562e3 100644 --- a/src/Index/IndexingVisitor.php +++ b/src/Index/IndexingVisitor.php @@ -16,7 +16,6 @@ use PhpParser\Node\Stmt\Trait_; use PhpParser\Node\Stmt\TraitUse; use PhpParser\NodeVisitorAbstract; -use PhpTramp\Ignore\SuppressionIndex; /** * Accumulates class-hierarchy facts and a list of pending function/method @@ -78,9 +77,15 @@ public function recordIgnoreComments(string $code): void $this->suppressionCollector->recordIgnoreComments($this->file, $code); } - public function suppressions(): SuppressionIndex + /** + * The raw suppression parts accumulated for the files seen so far, before + * they are aggregated into a {@see \PhpTramp\Ignore\SuppressionIndex}. The + * Indexer merges these across files and builds one index from the + * concatenation. + */ + public function suppressionParts(): SuppressionParts { - return $this->suppressionCollector->suppressions($this->pending); + return $this->suppressionCollector->parts($this->pending); } /** diff --git a/src/Index/SuppressionCollector.php b/src/Index/SuppressionCollector.php index 6e38872..cd08fcb 100644 --- a/src/Index/SuppressionCollector.php +++ b/src/Index/SuppressionCollector.php @@ -8,14 +8,15 @@ use PhpParser\Node\Expr\Variable; use PhpParser\Node\FunctionLike; use PhpParser\Node\Param; -use PhpTramp\Ignore\SuppressionIndex; /** * Accumulates every `TrampIgnore` attribute and `// phptramp-ignore` comment - * seen while {@see IndexingVisitor} traverses a project, and turns them into a - * {@see SuppressionIndex}. A separate collaborator so that suppression - * bookkeeping - a distinct responsibility from recording the class hierarchy - * and pending methods - does not inflate IndexingVisitor's own complexity. + * seen while {@see IndexingVisitor} traverses a file, and exposes them as the + * raw parts the Indexer merges across files before building a + * {@see \PhpTramp\Ignore\SuppressionIndex}. A separate collaborator so that + * suppression bookkeeping - a distinct responsibility from recording the class + * hierarchy and pending methods - does not inflate IndexingVisitor's own + * complexity. * * Attribute matching is by short name only: an attribute counts iff its * name's last segment is `TrampIgnore`, so analyzed codebases are never @@ -73,11 +74,15 @@ public function recordIgnoreComments(string $file, string $code): void } /** + * The raw suppression parts accumulated so far, before they are aggregated + * into a {@see \PhpTramp\Ignore\SuppressionIndex}. The Indexer merges these + * across per-file visitors and builds one index from the concatenation. + * * @param list $pending */ - public function suppressions(array $pending): SuppressionIndex + public function parts(array $pending): SuppressionParts { - return new SuppressionIndex( + return new SuppressionParts( $this->expandClassSuppressions($pending), $this->suppressedParams, $this->ignoreLines, diff --git a/src/Index/SuppressionParts.php b/src/Index/SuppressionParts.php new file mode 100644 index 0000000..a0d2add --- /dev/null +++ b/src/Index/SuppressionParts.php @@ -0,0 +1,60 @@ + $methods FQMNs suppressed by a method- or class-level attribute + * @param list $params [fqmn, paramName] pairs + * @param array> $lines file -> lines carrying an ignore comment + */ + public function __construct( + public readonly array $methods, + public readonly array $params, + public readonly array $lines, + ) { + } + + /** + * Folds per-file parts into one: methods and params are concatenated in + * input order, ignore lines are unioned by file. Folding all parts in a + * single pass keeps the merge linear in the total number of entries. + */ + public static function merge(self ...$parts): self + { + $methods = []; + $params = []; + $lines = []; + + foreach ($parts as $part) { + foreach ($part->methods as $method) { + $methods[] = $method; + } + foreach ($part->params as $param) { + $params[] = $param; + } + foreach ($part->lines as $file => $fileLines) { + $lines[$file] = $fileLines; + } + } + + return new self($methods, $params, $lines); + } + + public function toSuppressionIndex(): SuppressionIndex + { + return new SuppressionIndex($this->methods, $this->params, $this->lines); + } +} diff --git a/tests/Cache/FileIndexCacheTest.php b/tests/Cache/FileIndexCacheTest.php new file mode 100644 index 0000000..4755bbc --- /dev/null +++ b/tests/Cache/FileIndexCacheTest.php @@ -0,0 +1,254 @@ + */ + private array $directories = []; + + /** @var list */ + private array $files = []; + + protected function tearDown(): void + { + foreach ($this->files as $file) { + if (is_file($file)) { + @unlink($file); + } + } + foreach ($this->directories as $directory) { + if (is_dir($directory)) { + @chmod($directory, 0o755); + $this->removeTree($directory); + } + } + } + + public function testPutThenGetRoundTripsAnEqualFileIndex(): void + { + $cache = new FileIndexCache($this->cacheDirectory()); + $file = $this->sourceFile(self::SAMPLE); + $index = (new FileIndexer())->index($file); + + $cache->put($file, $index); + $hit = $cache->get($file); + + self::assertNotNull($hit); + self::assertEquals($index, $hit); + } + + public function testGetOnNeverPutFileReturnsNull(): void + { + $cache = new FileIndexCache($this->cacheDirectory()); + $file = $this->sourceFile(self::SAMPLE); + + self::assertNull($cache->get($file)); + } + + public function testGetReturnsNullWhenSourceMtimeAdvances(): void + { + $cache = new FileIndexCache($this->cacheDirectory()); + $file = $this->sourceFile(self::SAMPLE); + $index = (new FileIndexer())->index($file); + + $cache->put($file, $index); + + touch($file, time() + 10); + + self::assertNull($cache->get($file)); + } + + public function testGetReturnsNullWhenSourceSizeChanges(): void + { + $cache = new FileIndexCache($this->cacheDirectory()); + $file = $this->sourceFile(self::SAMPLE); + $index = (new FileIndexer())->index($file); + + $cache->put($file, $index); + + // Capture to the second — stat() records integer mtime, so sub-second + // drift is irrelevant; restoring to the exact cached second neutralizes + // mtime so the size check is the only differing field. + $originalMtime = filemtime($file); + + file_put_contents($file, self::SAMPLE . "\n// appended\n"); + touch($file, $originalMtime); + + // Restoring mtime to the second the file already carries is a no-op + // touch, which on some platforms (PHP 8.2) does not flush PHP's stat + // cache — so the stat here would report the pre-write size. Flush it so + // get() sees the grown file, exactly as a fresh process would. + clearstatcache(); + + self::assertNull( + $cache->get($file), + 'size mismatch alone (mtime restored to cached value) must invalidate the entry', + ); + } + + public function testGetReturnsNullOnCorruptPayload(): void + { + $directory = $this->cacheDirectory(); + $cache = new FileIndexCache($directory); + $file = $this->sourceFile(self::SAMPLE); + + $cache->put($file, (new FileIndexer())->index($file)); + + $entryPath = $directory . '/' . sha1(realpath($file)) . '.cache'; + file_put_contents($entryPath, 'this is not serialized payload'); + + self::assertNull($cache->get($file)); + } + + public function testGetReturnsNullOnFormatMismatch(): void + { + $directory = $this->cacheDirectory(); + $cache = new FileIndexCache($directory); + $file = $this->sourceFile(self::SAMPLE); + + $realpath = realpath($file); + $stat = stat($file); + $payload = serialize([ + 'format' => 0, + 'version' => Application::VERSION, + 'path' => $realpath, + 'mtime' => $stat['mtime'], + 'size' => $stat['size'], + 'index' => (new FileIndexer())->index($file), + ]); + + $entryPath = $directory . '/' . sha1($realpath) . '.cache'; + file_put_contents($entryPath, $payload); + + self::assertNull($cache->get($file)); + } + + public function testPutIntoReadOnlyDirectoryDoesNotThrow(): void + { + $directory = $this->cacheDirectory(); + chmod($directory, 0o500); + + $cache = new FileIndexCache($directory); + $file = $this->sourceFile(self::SAMPLE); + + $cache->put($file, (new FileIndexer())->index($file)); + + $this->expectNotToPerformAssertions(); + } + + public function testGetReturnsNullWhenPayloadUnserializesToANonArray(): void + { + $directory = $this->cacheDirectory(); + $cache = new FileIndexCache($directory); + $file = $this->sourceFile(self::SAMPLE); + + $realpath = realpath($file); + $entryPath = $directory . '/' . sha1($realpath) . '.cache'; + // A valid serialization of a non-array scalar: unserialize returns the + // scalar (not false), so the `=== false` guard alone must not let it + // through — the `!is_array` half of the check is what catches it. + file_put_contents($entryPath, serialize(42)); + + self::assertNull($cache->get($file)); + } + + public function testGetReturnsNullWhenPayloadUnserializesToAnObject(): void + { + $directory = $this->cacheDirectory(); + $cache = new FileIndexCache($directory); + $file = $this->sourceFile(self::SAMPLE); + + $realpath = realpath($file); + $entryPath = $directory . '/' . sha1($realpath) . '.cache'; + // A non-allowed top-level object becomes __PHP_Incomplete_Class, whose + // array access THROWS an Error that ?? does not swallow. Without the + // `!is_array` guard, get() would let the Error escape — violating the + // transparency contract. This test pins that the guard catches it. + file_put_contents($entryPath, serialize((object) ['x' => 1])); + + self::assertNull($cache->get($file)); + } + + public function testPutCreatesCacheDirectoryLazily(): void + { + $directory = $this->uncreatedCacheDirectory(); + $cache = new FileIndexCache($directory); + $file = $this->sourceFile(self::SAMPLE); + $index = (new FileIndexer())->index($file); + + $cache->put($file, $index); + + self::assertEquals($index, $cache->get($file)); + } + + private function cacheDirectory(): string + { + $directory = sys_get_temp_dir() . '/' . uniqid('phptramp-cache-', true); + mkdir($directory, 0o755, true); + $this->directories[] = $directory; + + return $directory; + } + + /** + * Returns a cache directory path whose parent exists but which itself does + * not yet exist — exercises {@see FileIndexCache::put()}'s lazy `mkdir`. + */ + private function uncreatedCacheDirectory(): string + { + $directory = sys_get_temp_dir() . '/' . uniqid('phptramp-cache-', true); + $this->directories[] = $directory; + + return $directory; + } + + private function sourceFile(string $code): string + { + $file = tempnam(sys_get_temp_dir(), 'phptramp-src') . '.php'; + file_put_contents($file, $code); + $this->files[] = $file; + + return $file; + } + + private function removeTree(string $directory): void + { + foreach (glob($directory . '/*') ?: [] as $entry) { + if (is_dir($entry)) { + $this->removeTree($entry); + } else { + @unlink($entry); + } + } + @rmdir($directory); + } +} diff --git a/tests/Config/ConfigLoaderTest.php b/tests/Config/ConfigLoaderTest.php index 4d41ccd..72f5a8b 100644 --- a/tests/Config/ConfigLoaderTest.php +++ b/tests/Config/ConfigLoaderTest.php @@ -203,4 +203,22 @@ public function testWarnLimitIsMapped(): void self::assertSame(2, $options->warnLimit); } + + public function testCacheKeyIsMapped(): void + { + $this->writeConfig('phptramp.json', '{"cache": "build/tramp-cache"}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame('build/tramp-cache', $options->cacheDir); + } + + public function testWrongTypeForCacheThrows(): void + { + $this->writeConfig('phptramp.json', '{"cache": 5}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } } diff --git a/tests/Console/ApplicationTest.php b/tests/Console/ApplicationTest.php index 771b9ae..9e35e03 100644 --- a/tests/Console/ApplicationTest.php +++ b/tests/Console/ApplicationTest.php @@ -21,14 +21,39 @@ final class ApplicationTest extends TestCase /** @var list */ private array $folders = []; + /** @var list */ + private array $directoriesToRemove = []; + protected function tearDown(): void { foreach ($this->folders as $folder) { - array_map('unlink', glob($folder . '/*') ?: []); - if (is_dir($folder)) { - rmdir($folder); + $this->removeTree($folder); + } + foreach ($this->directoriesToRemove as $directory) { + $this->removeTree($directory); + } + } + + /** + * Recursively removes a directory and its contents. Fixture folders now + * hold a `.phptramp.cache/` subdirectory (the default cache is on), so the + * cleanup must descend one level rather than only unlink direct children. + */ + private function removeTree(string $directory): void + { + if (! is_dir($directory)) { + return; + } + // scandir (not glob) so dotfiles like `.phptramp.cache/` are cleared. + foreach (array_diff(scandir($directory) ?: [], ['.', '..']) as $entry) { + $path = $directory . '/' . $entry; + if (is_dir($path)) { + $this->removeTree($path); + } else { + unlink($path); } } + rmdir($directory); } protected function setUp(): void @@ -69,7 +94,7 @@ public function testHelpDocumentsTheCliContract(): void $help = self::contents($this->stdout); $documented = [ '--folder', '--file', '--files', '--limit', - '--format', '--changed-only', '--baseline', 'Exit codes', + '--format', '--changed-only', '--baseline', '--no-cache', 'Exit codes', ]; foreach ($documented as $expected) { self::assertStringContainsString($expected, $help); @@ -584,6 +609,136 @@ public function testChangedOnlySkipsStaleDetectionEntirely(): void self::assertSame([], $this->linesStartingWith($stderr, 'phptramp: stale')); } + /** + * Two consecutive runs against the same cache dir must produce + * byte-identical stdout (cache transparency) and leave one `.cache` entry + * per indexed source file. The cache dir lives outside the analyzed cwd + * so the file locator never sees it. + */ + public function testTwoRunsShareCacheAndProduceByteIdenticalStdoutWithOneEntryPerSourceFile(): void + { + [$cwd, $cacheDir] = $this->cacheableCwdWithThreeHopChain(); + + $firstStdout = $this->runInCwd($cwd); + self::assertStringContainsString('FINDING', $firstStdout); + + $secondStdout = $this->runInCwd($cwd); + + self::assertSame($firstStdout, $secondStdout); + self::assertCount(1, glob($cacheDir . '/*.cache') ?: []); + } + + /** + * Editing a source file between runs invalidates its cache entry by + * mtime/size, so the re-parse surfaces the new chain. Proves the cache + * never masks a real change. + */ + public function testEditedSourceFileInvalidatesCacheAndSurfacesNewFinding(): void + { + [$cwd, $cacheDir] = $this->cacheableCwdWithOneHopChain(); + + $firstStdout = $this->runInCwd($cwd); + self::assertStringContainsString('No tramp data found', $firstStdout); + + file_put_contents($cwd . '/Demo.php', $this->threeHopChainCode()); + + $secondStdout = $this->runInCwd($cwd); + self::assertStringContainsString('FINDING', $secondStdout); + } + + /** + * --no-cache disables the cache entirely: even with a configured cache dir, + * no cache directory is ever created on disk. + */ + public function testNoCacheFlagLeavesCacheDirAbsent(): void + { + $folder = sys_get_temp_dir() . '/phptramp-cache-' . uniqid(); + mkdir($folder); + $this->folders[] = $folder; + file_put_contents($folder . '/Demo.php', $this->threeHopChainCode()); + + $cacheDir = $folder . '/.phptramp.cache'; + + self::assertSame(1, $this->app->run([ + 'phptramp', '--folder', $folder, '--no-config', '--no-cache', + ])); + self::assertDirectoryDoesNotExist($cacheDir); + } + + /** + * Builds a temp cwd containing a 3-hop chain Demo.php plus a phptramp.json + * that points the cache at a separate temp dir, and returns both paths. + * + * @return array{0: string, 1: string} [cwd, cacheDir] + */ + private function cacheableCwdWithThreeHopChain(): array + { + return $this->cacheableCwd($this->threeHopChainCode()); + } + + /** @return array{0: string, 1: string} [cwd, cacheDir] */ + private function cacheableCwdWithOneHopChain(): array + { + $oneHop = 'c = $config; } }'; + + return $this->cacheableCwd($oneHop); + } + + /** + * @return array{0: string, 1: string} [cwd, cacheDir] + */ + private function cacheableCwd(string $sourceCode): array + { + $cwd = sys_get_temp_dir() . '/phptramp-cwd-' . uniqid(); + $cacheDir = sys_get_temp_dir() . '/phptramp-cache-' . uniqid(); + mkdir($cwd); + $this->folders[] = $cwd; + $this->directoriesToRemove[] = $cacheDir; + + file_put_contents($cwd . '/Demo.php', $sourceCode); + file_put_contents( + $cwd . '/phptramp.json', + '{"paths": ["."], "cache": ' . json_encode($cacheDir) . '}', + ); + + return [$cwd, $cacheDir]; + } + + /** + * Runs a fresh Application with cwd set to $cwd and returns its stdout. + */ + private function runInCwd(string $cwd): string + { + $stdout = fopen('php://memory', 'w+'); + $stderr = fopen('php://memory', 'w+'); + self::assertIsResource($stdout); + self::assertIsResource($stderr); + + $app = new Application($stdout, $stderr); + $previousCwd = getcwd(); + self::assertIsString($previousCwd); + + try { + chdir($cwd); + $app->run(['phptramp']); + } finally { + chdir($previousCwd); + } + + return self::contents($stdout); + } + + private function threeHopChainCode(): string + { + return 'process($config); } } ' + . 'class ServiceA { public function process(Cfg $config): void { (new ServiceB())->run($config); } } ' + . 'class ServiceB { public function run(Cfg $config): void { new Mailer($config); } } ' + . 'class Mailer { private Cfg $c; public function __construct(Cfg $config) { $this->c = $config; } }'; + } + /** * @param list $argv */ diff --git a/tests/Console/ArgvParserTest.php b/tests/Console/ArgvParserTest.php index 90a6ca9..d110649 100644 --- a/tests/Console/ArgvParserTest.php +++ b/tests/Console/ArgvParserTest.php @@ -180,6 +180,21 @@ public function testNoConfigDefaultsToFalse(): void self::assertFalse($this->parse()->noConfig); } + public function testNoCacheFlagSetsNoCache(): void + { + self::assertTrue($this->parse('--no-cache')->noCache); + } + + public function testNoCacheDefaultsToFalse(): void + { + self::assertFalse($this->parse()->noCache); + } + + public function testCacheDirDefaultsToPhpTrampCache(): void + { + self::assertSame('.phptramp.cache', $this->parse()->cacheDir); + } + public function testHelpFlags(): void { self::assertTrue($this->parse('--help')->help); diff --git a/tests/FixtureTest.php b/tests/FixtureTest.php index f7e0673..2d8e6dc 100644 --- a/tests/FixtureTest.php +++ b/tests/FixtureTest.php @@ -126,7 +126,7 @@ private function runFindingsJson(string $caseDir): array private function runFolderMode(string $caseDir): array { $argv = [ - 'phptramp', '--folder', $caseDir . '/src', '--no-config', '--format', 'json', + 'phptramp', '--folder', $caseDir . '/src', '--no-config', '--no-cache', '--format', 'json', ...$this->extraCliArgs($caseDir), ]; [$exitCode, $decoded] = $this->runApplication($argv); @@ -139,7 +139,9 @@ private function runFolderMode(string $caseDir): array * supplies paths/exclude/limit, then run with no `--folder` at all. Always * restores the previous cwd — other tests depend on it. Findings' `file` * values come out already fixture-relative (cwd = fixture dir at render - * time), so no prefix-strip is needed. + * time), so no prefix-strip is needed. `--no-cache` keeps fixtures + * hermetic: they pin findings semantics, not cache behavior, and must not + * litter a `.phptramp.cache/` into the committed fixture directory. * * @return array{0: int, 1: array} */ @@ -151,7 +153,7 @@ private function runConfigMode(string $caseDir): array try { self::assertTrue(chdir($caseDir)); - return $this->runApplication(['phptramp', '--format', 'json']); + return $this->runApplication(['phptramp', '--no-cache', '--format', 'json']); } finally { chdir($previousCwd); } @@ -271,7 +273,7 @@ private function dumpIndex(string $srcDir): array self::assertIsResource($stderr); $exit = (new Application($stdout, $stderr))->run( - ['phptramp', '--folder', $srcDir, '--no-config', '--dump-index'], + ['phptramp', '--folder', $srcDir, '--no-config', '--no-cache', '--dump-index'], ); self::assertSame(0, $exit); diff --git a/tests/Index/FileIndexerTest.php b/tests/Index/FileIndexerTest.php new file mode 100644 index 0000000..49aaa51 --- /dev/null +++ b/tests/Index/FileIndexerTest.php @@ -0,0 +1,113 @@ +file !== '' && is_file($this->file)) { + unlink($this->file); + } + } + + private function index(string $code): FileIndex + { + $this->file = tempnam(sys_get_temp_dir(), 'phptramp') . '.php'; + file_put_contents($this->file, $code); + + return (new FileIndexer())->index($this->file); + } + + public function testCarriesTheClassifiedMethod(): void + { + $index = $this->index(self::SAMPLE); + + $method = $index->methods['Demo\Service::handle'] ?? null; + self::assertNotNull($method); + self::assertSame('Demo\Service::handle', $method->fqmn); + self::assertSame($this->file, $method->file); + self::assertSame('Demo\Service', $method->class); + + $param = $method->paramNamed('unused'); + self::assertNotNull($param); + self::assertSame(ParamFate::Unused, $param->fate); + } + + public function testCarriesTheClass(): void + { + $index = $this->index(self::SAMPLE); + + $class = $index->classes['Demo\Service'] ?? null; + self::assertNotNull($class); + self::assertSame('Demo\Service', $class->name); + self::assertSame(ClassKind::ConcreteClass, $class->kind); + } + + public function testCarriesAllThreeSuppressionPartKinds(): void + { + $index = $this->index(self::SAMPLE); + + self::assertContains('Demo\Service::handle', $index->suppression->methods); + self::assertSame([['Demo\Service::handle', 'unused']], $index->suppression->params); + self::assertArrayHasKey($this->file, $index->suppression->lines); + self::assertCount(1, $index->suppression->lines[$this->file]); + } + + public function testUnreadablePathThrowsParseExceptionNamingTheFile(): void + { + $missing = sys_get_temp_dir() . '/phptramp_does_not_exist_' . bin2hex(random_bytes(4)) . '.php'; + + try { + (new FileIndexer())->index($missing); + self::fail('expected ParseException'); + } catch (ParseException $e) { + self::assertStringContainsString($missing, $e->getMessage()); + } + } + + public function testSyntaxErrorThrowsParseExceptionNamingTheFile(): void + { + $this->file = tempnam(sys_get_temp_dir(), 'phptramp') . '.php'; + file_put_contents($this->file, "index($this->file); + self::fail('expected ParseException'); + } catch (ParseException $e) { + self::assertStringContainsString($this->file, $e->getMessage()); + } + } +} diff --git a/tests/Index/IndexerTest.php b/tests/Index/IndexerTest.php index 42d5235..293f9c3 100644 --- a/tests/Index/IndexerTest.php +++ b/tests/Index/IndexerTest.php @@ -52,11 +52,20 @@ function helper(Cfg $config): void private string $file = ''; + /** @var list */ + private array $files = []; + protected function tearDown(): void { if ($this->file !== '' && is_file($this->file)) { unlink($this->file); } + + foreach ($this->files as $file) { + if (is_file($file)) { + unlink($file); + } + } } private function index(string $code): MethodIndex @@ -67,6 +76,31 @@ private function index(string $code): MethodIndex return (new Indexer())->index([$this->file]); } + private function writeFile(string $code): string + { + $file = tempnam(sys_get_temp_dir(), 'phptramp') . '.php'; + file_put_contents($file, $code); + $this->files[] = $file; + + return $file; + } + + /** + * @return array{MethodIndex, string, string} + */ + private function indexFiles(string $codeA, string $codeB): array + { + $fileA = $this->writeFile($codeA); + $fileB = $this->writeFile($codeB); + + return [(new Indexer())->index([$fileA, $fileB]), $fileA, $fileB]; + } + + private function lineOf(string $code, string $marker): int + { + return substr_count(substr($code, 0, strpos($code, $marker)), "\n") + 1; + } + public function testIndexesMethodsFunctionsAndTraitMethods(): void { $index = $this->index(self::SAMPLE); @@ -184,4 +218,89 @@ public function testParseErrorMessageNamesTheFileAndReason(): void self::assertStringContainsString($this->file, $e->getMessage()); } } + + public function testMergesMethodsAndClassesFromMultipleFiles(): void + { + $codeA = <<<'PHP' + indexFiles($codeA, $codeB); + + self::assertNotNull($index->get('DemoA\ServiceA::handle')); + self::assertNotNull($index->get('DemoB\ServiceB::run')); + self::assertNotNull($index->classInfo('DemoA\ServiceA')); + self::assertNotNull($index->classInfo('DemoB\ServiceB')); + } + + public function testMergesSuppressedLinesFromMultipleFiles(): void + { + $codeA = <<<'PHP' + indexFiles($codeA, $codeB); + $suppressions = $index->suppressions(); + + self::assertTrue($suppressions->suppressesLine($fileA, $this->lineOf($codeA, '// phptramp-ignore'))); + self::assertTrue($suppressions->suppressesLine($fileB, $this->lineOf($codeB, '// phptramp-ignore'))); + } + + public function testParseErrorAggregatesMessagesFromAllBadFiles(): void + { + $fileA = $this->writeFile("writeFile("index([$fileA, $fileB]); + self::fail('expected ParseException'); + } catch (ParseException $e) { + self::assertStringContainsString($fileA, $e->getMessage()); + self::assertStringContainsString($fileB, $e->getMessage()); + } + } } diff --git a/tests/Index/SuppressionPartsTest.php b/tests/Index/SuppressionPartsTest.php new file mode 100644 index 0000000..b9c3233 --- /dev/null +++ b/tests/Index/SuppressionPartsTest.php @@ -0,0 +1,60 @@ + [3]], + ); + $second = new SuppressionParts( + ['B::two'], + [['B::two', 'y']], + ['b.php' => [7, 9]], + ); + + $merged = SuppressionParts::merge($first, $second); + + self::assertSame(['A::one', 'B::two'], $merged->methods); + self::assertSame([['A::one', 'x'], ['B::two', 'y']], $merged->params); + self::assertSame(['a.php' => [3], 'b.php' => [7, 9]], $merged->lines); + } + + public function testMergeOfNoPartsIsEmpty(): void + { + $merged = SuppressionParts::merge(); + + self::assertSame([], $merged->methods); + self::assertSame([], $merged->params); + self::assertSame([], $merged->lines); + } + + public function testToSuppressionIndexExposesEveryMergedPart(): void + { + $parts = new SuppressionParts( + ['A::one'], + [['A::one', 'x']], + ['a.php' => [3]], + ); + + $index = $parts->toSuppressionIndex(); + + self::assertTrue($index->suppressesMethod('A::one')); + self::assertTrue($index->suppressesParam('A::one', 'x')); + self::assertTrue($index->suppressesLine('a.php', 3)); + } +}