diff --git a/.gitignore b/.gitignore index 32a5f86..84893ba 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /.phpunit.cache/ /.idea/ /infection.log +/.superpowers/ diff --git a/README.md b/README.md index b6449ef..67a1af9 100644 --- a/README.md +++ b/README.md @@ -4,10 +4,11 @@ > methods that never use them — "this parameter was passed through 4 classes / 5 methods > before being used." -**Status: Phase 2.** Cross-file chain reporting works: `phptramp --folder src` -stitches forwarding chains across files and prints text findings, exiting `0`/`1`/`2`. -The classifier and whole-project index remain inspectable via `--dump-index`; other -output formats (`json`, `github`, …) land in Phase 3. +**Status: Phase 3.** 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. 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. ## What it does @@ -55,10 +56,14 @@ Add a script to your project's `composer.json` to invoke it the short way: ```bash composer tramp -composer tramp -- --changed-only --git-base origin/main +composer tramp -- --format json --warn-limit 2 ``` -## Planned CLI +phptramp itself does not ship a `composer.json` script of this name — `vendor/bin/phptramp` +(or `bin/phptramp` inside this repo) is the invocation; the snippet above is a convenience +you add to a *consuming* project. + +## CLI ```text phptramp [options] @@ -68,17 +73,78 @@ phptramp [options] --files Comma-separated list of files --limit Fail on chains with >= n pass-through hops (default: 3) --warn-limit Warn (do not fail CI) on chains with >= n hops - --format text|json|github|checkstyle|sarif (default: text) + --format text|json|github|checkstyle|sarif|summary (default: text) --explain Show why chains ended (call resolution trace) - --changed-only Only report chains touching changed lines - --git-base Diff base for --changed-only (default: origin/main) - --baseline Ignore findings recorded in the baseline - --generate-baseline Write current findings to a baseline file + --changed-only Reserved for diff-aware mode (Phase 4) — accepted, not yet wired up + --git-base Reserved for diff-aware mode (Phase 4) — accepted, not yet wired up + --baseline Reserved for baselining (Phase 5) — accepted, not yet wired up + --generate-baseline Reserved for baselining (Phase 5) — accepted, not yet wired up +``` + +Exit codes: `0` no finding at error severity, `1` at least one finding at/over +`--limit`, `2` tool error — so it drops straight into any CI pipeline. + +## Configuration + +phptramp reads `phptramp.json` (falling back to `phptramp.dist.json`) from the current +working directory. CLI flags always take precedence over the config file. An unknown +key, or a value of the wrong type, is a config error rather than a silently ignored typo. + +```json +{ + "paths": ["src"], + "exclude": ["src/Legacy/*.php"], + "limit": 3, + "warnLimit": 2, + "format": "json" +} ``` -Exit codes: `0` no findings, `1` findings at/over `--limit`, `2` tool error — -so it drops straight into any CI pipeline. Configuration lives in `phptramp.json` -(or `phptramp.dist.json`); CLI flags override config. +| Key | Type | Same as | +|---|---|---| +| `paths` | list of strings | entries ending in `.php` become `--file`s, everything else a `--folder` | +| `exclude` | list of strings | `fnmatch` globs matched against each folder-discovered file's path, relative to the config file's directory; explicit files (`paths` entries ending in `.php`) are never excluded | +| `limit` | integer | `--limit` | +| `warnLimit` | integer | `--warn-limit` | +| `format` | string | `--format` | +| `baseline` | string | `--baseline` (reserved for Phase 5) | + +## Output formats + +`--format` selects the renderer; all six are implemented and each has an exact-string +unit test. `json` is additionally exercised end-to-end by the `tests/fixtures/` harness, +which always runs with `--format json`. + +| Format | One-line example | +|---|---| +| `text` | `FINDING $config: 3 pass-through hops across 4 classes` | +| `json` | `{"limit":3,"warnLimit":null,"findings":[{"param":"config","severity":"error","hops":3,...}]}` | +| `github` | `::error file=src/Demo.php,line=12,title=phptramp%3A%3A$config%3A 3 pass-through hops across 4 classes (terminal%3A Demo\Mailer%3A%3A__construct [stored])` | +| `checkstyle` | `` | +| `sarif` | `{"ruleId":"phptramp.trampData","level":"error","message":{"text":"$config: 3 pass-through hops across 4 classes (terminal: Demo\\Mailer::__construct [stored])"}}` | +| `summary` | `12 chains total; 5 at or over the limit (limit: 3 hops).` | + +## Suppression + +Mark a specific false positive as intentional instead of raising `--limit` project-wide: + +- **`#[TrampIgnore]`** on a class, method, function, or parameter. Matching is by the + attribute name's *short name*, so an analyzed codebase never has to `require` or + autoload phptramp's own `PhpTramp\Ignore\TrampIgnore` class to use it. +- **`// phptramp-ignore`** as a real PHP comment (a marker inside a string literal does + not count) on a hop's declaration line, the line directly above it, or a forwarding + call-site line. + +Either drops the *entire chain* passing through that hop, not just the one parameter +reaching the flagged declaration. + +## `--warn-limit` + +`--warn-limit ` adds a second, lower threshold below `--limit`. Chains whose hop +count falls in `[warn-limit, limit)` render at `severity: "warning"` (`WARNING` in text, +`::warning` in the GitHub format, `"level": "warning"` in SARIF, …) but never fail the +run — only chains at or over `--limit` set exit code `1`. Useful for tightening a hop +budget gradually: warn today, promote to a hard failure once the codebase is clean. ## IDE integration diff --git a/docs/plan.md b/docs/plan.md index 1d0cd37..adaf828 100644 --- a/docs/plan.md +++ b/docs/plan.md @@ -680,26 +680,36 @@ Tasks (expanded in `docs/plans/phase-2.md`): --- -## Phase 3 — CI & config surface +## Phase 3 — CI & config surface ✅ (issue #5) **Deliverable:** all output formats, config file, suppression, dual thresholds, summary. -- **3.1 ConfigLoader:** `phptramp.json` then `phptramp.dist.json`; keys `paths`, +**Detailed step-by-step plan: [docs/plans/phase-3.md](plans/phase-3.md)** — written +against the actual Phase 1–2 interfaces; where it refines the sketches below (e.g. +`Severity`/`Thresholds` as reporting-layer concepts, config precedence via +parser-default seeding, suppression exposed through `MethodIndex::suppressions()`), +the detailed plan wins. + +- [x] **3.1 ConfigLoader:** `phptramp.json` then `phptramp.dist.json`; keys `paths`, `exclude`, `limit`, `warnLimit`, `format`, `baseline`; strict unknown-key error; precedence CLI > config > defaults (merge into `Options`). `exclude` glob patterns feed FileLocator. -- **3.2 Formats:** `JsonReporter` (the `expected.json` schema — one schema everywhere), +- [x] **3.2 Formats:** `JsonReporter` (the `expected.json` schema — one schema everywhere), `GithubReporter` (`::error file=...,line=...::...` per origin, `::notice` per hop), `CheckstyleReporter`, `SarifReporter` (rule id `phptramp.trampData`). Fixtures assert each format byte-for-byte. -- **3.3 Suppression:** `#[TrampIgnore]` attribute class + `// phptramp-ignore` comments +- [x] **3.3 Suppression:** `#[TrampIgnore]` attribute class + `// phptramp-ignore` comments scanned during indexing into a `SuppressionIndex`; ChainBuilder drops chains with any suppressed hop (frozen rule 11). -- **3.4 Dual thresholds:** `--warn-limit` — findings in `[warn, limit)` render as +- [x] **3.4 Dual thresholds:** `--warn-limit` — findings in `[warn, limit)` render as warnings, never affect exit code. GitHub format uses `::warning`. -- **3.5 SummaryReporter:** `--format=summary` — histogram of chain lengths, top 10 +- [x] **3.5 SummaryReporter:** `--format=summary` — histogram of chain lengths, top 10 longest chains, most-forwarded parameters. +`--changed-only`/`--git-base` (Phase 4) and `--baseline`/`--generate-baseline` (Phase 5) +remain accepted-but-inert CLI flags — parsed into `Options` for forward compatibility, +not yet consumed by analysis. Not a Phase 3 deliverable; tracked in their own phases. + --- ## Phase 4 — Diff-aware mode (flagship) diff --git a/docs/plans/phase-3.md b/docs/plans/phase-3.md new file mode 100644 index 0000000..2529c09 --- /dev/null +++ b/docs/plans/phase-3.md @@ -0,0 +1,449 @@ +# Phase 3 Implementation Plan — CI & config surface + +> **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 3 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. + +**Goal:** phptramp becomes CI-shaped: machine formats (`json`, `github`, +`checkstyle`, `sarif`), a `phptramp.json` config file with CLI-over-config +precedence and `exclude` globs, `#[TrampIgnore]` / `// phptramp-ignore` +suppression, `--warn-limit` warning severity, and `--format=summary`. + +**Architecture:** The chain layer stays untouched except for one filter hook +(suppression). New `src/Config/` loads and merges the config file *under* CLI +flags by seeding `ArgvParser` with config-derived defaults. `src/Report/` grows a +`Reporter` interface, a `Thresholds`/`Severity` pair (severity is a *reporting* +concept — `Finding` stays threshold-agnostic), and one reporter per format behind +a factory. Suppression is collected during indexing (attributes + comments are +only visible there) and exposed via `MethodIndex::suppressions()` so existing +call sites keep working. + +**Tech stack:** unchanged — PHP ≥ 8.2, nikic/php-parser ^5 (only prod dependency). + +## 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: #5.** Branch `feature/5-ci-config-surface` off + `develop` (already created, carries this plan). Commits `type(#5): …`. PR into + `develop` with `Closes #5`; diff-scoped Infection gate (minMsi 80) applies. +- Frozen semantics rule 11 (suppression) is the contract for Task 7. Reporter + output contracts pinned in this plan are byte-for-byte — a fixture asserts each. +- Exit-code contract unchanged: `0` no errors, `1` ≥ 1 error-severity finding, + `2` tool error. Warnings never affect the exit code. + +## Existing interfaces this phase consumes (Phase 1–2, verbatim) + +- `Finding{ string $param, string $origin, ?string $terminal, TerminalKind $terminalKind, int $hops, list $chain, int $classes, list $notes, list $trace }` + — chain includes the terminal node only for kinds `used|stored|&-terminated|unused-end`. +- `Hop{ string $fqmn, ?string $class, string $file, int $line, ?int $forwardLine }` + — `file` is realpath-absolute; `forwardLine` null on the terminal node. +- `TerminalKind: string { Used='used', Stored='stored', ByRef='&-terminated', Unused='unused-end', External='external', Truncated='truncated' }`. +- `ChainBuilder::__construct(CallResolver)`, `::build(MethodIndex): list` + (unfiltered; delegates to `ChainTraversal`). +- `TextReporter::__construct(int $limit, bool $explain = false)`, + `::render(list): string` — output format pinned in + `tests/Report/TextReporterTest.php`. +- `ArgvParser::parse(list $args): Options` — already accepts + `--warn-limit`, `--format` with the six valid values, `--baseline`; unknown + flags/formats throw `InvalidArgsException`. `Options` already carries + `warnLimit`, `format`, `baseline` (all currently unused downstream). +- `Application::analyze()` currently hard-rejects `format !== 'text'` with exit 2 + — that guard dies in Task 2. +- `FileLocator::locate(Options): list` — folders + files, realpath, + sorted; no exclude support yet. +- `MethodIndex{ get(), all(), classInfo() }`, built solely by `Indexer::index()`. +- Fixture harness `tests/FixtureTest.php`: `expected-index.json` compares + `--dump-index` through the CLI; `expected-findings.json` currently calls + `ChainBuilder` directly (flattened chains) — Task 3 switches it to the CLI. + +## File structure (this phase) + +``` +src/Report/Severity.php NEW enum error|warning (Task 1) +src/Report/Thresholds.php NEW limit + warnLimit -> severityOf(Finding) (Task 1) +src/Report/Reporter.php NEW interface (Task 1) +src/Report/ReporterFactory.php NEW format string -> Reporter (Task 1) +src/Report/TextReporter.php MOD implements Reporter; renders warnings (Task 1) +src/Console/Application.php MOD thresholds + factory wiring; exit on errors only (Task 1) +src/Report/Paths.php NEW cwd-relativizing helper for all reporters (Task 2) +src/Report/JsonReporter.php NEW (Task 2) +tests/FixtureTest.php MOD findings mode goes through the CLI (Task 3) +src/Config/ConfigException.php NEW (Task 4) +src/Config/ConfigLoader.php NEW phptramp.json -> seed Options (Task 4) +src/Console/ArgvParser.php MOD parse(args, Options $defaults) (Task 4) +src/Console/Options.php MOD + readonly list $exclude (Task 5) +src/Discovery/FileLocator.php MOD exclude glob filter (Task 5) +src/Report/GithubReporter.php NEW (Task 6) +src/Report/CheckstyleReporter.php NEW (Task 6) +src/Report/SarifReporter.php NEW (Task 6) +src/Ignore/TrampIgnore.php NEW the attribute class (Task 7) +src/Ignore/SuppressionIndex.php NEW (Task 7) +src/Index/IndexingVisitor.php MOD collect attributes + ignore comments (Task 7) +src/Index/MethodIndex.php MOD + suppressions(): SuppressionIndex (Task 7) +src/Chain/ChainTraversal.php MOD drop chains with a suppressed hop (Task 7) +src/Report/SummaryReporter.php NEW (Task 8) +tests/… mirrors all of the above +tests/fixtures/… format/config/suppression cases (Tasks 3, 9) +``` + +--- + +### Task 1: `Severity`, `Thresholds`, `Reporter` interface, warning rendering + +`--warn-limit` becomes real. Severity is computed at reporting time so `Finding` +stays a pure chain fact. + +**Files:** create `src/Report/{Severity,Thresholds,Reporter,ReporterFactory}.php`; +modify `src/Report/TextReporter.php`, `src/Console/Application.php`; +tests `tests/Report/ThresholdsTest.php`, extend `TextReporterTest`, +`ApplicationTest`. + +**Produces:** + +```php +enum Severity +{ + case Error; + case Warning; +} + +final class Thresholds +{ + /** @throws InvalidArgsException if warnLimit >= limit (a warn bar above the fail bar is a config error) */ + public function __construct( + public readonly int $limit, + public readonly ?int $warnLimit, + ) {} + + /** Null = below every threshold: not reported at all. */ + public function severityOf(Finding $finding): ?Severity; +} + +interface Reporter +{ + /** @param list $findings ALL findings, unfiltered — each reporter applies $thresholds itself */ + public function render(array $findings): string; +} + +final class ReporterFactory +{ + /** Constructed from Options; returns the Reporter for Options->format. */ + public function create(Options $options): Reporter; +} +``` + +Behavior: + +- `severityOf`: hops ≥ limit → `Error`; warnLimit set and hops ≥ warnLimit → + `Warning`; else null. +- `TextReporter` header for warnings is `WARNING` instead of `FINDING`; summary + line becomes `N findings (E errors, W warnings; limit: 3 hops, warn-limit: 2 hops).` + — with warnLimit unset, existing pinned output must NOT change (regression + guard: the current `TextReporterTest` expectations stay green untouched). +- `Application`: exit 1 iff any `Error`; reporters receive unfiltered findings. + The `format !== 'text'` guard stays for exactly the formats the factory cannot + build *yet* (it shrinks task by task and dies in Task 6). + +- [ ] **Step 1:** failing tests — `ThresholdsTest` (error at limit, warning in + `[warn, limit)`, null below, `warnLimit >= limit` throws); `TextReporterTest` + warning block + mixed summary line; `ApplicationTest`: warn-only run exits 0 + but prints `WARNING`. +- [ ] **Step 2:** red. **Step 3:** implement. **Step 4:** green + `composer check`. +- [ ] **Step 5:** commit `feat(#5): dual thresholds with warning severity`. + +### Task 2: `JsonReporter` + `Paths` + +**Files:** create `src/Report/{Paths,JsonReporter}.php`; +test `tests/Report/JsonReporterTest.php`. + +**Produces:** `Paths::relativize(string $absolute): string` — path relative to the +cwd passed at construction when the path lies under it, else unchanged. Every +machine reporter uses it (GitHub annotations and SARIF need workspace-relative +paths; JSON follows for consistency). + +**Output contract (pin byte-for-byte;** `JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES`, +trailing newline**):** + +```json +{ + "limit": 3, + "warnLimit": 2, + "findings": [ + { + "param": "config", + "severity": "error", + "origin": "Demo\\Controller::handle", + "terminal": "Demo\\Mailer::__construct", + "terminalKind": "stored", + "hops": 3, + "classes": 4, + "chain": [ + {"method": "Demo\\Controller::handle", "role": "origin", "file": "src/Demo.php", "line": 12, "forwardLine": 14}, + {"method": "Demo\\ServiceA::process", "role": "hop", "file": "src/Demo.php", "line": 18, "forwardLine": 20}, + {"method": "Demo\\ServiceB::run", "role": "hop", "file": "src/Demo.php", "line": 24, "forwardLine": 26}, + {"method": "Demo\\Mailer::__construct", "role": "terminal", "file": "src/Demo.php", "line": 32, "forwardLine": null} + ], + "notes": [] + } + ] +} +``` + +`warnLimit` renders as JSON `null` when unset. Roles: first chain entry `origin`, +last `terminal` only when the terminal node is in the chain (see `Finding` +contract), everything else `hop`. Findings below every threshold are omitted. +`severity` is `"error"` or `"warning"`. Empty run → `"findings": []`. + +- [ ] Steps: failing test (full document for the 3-hop case, warning case, empty + case) → red → implement → green + check → + commit `feat(#5): json reporter`. + +### Task 3: Fixture harness through the CLI + +**Files:** modify `tests/FixtureTest.php`; rewrite every +`tests/fixtures/*/expected-findings.json` to the Task 2 schema. + +The findings mode now invokes `Application` with +`['phptramp', '--folder', , '--format', 'json', '--limit', '1']` — real CLI, +real exit codes (`--limit 1` so every chain is an error and short chains stay +visible). Normalization stays: strip the case-dir prefix from `file` values. +Assert exit 1 when findings exist, 0 when not. + +- [ ] Steps: adapt the harness test-first (red on the first fixture), rewrite the + seven expected files by hand (do NOT generate them from the tool's own output — + a wrong tool would then pin its own bug; derive them from the fixture source), + green + check → commit `test(#5): fixture harness exercises the CLI json path`. + +### Task 4: `ConfigLoader` + precedence + +**Files:** create `src/Config/{ConfigException,ConfigLoader}.php`; modify +`src/Console/ArgvParser.php`, `src/Console/Application.php`; +tests `tests/Config/ConfigLoaderTest.php`, extend `ArgvParserTest`, +`ApplicationTest`. + +**Produces:** + +```php +final class ConfigLoader +{ + /** + * Reads phptramp.json, falling back to phptramp.dist.json, in $directory. + * Missing both -> Options defaults. Unknown key, non-JSON, or wrong value + * type -> ConfigException (exit 2). Never consults the environment. + */ + public function load(string $directory): Options; +} +``` + +- Recognized keys, mapped onto `Options`: `paths` (list of strings; entries + ending `.php` become `files`, all others `folders`), `exclude` (list of glob + strings — storage arrives in Task 5, the loader already accepts the key), + `limit`, `warnLimit`, `format`, `baseline`. Anything else → + `ConfigException("unknown config key: ")` — strict by design, typos must + not silently disable gating. +- **Precedence mechanic (CLI > config > defaults):** + `ArgvParser::parse(array $args, Options $defaults = new Options())` — the + parser's `reset()` seeds every field from `$defaults` instead of literals. Path + precedence is all-or-nothing: the first `--folder`/`--file`/`--files` flag + clears *both* seeded lists (config paths replaced entirely, never mixed). +- `Application` wires `ConfigLoader->load(getcwd())` as the parser's defaults; + a `ConfigException` prints to stderr and exits 2. + +- [ ] **Step 1:** failing tests — loader: reads `phptramp.json`; falls back to + `.dist`; prefers non-dist; missing → defaults; unknown key throws; `paths` + splits files/folders; wrong type (`"limit": "three"`) throws. Parser: seeded + defaults survive when flags absent; `--limit` overrides seed; first path flag + clears seeded paths. Application (chdir into a temp dir in try/finally): + config `limit: 1` makes a 1-hop chain fail without any CLI flag. +- [ ] **Step 2:** red. **Step 3:** implement. **Step 4:** green + check. +- [ ] **Step 5:** commit `feat(#5): phptramp.json config with CLI-over-config precedence`. + +### Task 5: `exclude` globs in `FileLocator` + +**Files:** modify `src/Console/Options.php` (add +`public readonly array $exclude = []`, `@param list`), +`src/Config/ConfigLoader.php` (store the key), `src/Discovery/FileLocator.php`; +tests extend `FileLocatorTest`, `ConfigLoaderTest`. + +Config-only (no CLI flag — YAGNI until asked; the master plan lists none). +Semantics, pinned: patterns are `fnmatch` globs with `FNM_PATHNAME` **not** set +(so `vendor/*` matches arbitrarily deep), matched against the path *relative to +the config directory* (= cwd), after realpath normalization. A file matching any +pattern is dropped after collection, before sorting. + +- [ ] Steps: failing tests (`vendor/*` drops nested file; `*Test.php` drops by + basename; explicit `--file` beats exclude — an explicitly named file is always + analyzed) → red → implement → green + check → + commit `feat(#5): exclude globs from config`. + +### Task 6: `GithubReporter`, `CheckstyleReporter`, `SarifReporter` + +**Files:** create the three reporters; register in `ReporterFactory` (the last +format guard in `Application` dies); tests one file per reporter. + +**GitHub contract (pin):** one annotation per finding at the origin, `::error` / +`::warning` by severity, plus one `::notice` per subsequent hop. Message data +escaping per Actions spec: `%` → `%25`, `\r` → `%0D`, `\n` → `%0A`. + +``` +::error file=src/Demo.php,line=12,title=phptramp::$config: 3 pass-through hops across 4 classes (terminal: Demo\Mailer::__construct [stored]) +::notice file=src/Demo.php,line=18,title=phptramp::hop 2 of $config chain from Demo\Controller::handle +``` + +Empty run → empty string, exit 0. + +**Checkstyle contract (pin):** findings grouped by origin file, one `` per +finding at the origin line, `source="phptramp.trampData"`, message XML-escaped +(`htmlspecialchars(..., ENT_XML1 | ENT_QUOTES)`): + +```xml + + + + + + +``` + +**SARIF contract (pin):** SARIF 2.1.0, one run, driver `name` `phptramp`, +`informationUri` `https://github.com/larspohlmann/phptramp`, **no `version` +property** (optional per spec; omitting keeps fixtures stable across releases), +one rule `phptramp.trampData`. One result per finding: `ruleId`, `level` +(`error`/`warning`), `message.text` (same message as checkstyle), one location at +the origin (`artifactLocation.uri` relative, `region.startLine`), and one +`relatedLocations` entry per further hop. Encoded +`JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES`. + +- [ ] Steps per reporter (three sub-cycles, ONE commit each): failing exact-string + test (error case, warning case, empty case) → red → implement → green + check → + commits `feat(#5): github actions reporter`, `feat(#5): checkstyle reporter`, + `feat(#5): sarif reporter`. + +### Task 7: Suppression + +**Files:** create `src/Ignore/{TrampIgnore,SuppressionIndex}.php`; modify +`src/Index/IndexingVisitor.php`, `src/Index/MethodIndex.php` (constructor gains +`SuppressionIndex $suppressions = new SuppressionIndex([], [], [])` — additive, +existing call sites keep working — plus `suppressions(): SuppressionIndex`), +`src/Chain/ChainTraversal.php`; tests `tests/Ignore/SuppressionIndexTest.php`, +extend `ChainBuilderTest`. + +**Produces:** + +```php +#[\Attribute(\Attribute::TARGET_CLASS | \Attribute::TARGET_METHOD | \Attribute::TARGET_FUNCTION | \Attribute::TARGET_PARAMETER)] +final class TrampIgnore +{ +} + +final class SuppressionIndex +{ + /** + * @param list $methods suppressed method FQMNs (method- or class-level attribute) + * @param list $params [fqmn, paramName] pairs + * @param array> $lines file -> lines carrying an ignore comment + */ + public function __construct(...); + + public function suppressesMethod(string $fqmn): bool; + public function suppressesParam(string $fqmn, string $param): bool; + public function suppressesLine(string $file, int $line): bool; +} +``` + +Semantics (frozen rule 11, made concrete): + +- **Attribute matching is by short name:** any attribute whose name's last + segment is `TrampIgnore` counts — analyzed codebases must not be forced to + autoload our exact class. Class-level attribute suppresses every method of the + class; method-/function-level every param of the method; param-level that param. +- **Comment form:** a `// phptramp-ignore` line comment (anywhere in the line, + e.g. after code) suppresses when it sits on the method's declaration line, the + line directly above it (doc-comment style), or on a forwarding call-site line + (suppressing chains through that specific edge). +- **Chain filter (in `ChainTraversal`):** a finding is dropped when ANY of its + hop nodes is suppressed — method-suppressed, param-suppressed for the chain's + param at that hop, or the hop's `forwardLine`/declaration line is + comment-suppressed. Terminal nodes never suppress (they are not hops). +- Stale-suppression *detection* is Phase 5 — do not build it here. + +- [ ] **Step 1:** failing tests — `SuppressionIndexTest` for the three lookups; + `ChainBuilderTest`: attribute on middle hop kills the chain; attribute on the + param only kills chains of that param; class-level kills all chains through the + class; ignore comment on the forwarding line kills the chain; comment on an + unrelated line does not; the README 3-hop chain without suppressions is still + found (regression). +- [ ] **Step 2:** red. **Step 3:** implement (visitor collects during the existing + traversal — attributes from `attrGroups`, comments via `getComments()` + + line scan). **Step 4:** green + check. +- [ ] **Step 5:** commit `feat(#5): TrampIgnore attribute and comment suppression`. + +### Task 8: `SummaryReporter` + +**Files:** create `src/Report/SummaryReporter.php`; register in factory; +test `tests/Report/SummaryReporterTest.php`. + +Summary is a whole-codebase overview: it renders **all** findings regardless of +thresholds (the point is triaging a legacy codebase before choosing a limit). +The exit code still follows the normal error rule. Contract (pin): + +``` +Chains by length: + 1 hop ######## 8 + 2 hops #### 4 + 3 hops ## 2 + +Top 10 longest chains: + 3 hops $config Demo\Controller::handle -> Demo\Mailer::__construct [stored] + 3 hops $request Demo\Kernel::boot -> external + +Most-forwarded parameters: + 6x $config + 3x $request + +14 chains total; 2 at or over the limit (limit: 3 hops). +``` + +Bars scale to the widest bucket = 40 `#` max; counts sorted descending, ties +alphabetical; top-10 ties broken by origin FQMN. + +- [ ] Steps: failing exact-string test (mixed case + empty case: + `No chains found.`) → red → implement → green + check → + commit `feat(#5): summary report`. + +### Task 9: End-to-end fixtures, docs, phase close-out + +**Files:** new fixture dirs; modify `README.md`, `docs/plan.md` (status), +`src/Console/Application.php` help text only if wording drifted. + +- **Fixtures:** `warn-vs-error` (chains of 2 and 3 hops; `expected-findings.json` + run with `--limit 3 --warn-limit 2` — extend the harness to read optional + per-fixture CLI args from a `phptramp-args.json` next to the expectation + file); `suppressed-chain` (attribute + comment variants, expects zero + findings); `config-driven` (a `phptramp.json` inside the fixture exercising + `paths` + `exclude` + `limit`). +- **README:** status → Phase 3; document config file, formats (one-line example + each), suppression, `--warn-limit`, and the `composer tramp` script again + against real behavior. +- **docs/plan.md:** check off Phase 3 bullets, mark the phase ✅. + +- [ ] Steps: harness extension + fixtures test-first → green + check → + commit `test(#5): format, config and suppression fixtures` → docs → + commit `docs(#5): phase 3 status and usage docs`. + +--- + +## Done when + +1. Every task committed on `feature/5-ci-config-surface`; `composer check` green + at every commit; `composer infection:diff` ≥ 80 MSI locally (`git fetch origin + main` first). +2. All Phase 3 bullets in `docs/plan.md` checked off; README/help text match + actual behavior; every reporter contract has an exact-string test AND a + fixture. +3. PR into `develop` with `Closes #5`; CI (matrix + static + mutation) green; + after merge, verify issue #5 auto-closed. diff --git a/src/Chain/ChainTraversal.php b/src/Chain/ChainTraversal.php index 934d60c..f96f121 100644 --- a/src/Chain/ChainTraversal.php +++ b/src/Chain/ChainTraversal.php @@ -4,6 +4,7 @@ namespace PhpTramp\Chain; +use PhpTramp\Ignore\SuppressionIndex; use PhpTramp\Index\ForwardSite; use PhpTramp\Index\MethodIndex; use PhpTramp\Index\MethodInfo; @@ -35,10 +36,13 @@ final class ChainTraversal /** @var array */ private array $visited = []; + private readonly SuppressionIndex $suppressions; + public function __construct( private readonly MethodIndex $index, private readonly CallResolver $resolver, ) { + $this->suppressions = $index->suppressions(); } /** @@ -182,6 +186,10 @@ private function terminalKind(ParamInfo $param): TerminalKind private function record(PartialChain $chain, Terminal $terminal): void { + if ($this->isSuppressedChain($chain)) { + return; + } + $fullChain = $terminal->hop === null ? $chain->hops : [...$chain->hops, $terminal->hop]; $this->findings[] = new Finding( @@ -197,6 +205,34 @@ private function record(PartialChain $chain, Terminal $terminal): void ); } + private function isSuppressedChain(PartialChain $chain): bool + { + foreach ($chain->hops as $index => $hop) { + [$fqmn, $param] = explode(self::KEY_SEPARATOR, $chain->keys[$index], 2); + if ($this->isSuppressedHop($hop, $fqmn, $param)) { + return true; + } + } + + return false; + } + + private function isSuppressedHop(Hop $hop, string $fqmn, string $param): bool + { + if ($this->suppressions->suppressesMethod($fqmn) || $this->suppressions->suppressesParam($fqmn, $param)) { + return true; + } + + if ( + $this->suppressions->suppressesLine($hop->file, $hop->line) + || $this->suppressions->suppressesLine($hop->file, $hop->line - 1) + ) { + return true; + } + + return $hop->forwardLine !== null && $this->suppressions->suppressesLine($hop->file, $hop->forwardLine); + } + /** * @param list $chain */ diff --git a/src/Config/ConfigException.php b/src/Config/ConfigException.php new file mode 100644 index 0000000..19e65ef --- /dev/null +++ b/src/Config/ConfigException.php @@ -0,0 +1,9 @@ +resolveFile($directory); + if ($file === null) { + return new Options(); + } + + return $this->toOptions($this->decode($file)); + } + + private function resolveFile(string $directory): ?string + { + $primary = $directory . '/phptramp.json'; + if (is_file($primary)) { + return $primary; + } + + $dist = $directory . '/phptramp.dist.json'; + + return is_file($dist) ? $dist : null; + } + + /** + * @return array + */ + private function decode(string $file): array + { + $contents = file_get_contents($file); + if ($contents === false) { + throw new ConfigException("unable to read config file: {$file}"); + } + + $decoded = json_decode($contents); + if (json_last_error() !== JSON_ERROR_NONE || ! $decoded instanceof \stdClass) { + throw new ConfigException("config file is not a JSON object: {$file}"); + } + + return $this->stringKeyed(get_object_vars($decoded)); + } + + /** + * Narrows get_object_vars()'s array to array: + * JSON object properties always decode to string keys, but PHPStan can't + * see that through stdClass alone. + * + * @param array $properties + * @return array + */ + private function stringKeyed(array $properties): array + { + $result = []; + foreach ($properties as $key => $value) { + $result[(string) $key] = $value; + } + + return $result; + } + + /** + * @param array $config + */ + private function toOptions(array $config): Options + { + $defaults = new Options(); + + $folders = []; + $files = []; + $exclude = $defaults->exclude; + $limit = $defaults->limit; + $warnLimit = $defaults->warnLimit; + $format = $defaults->format; + $baseline = $defaults->baseline; + + foreach ($config as $key => $value) { + match ($key) { + 'paths' => $this->assignPaths($value, $folders, $files), + 'exclude' => $exclude = $this->requireStringList('exclude', $value), + 'limit' => $limit = $this->requireInt('limit', $value), + 'warnLimit' => $warnLimit = $this->requireInt('warnLimit', $value), + 'format' => $format = $this->requireString('format', $value), + 'baseline' => $baseline = $this->requireString('baseline', $value), + default => throw new ConfigException("unknown config key: {$key}"), + }; + } + + return new Options( + folders: $folders, + files: $files, + limit: $limit, + warnLimit: $warnLimit, + format: $format, + baseline: $baseline, + exclude: $exclude, + ); + } + + /** + * @param list $folders + * @param list $files + * @param-out list $folders + * @param-out list $files + */ + private function assignPaths(mixed $value, array &$folders, array &$files): void + { + foreach ($this->requireStringList('paths', $value) as $path) { + if (str_ends_with($path, '.php')) { + $files[] = $path; + } else { + $folders[] = $path; + } + } + } + + /** + * @return list + */ + private function requireStringList(string $key, mixed $value): array + { + if (! is_array($value) || ! array_is_list($value)) { + throw new ConfigException("config key \"{$key}\" must be a list of strings"); + } + + $strings = []; + foreach ($value as $entry) { + if (! is_string($entry)) { + throw new ConfigException("config key \"{$key}\" must be a list of strings"); + } + $strings[] = $entry; + } + + return $strings; + } + + private function requireInt(string $key, mixed $value): int + { + if (! is_int($value)) { + throw new ConfigException("config key \"{$key}\" must be an integer"); + } + + return $value; + } + + private function requireString(string $key, mixed $value): string + { + if (! is_string($value)) { + throw new ConfigException("config key \"{$key}\" must be a string"); + } + + return $value; + } +} diff --git a/src/Console/Application.php b/src/Console/Application.php index b187004..ef34ea5 100644 --- a/src/Console/Application.php +++ b/src/Console/Application.php @@ -6,13 +6,17 @@ use PhpTramp\Chain\ChainBuilder; use PhpTramp\Chain\Finding; +use PhpTramp\Config\ConfigException; +use PhpTramp\Config\ConfigLoader; use PhpTramp\Discovery\FileLocator; use PhpTramp\Index\ForwardSite; use PhpTramp\Index\Indexer; use PhpTramp\Index\MethodIndex; use PhpTramp\Index\ParamInfo; use PhpTramp\Index\ParseException; -use PhpTramp\Report\TextReporter; +use PhpTramp\Report\ReporterFactory; +use PhpTramp\Report\Severity; +use PhpTramp\Report\Thresholds; use PhpTramp\Resolve\CallResolver; use PhpTramp\Resolve\ClassHierarchy; @@ -45,8 +49,9 @@ public function run(array $argv): int $args = array_slice($argv, 1); try { - $options = (new ArgvParser())->parse($args); - } catch (InvalidArgsException $e) { + $defaults = (new ConfigLoader())->load($this->workingDirectory()); + $options = (new ArgvParser())->parse($args, $defaults); + } catch (ConfigException | InvalidArgsException $e) { fwrite($this->stderr, 'phptramp: ' . $e->getMessage() . "\n"); return 2; @@ -71,31 +76,46 @@ public function run(array $argv): int return $this->analyze($options); } - private function analyze(Options $options): int + private function workingDirectory(): string { - if ($options->format !== 'text') { - fwrite($this->stderr, "phptramp: format '{$options->format}' is not implemented until Phase 3.\n"); - - return 2; - } + return getcwd() ?: '.'; + } + private function analyze(Options $options): int + { try { + $thresholds = new Thresholds($options->limit, $options->warnLimit); $index = $this->buildIndex($options); + $reporter = (new ReporterFactory($this->workingDirectory()))->create($options); } catch (InvalidArgsException | ParseException $e) { fwrite($this->stderr, 'phptramp: ' . $e->getMessage() . "\n"); return 2; } - $findings = $this->findChains($index, $options->limit); - fwrite($this->stdout, (new TextReporter($options->limit, $options->explain))->render($findings)); + $findings = $this->findChains($index); + fwrite($this->stdout, $reporter->render($findings)); + + return $this->hasError($findings, $thresholds) ? 1 : 0; + } + + /** + * @param list $findings + */ + private function hasError(array $findings, Thresholds $thresholds): bool + { + foreach ($findings as $finding) { + if ($thresholds->severityOf($finding) === Severity::Error) { + return true; + } + } - return $findings === [] ? 0 : 1; + return false; } private function buildIndex(Options $options): MethodIndex { - $files = (new FileLocator())->locate($options); + $files = (new FileLocator($this->workingDirectory()))->locate($options); return (new Indexer())->index($files); } @@ -103,18 +123,17 @@ private function buildIndex(Options $options): MethodIndex /** * @return list */ - private function findChains(MethodIndex $index, int $limit): array + private function findChains(MethodIndex $index): array { $resolver = new CallResolver($index, new ClassHierarchy($index)); - $findings = (new ChainBuilder($resolver))->build($index); - return array_values(array_filter($findings, static fn (Finding $finding): bool => $finding->hops >= $limit)); + return (new ChainBuilder($resolver))->build($index); } private function dumpIndex(Options $options): int { try { - $files = (new FileLocator())->locate($options); + $files = (new FileLocator($this->workingDirectory()))->locate($options); $index = (new Indexer())->index($files); } catch (InvalidArgsException | ParseException $e) { fwrite($this->stderr, 'phptramp: ' . $e->getMessage() . "\n"); @@ -201,8 +220,10 @@ private static function helpText(): string 1 at least one finding at or over --limit 2 tool error (bad arguments, parse failure, ...) - Status: Phase 2 - cross-file chain reporting works in the text format; - other formats land in Phase 3. See docs/plan.md. + Status: Phase 3 - cross-file chain reporting works 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. TXT; } diff --git a/src/Console/ArgvParser.php b/src/Console/ArgvParser.php index 54d3af0..24d234c 100644 --- a/src/Console/ArgvParser.php +++ b/src/Console/ArgvParser.php @@ -50,13 +50,16 @@ final class ArgvParser private bool $dumpIndex = false; private bool $help = false; private bool $version = false; + private bool $clearedSeededPaths = false; + /** @var list */ + private array $exclude = []; /** * @param list $args */ - public function parse(array $args): Options + public function parse(array $args, Options $defaults = new Options()): Options { - $this->reset(); + $this->reset($defaults); $index = 0; $count = count($args); @@ -78,24 +81,27 @@ public function parse(array $args): Options dumpIndex: $this->dumpIndex, help: $this->help, version: $this->version, + exclude: $this->exclude, ); } - private function reset(): void + private function reset(Options $defaults): void { - $this->folders = []; - $this->files = []; - $this->limit = 3; - $this->warnLimit = null; - $this->format = 'text'; - $this->explain = false; - $this->changedOnly = false; - $this->gitBase = 'origin/main'; - $this->baseline = null; - $this->generateBaseline = null; - $this->dumpIndex = false; - $this->help = false; - $this->version = false; + $this->folders = $defaults->folders; + $this->files = $defaults->files; + $this->limit = $defaults->limit; + $this->warnLimit = $defaults->warnLimit; + $this->format = $defaults->format; + $this->explain = $defaults->explain; + $this->changedOnly = $defaults->changedOnly; + $this->gitBase = $defaults->gitBase; + $this->baseline = $defaults->baseline; + $this->generateBaseline = $defaults->generateBaseline; + $this->dumpIndex = $defaults->dumpIndex; + $this->help = $defaults->help; + $this->version = $defaults->version; + $this->exclude = $defaults->exclude; + $this->clearedSeededPaths = false; } /** @@ -134,8 +140,8 @@ private function consume(array $args, int $index, int $count): int private function applyValueFlag(string $name, string $value): void { match ($name) { - '--folder' => $this->folders[] = $value, - '--file' => $this->files[] = $value, + '--folder' => $this->appendFolder($value), + '--file' => $this->appendFile($value), '--files' => $this->appendFiles($value), '--limit' => $this->limit = $this->toInt($name, $value), '--warn-limit' => $this->warnLimit = $this->toInt($name, $value), @@ -159,13 +165,41 @@ private function applyBoolFlag(string $name): void }; } + private function appendFolder(string $value): void + { + $this->clearSeededPathsOnce(); + $this->folders[] = $value; + } + + private function appendFile(string $value): void + { + $this->clearSeededPathsOnce(); + $this->files[] = $value; + } + private function appendFiles(string $value): void { + $this->clearSeededPathsOnce(); foreach (explode(',', $value) as $file) { $this->files[] = $file; } } + /** + * The first path flag replaces config-seeded paths entirely (never mixed + * with CLI paths); later path flags append as usual. + */ + private function clearSeededPathsOnce(): void + { + if ($this->clearedSeededPaths) { + return; + } + + $this->folders = []; + $this->files = []; + $this->clearedSeededPaths = true; + } + private function toInt(string $flag, string $value): int { if ($value === '' || ! ctype_digit($value)) { diff --git a/src/Console/Options.php b/src/Console/Options.php index a4b44c6..ece4ba9 100644 --- a/src/Console/Options.php +++ b/src/Console/Options.php @@ -17,6 +17,7 @@ final class Options /** * @param list $folders * @param list $files + * @param list $exclude */ public function __construct( public readonly array $folders = [], @@ -32,6 +33,7 @@ public function __construct( public readonly bool $dumpIndex = false, public readonly bool $help = false, public readonly bool $version = false, + public readonly array $exclude = [], ) { } } diff --git a/src/Discovery/FileLocator.php b/src/Discovery/FileLocator.php index 20dff75..8bd6764 100644 --- a/src/Discovery/FileLocator.php +++ b/src/Discovery/FileLocator.php @@ -13,18 +13,24 @@ */ final class FileLocator { + public function __construct(private readonly string $workingDirectory) + { + } + /** * @return list */ public function locate(Options $options): array { - /** @var array $found */ - $found = []; + /** @var array $discovered */ + $discovered = []; foreach ($options->folders as $folder) { - $this->collectFolder($folder, $found); + $this->collectFolder($folder, $discovered); } + $found = $this->withoutExcluded($discovered, $options->exclude); + foreach ($options->files as $file) { $real = realpath($file); if ($real === false || ! is_file($real)) { @@ -39,6 +45,57 @@ public function locate(Options $options): array return $paths; } + /** + * @param array $discovered + * @param list $exclude + * @return array + */ + private function withoutExcluded(array $discovered, array $exclude): array + { + if ($exclude === []) { + return $discovered; + } + + $base = realpath($this->workingDirectory); + + $kept = []; + foreach ($discovered as $path => $present) { + if (! $this->matchesAny($this->relativeTo($base, $path), $exclude)) { + $kept[$path] = $present; + } + } + + return $kept; + } + + /** + * @param list $exclude + */ + private function matchesAny(string $relative, array $exclude): bool + { + foreach ($exclude as $pattern) { + if (fnmatch($pattern, $relative)) { + return true; + } + } + + return false; + } + + private function relativeTo(string|false $base, string $path): string + { + if ($base === false) { + return $path; + } + + $prefix = $base . '/'; + if (! str_starts_with($path, $prefix)) { + return $path; + } + + return substr($path, strlen($prefix)); + } + /** * @param array $found * @param-out array $found diff --git a/src/Ignore/SuppressionIndex.php b/src/Ignore/SuppressionIndex.php new file mode 100644 index 0000000..1283b89 --- /dev/null +++ b/src/Ignore/SuppressionIndex.php @@ -0,0 +1,81 @@ + */ + private readonly array $methods; + + /** @var array */ + private readonly array $params; + + /** @var array> */ + private readonly array $lines; + + /** + * @param list $methods suppressed method FQMNs (method- or class-level attribute) + * @param list $params [fqmn, paramName] pairs + * @param array> $lines file -> lines carrying an ignore comment + */ + public function __construct(array $methods, array $params, array $lines) + { + $this->methods = array_fill_keys($methods, true); + $this->params = $this->indexParams($params); + $this->lines = $this->indexLines($lines); + } + + public function suppressesMethod(string $fqmn): bool + { + return isset($this->methods[$fqmn]); + } + + public function suppressesParam(string $fqmn, string $param): bool + { + return isset($this->params[$fqmn . self::KEY_SEPARATOR . $param]); + } + + public function suppressesLine(string $file, int $line): bool + { + return isset($this->lines[$file][$line]); + } + + /** + * @param list $params + * @return array + */ + private function indexParams(array $params): array + { + $index = []; + foreach ($params as [$fqmn, $param]) { + $index[$fqmn . self::KEY_SEPARATOR . $param] = true; + } + + return $index; + } + + /** + * @param array> $lines + * @return array> + */ + private function indexLines(array $lines): array + { + $index = []; + foreach ($lines as $file => $lineNumbers) { + $index[$file] = array_fill_keys($lineNumbers, true); + } + + return $index; + } +} diff --git a/src/Ignore/TrampIgnore.php b/src/Ignore/TrampIgnore.php new file mode 100644 index 0000000..6d8af69 --- /dev/null +++ b/src/Ignore/TrampIgnore.php @@ -0,0 +1,20 @@ +classifyPending($visitor->pending()), $visitor->classes()); + return new MethodIndex( + $this->classifyPending($visitor->pending()), + $visitor->classes(), + $visitor->suppressions(), + ); } /** @@ -95,6 +99,7 @@ private function indexFile( } $visitor->setFile($file); + $visitor->recordIgnoreComments($code); $traverser->traverse($ast); return null; diff --git a/src/Index/IndexingVisitor.php b/src/Index/IndexingVisitor.php index 98b8e1a..4dcb866 100644 --- a/src/Index/IndexingVisitor.php +++ b/src/Index/IndexingVisitor.php @@ -16,6 +16,7 @@ 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 @@ -39,6 +40,11 @@ final class IndexingVisitor extends NodeVisitorAbstract private string $file = ''; + public function __construct( + private readonly SuppressionCollector $suppressionCollector = new SuppressionCollector(), + ) { + } + public function setFile(string $file): void { $this->file = $file; @@ -67,6 +73,16 @@ public function pending(): array return $this->pending; } + public function recordIgnoreComments(string $code): void + { + $this->suppressionCollector->recordIgnoreComments($this->file, $code); + } + + public function suppressions(): SuppressionIndex + { + return $this->suppressionCollector->suppressions($this->pending); + } + /** * @return array */ @@ -111,6 +127,8 @@ private function recordClass(ClassLike $node): void 'interfaces' => $interfaces, 'traits' => [], ]; + + $this->suppressionCollector->recordClassAttributes($fqcn, $node->attrGroups); } private function kindOf(ClassLike $node): ClassKind @@ -143,13 +161,16 @@ private function recordMethod(ClassMethod $node): void return; } + $fqmn = $fqcn . '::' . $node->name->toString(); $this->pending[] = [ - 'fqmn' => $fqcn . '::' . $node->name->toString(), + 'fqmn' => $fqmn, 'file' => $this->file, 'line' => $node->getStartLine(), 'class' => $fqcn, 'node' => $node, ]; + + $this->suppressionCollector->recordFunctionLikeAttributes($fqmn, $node); } private function recordFunction(Function_ $node): void @@ -166,6 +187,8 @@ private function recordFunction(Function_ $node): void 'class' => null, 'node' => $node, ]; + + $this->suppressionCollector->recordFunctionLikeAttributes($fqmn, $node); } private function enclosingClassName(Node $node): ?string diff --git a/src/Index/MethodIndex.php b/src/Index/MethodIndex.php index 49086ab..44bb392 100644 --- a/src/Index/MethodIndex.php +++ b/src/Index/MethodIndex.php @@ -4,9 +4,12 @@ namespace PhpTramp\Index; +use PhpTramp\Ignore\SuppressionIndex; + /** * The whole-project index: fully-qualified method name -> MethodInfo, plus - * per-class hierarchy facts. Immutable; produced by {@see Indexer}. + * per-class hierarchy facts and collected suppressions. Immutable; produced by + * {@see Indexer}. */ final class MethodIndex { @@ -17,9 +20,15 @@ final class MethodIndex public function __construct( private readonly array $methods, private readonly array $classes = [], + private readonly SuppressionIndex $suppressions = new SuppressionIndex([], [], []), ) { } + public function suppressions(): SuppressionIndex + { + return $this->suppressions; + } + public function get(string $fqmn): ?MethodInfo { return $this->methods[$fqmn] ?? null; diff --git a/src/Index/SuppressionCollector.php b/src/Index/SuppressionCollector.php new file mode 100644 index 0000000..6e38872 --- /dev/null +++ b/src/Index/SuppressionCollector.php @@ -0,0 +1,137 @@ + */ + private array $suppressedMethods = []; + + /** @var list */ + private array $suppressedClasses = []; + + /** @var list */ + private array $suppressedParams = []; + + /** @var array> */ + private array $ignoreLines = []; + + /** + * @param array $attrGroups + */ + public function recordClassAttributes(string $fqcn, array $attrGroups): void + { + if ($this->hasIgnoreAttribute($attrGroups)) { + $this->suppressedClasses[] = $fqcn; + } + } + + public function recordFunctionLikeAttributes(string $fqmn, FunctionLike $node): void + { + if ($this->hasIgnoreAttribute($node->getAttrGroups())) { + $this->suppressedMethods[] = $fqmn; + } + + foreach ($node->getParams() as $param) { + if ($this->hasIgnoreAttribute($param->attrGroups)) { + $this->recordSuppressedParam($fqmn, $param); + } + } + } + + public function recordIgnoreComments(string $file, string $code): void + { + foreach (token_get_all($code) as $token) { + if (is_array($token) && $this->isIgnoreCommentToken($token)) { + $this->ignoreLines[$file][] = $token[2]; + } + } + } + + /** + * @param list $pending + */ + public function suppressions(array $pending): SuppressionIndex + { + return new SuppressionIndex( + $this->expandClassSuppressions($pending), + $this->suppressedParams, + $this->ignoreLines, + ); + } + + /** + * @param list $pending + * @return list + */ + private function expandClassSuppressions(array $pending): array + { + $methods = $this->suppressedMethods; + foreach ($pending as $entry) { + if ($entry['class'] !== null && in_array($entry['class'], $this->suppressedClasses, true)) { + $methods[] = $entry['fqmn']; + } + } + + return $methods; + } + + private function recordSuppressedParam(string $fqmn, Param $param): void + { + if (! $param->var instanceof Variable || ! is_string($param->var->name)) { + return; + } + + $this->suppressedParams[] = [$fqmn, $param->var->name]; + } + + /** + * @param array $attrGroups + */ + private function hasIgnoreAttribute(array $attrGroups): bool + { + foreach ($attrGroups as $group) { + foreach ($group->attrs as $attribute) { + if ($attribute->name->getLast() === self::IGNORE_ATTRIBUTE_NAME) { + return true; + } + } + } + + return false; + } + + /** + * @param array{0: int, 1: string, 2: int} $token + */ + private function isIgnoreCommentToken(array $token): bool + { + [$id, $text] = $token; + + return ($id === T_COMMENT || $id === T_DOC_COMMENT) && str_contains($text, self::IGNORE_COMMENT_MARKER); + } +} diff --git a/src/Report/CheckstyleReporter.php b/src/Report/CheckstyleReporter.php new file mode 100644 index 0000000..1571a33 --- /dev/null +++ b/src/Report/CheckstyleReporter.php @@ -0,0 +1,92 @@ +` per finding at + * its origin line, grouped into `` elements by origin file. Consumed by + * CI tooling that already understands the Checkstyle format (GitLab Code + * Quality, Jenkins Warnings NG, ...). + */ +final class CheckstyleReporter implements Reporter +{ + private readonly FindingMessage $message; + + public function __construct( + private readonly Thresholds $thresholds, + private readonly Paths $paths, + ) { + $this->message = new FindingMessage(); + } + + /** + * @param list $findings ALL findings, unfiltered + */ + public function render(array $findings): string + { + $lines = ['', '']; + foreach ($this->groupByOriginFile($findings) as $file => $fileFindings) { + foreach ($this->fileBlock($file, $fileFindings) as $line) { + $lines[] = $line; + } + } + $lines[] = ''; + + return implode("\n", $lines) . "\n"; + } + + /** + * @param list $fileFindings + * @return list + */ + private function fileBlock(string $file, array $fileFindings): array + { + $lines = [' ']; + foreach ($fileFindings as [$finding, $severity]) { + $lines[] = $this->errorLine($finding, $severity); + } + $lines[] = ' '; + + return $lines; + } + + private function errorLine(Finding $finding, Severity $severity): string + { + $origin = $finding->chain[0]; + + return ' '; + } + + /** + * Groups reportable findings by relativized origin file, in first-seen + * file order; findings within a file keep their input order. + * + * @param list $findings + * @return array> + */ + private function groupByOriginFile(array $findings): array + { + $groups = []; + foreach ($findings as $finding) { + $severity = $this->thresholds->severityOf($finding); + if ($severity === null) { + continue; + } + $file = $this->paths->relativize($finding->chain[0]->file); + $groups[$file][] = [$finding, $severity]; + } + + return $groups; + } + + private function escape(string $value): string + { + return htmlspecialchars($value, ENT_XML1 | ENT_QUOTES, 'UTF-8'); + } +} diff --git a/src/Report/FindingMessage.php b/src/Report/FindingMessage.php new file mode 100644 index 0000000..df4bbbe --- /dev/null +++ b/src/Report/FindingMessage.php @@ -0,0 +1,40 @@ +pluralizer = new Pluralizer(); + } + + public function describe(Finding $finding): string + { + return '$' . $finding->param . ': ' . $finding->hops . ' pass-through ' + . $this->pluralizer->of($finding->hops, 'hop') + . ' across ' . $finding->classes . ' ' . $this->pluralizer->of($finding->classes, 'class', 'classes') + . ' (terminal: ' . $this->terminalClause($finding) . ')'; + } + + /** Shared with {@see SummaryReporter}'s "Top 10 longest chains" column. */ + public function terminalClause(Finding $finding): string + { + if ($finding->terminal === null) { + return $finding->terminalKind->value; + } + + return $finding->terminal . ' [' . $finding->terminalKind->value . ']'; + } +} diff --git a/src/Report/GithubReporter.php b/src/Report/GithubReporter.php new file mode 100644 index 0000000..d7f8d74 --- /dev/null +++ b/src/Report/GithubReporter.php @@ -0,0 +1,113 @@ +message = new FindingMessage(); + } + + /** + * @param list $findings ALL findings, unfiltered + */ + public function render(array $findings): string + { + $lines = []; + foreach ($findings as $finding) { + $severity = $this->thresholds->severityOf($finding); + if ($severity === null) { + continue; + } + foreach ($this->annotationLines($finding, $severity) as $line) { + $lines[] = $line; + } + } + + if ($lines === []) { + return ''; + } + + return implode("\n", $lines) . "\n"; + } + + /** + * @return list + */ + private function annotationLines(Finding $finding, Severity $severity): array + { + $lines = [$this->originAnnotation($finding, $severity)]; + foreach ($this->subsequentHops($finding) as $index => $hop) { + $lines[] = $this->hopNotice($finding, $hop, $index); + } + + return $lines; + } + + private function originAnnotation(Finding $finding, Severity $severity): string + { + $origin = $finding->chain[0]; + $title = 'phptramp::' . $this->message->describe($finding); + + return '::' . $severity->label() + . ' file=' . $this->escapeProperty($this->paths->relativize($origin->file)) + . ',line=' . $origin->line + . ',title=' . $this->escapeProperty($title); + } + + private function hopNotice(Finding $finding, Hop $hop, int $index): string + { + $title = 'phptramp::hop ' . ($index + 1) . ' of $' . $finding->param . ' chain from ' . $finding->origin; + + return '::notice file=' . $this->escapeProperty($this->paths->relativize($hop->file)) + . ',line=' . $hop->line + . ',title=' . $this->escapeProperty($title); + } + + /** + * @return array subsequent-hop entries keyed by their chain index + * (1 .. hops-1) — the terminal node, at index + * hops, is never included + */ + private function subsequentHops(Finding $finding): array + { + return array_slice($finding->chain, 1, $finding->hops - 1, true); + } + + /** + * Escapes a workflow-command PROPERTY value (a `file=`/`title=` component), + * per `@actions/core`'s `escapeProperty`: the same %/\r/\n data-escaping as + * a command's message body, plus `:` and `,` — the two characters that + * would otherwise be misread as property separators. `strtr` replaces every + * character in a single left-to-right pass, so the `%NN` sequences it + * introduces are never themselves re-escaped. + */ + private function escapeProperty(string $value): string + { + return strtr($value, [ + '%' => '%25', + ':' => '%3A', + ',' => '%2C', + "\r" => '%0D', + "\n" => '%0A', + ]); + } +} diff --git a/src/Report/JsonEncoder.php b/src/Report/JsonEncoder.php new file mode 100644 index 0000000..0592f0c --- /dev/null +++ b/src/Report/JsonEncoder.php @@ -0,0 +1,30 @@ + $document + * @throws JsonEncodingException + */ + public function encode(array $document, string $subject): string + { + $json = json_encode($document, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES); + if ($json === false) { + // Unreachable: $document is built entirely from scalars, strings, and + // arrays thereof — nothing here can produce a resource or invalid + // UTF-8 that would make json_encode fail. + throw new JsonEncodingException("Failed to encode findings as {$subject}."); + } + + return $json; + } +} diff --git a/src/Report/JsonEncodingException.php b/src/Report/JsonEncodingException.php new file mode 100644 index 0000000..82ef887 --- /dev/null +++ b/src/Report/JsonEncodingException.php @@ -0,0 +1,9 @@ +encoder = new JsonEncoder(); + } + + /** + * @param list $findings ALL findings, unfiltered + */ + public function render(array $findings): string + { + $document = [ + 'limit' => $this->thresholds->limit, + 'warnLimit' => $this->thresholds->warnLimit, + 'findings' => $this->findingDocuments($findings), + ]; + + return $this->encoder->encode($document, 'JSON') . "\n"; + } + + /** + * @param list $findings + * @return list> + */ + private function findingDocuments(array $findings): array + { + $documents = []; + foreach ($findings as $finding) { + $severity = $this->thresholds->severityOf($finding); + if ($severity === null) { + continue; + } + $documents[] = $this->findingDocument($finding, $severity); + } + + return $documents; + } + + /** + * @return array + */ + private function findingDocument(Finding $finding, Severity $severity): array + { + return [ + 'param' => $finding->param, + 'severity' => $severity->label(), + 'origin' => $finding->origin, + 'terminal' => $finding->terminal, + 'terminalKind' => $finding->terminalKind->value, + 'hops' => $finding->hops, + 'classes' => $finding->classes, + 'chain' => $this->chainDocument($finding), + 'notes' => $finding->notes, + ]; + } + + /** + * @return list> + */ + private function chainDocument(Finding $finding): array + { + $hasTerminalNode = count($finding->chain) > $finding->hops; + + $chain = []; + foreach ($finding->chain as $index => $hop) { + $isTerminal = $hasTerminalNode && $index === $finding->hops; + $chain[] = [ + 'method' => $hop->fqmn, + 'role' => $this->role($index, $isTerminal), + 'file' => $this->paths->relativize($hop->file), + 'line' => $hop->line, + 'forwardLine' => $hop->forwardLine, + ]; + } + + return $chain; + } + + private function role(int $index, bool $isTerminal): string + { + if ($isTerminal) { + return 'terminal'; + } + if ($index === 0) { + return 'origin'; + } + + return 'hop'; + } +} diff --git a/src/Report/Paths.php b/src/Report/Paths.php new file mode 100644 index 0000000..f134b57 --- /dev/null +++ b/src/Report/Paths.php @@ -0,0 +1,34 @@ +workingDirectory = rtrim($workingDirectory, '/'); + } + + public function relativize(string $absolutePath): string + { + if ($absolutePath === $this->workingDirectory) { + return ''; + } + + $prefix = $this->workingDirectory . '/'; + if (str_starts_with($absolutePath, $prefix)) { + return substr($absolutePath, strlen($prefix)); + } + + return $absolutePath; + } +} diff --git a/src/Report/Pluralizer.php b/src/Report/Pluralizer.php new file mode 100644 index 0000000..0ade64a --- /dev/null +++ b/src/Report/Pluralizer.php @@ -0,0 +1,22 @@ + $findings ALL findings, unfiltered — each reporter + * applies its Thresholds itself + */ + public function render(array $findings): string; +} diff --git a/src/Report/ReporterFactory.php b/src/Report/ReporterFactory.php new file mode 100644 index 0000000..05dc2fc --- /dev/null +++ b/src/Report/ReporterFactory.php @@ -0,0 +1,32 @@ +limit, $options->warnLimit); + + return match ($options->format) { + 'text' => new TextReporter($thresholds, new Paths($this->workingDirectory), $options->explain), + 'json' => new JsonReporter($thresholds, new Paths($this->workingDirectory)), + 'github' => new GithubReporter($thresholds, new Paths($this->workingDirectory)), + 'checkstyle' => new CheckstyleReporter($thresholds, new Paths($this->workingDirectory)), + 'sarif' => new SarifReporter($thresholds, new Paths($this->workingDirectory)), + 'summary' => new SummaryReporter($thresholds), + // Unreachable: ArgvParser::validateFormat rejects any format outside + // VALID_FORMATS at parse time, so every case above is exhaustive. + default => throw new InvalidArgsException("format '{$options->format}' is not implemented yet."), + }; + } +} diff --git a/src/Report/SarifReporter.php b/src/Report/SarifReporter.php new file mode 100644 index 0000000..97a448e --- /dev/null +++ b/src/Report/SarifReporter.php @@ -0,0 +1,137 @@ +message = new FindingMessage(); + $this->encoder = new JsonEncoder(); + } + + /** + * @param list $findings ALL findings, unfiltered + */ + public function render(array $findings): string + { + $document = [ + 'version' => '2.1.0', + '$schema' => self::SCHEMA_URL, + 'runs' => [$this->run($findings)], + ]; + + return $this->encoder->encode($document, 'SARIF') . "\n"; + } + + /** + * @param list $findings + * @return array + */ + private function run(array $findings): array + { + return [ + 'tool' => [ + 'driver' => [ + 'name' => 'phptramp', + 'informationUri' => 'https://github.com/larspohlmann/phptramp', + 'rules' => [ + ['id' => 'phptramp.trampData'], + ], + ], + ], + 'results' => $this->results($findings), + ]; + } + + /** + * @param list $findings + * @return list> + */ + private function results(array $findings): array + { + $results = []; + foreach ($findings as $finding) { + $severity = $this->thresholds->severityOf($finding); + if ($severity === null) { + continue; + } + $results[] = $this->result($finding, $severity); + } + + return $results; + } + + /** + * @return array + */ + private function result(Finding $finding, Severity $severity): array + { + $result = [ + 'ruleId' => 'phptramp.trampData', + 'level' => $severity->label(), + 'message' => ['text' => $this->message->describe($finding)], + 'locations' => [$this->location($finding->chain[0])], + ]; + + $relatedLocations = $this->relatedLocations($finding); + if ($relatedLocations !== []) { + $result['relatedLocations'] = $relatedLocations; + } + + return $result; + } + + /** + * @return list> one entry per subsequent hop (chain + * indices 1 .. hops-1); the terminal + * node is never included + */ + private function relatedLocations(Finding $finding): array + { + $locations = []; + for ($index = 1; $index < $finding->hops; $index++) { + $hop = $finding->chain[$index]; + $location = $this->location($hop); + $location['message'] = [ + 'text' => 'hop ' . ($index + 1) . ' of $' . $finding->param . ' chain from ' . $finding->origin, + ]; + $locations[] = $location; + } + + return $locations; + } + + /** + * @return array + */ + private function location(Hop $hop): array + { + return [ + 'physicalLocation' => [ + 'artifactLocation' => ['uri' => $this->paths->relativize($hop->file)], + 'region' => ['startLine' => $hop->line], + ], + ]; + } +} diff --git a/src/Report/Severity.php b/src/Report/Severity.php new file mode 100644 index 0000000..dc33ef4 --- /dev/null +++ b/src/Report/Severity.php @@ -0,0 +1,20 @@ + 'error', + self::Warning => 'warning', + }; + } +} diff --git a/src/Report/SummaryReporter.php b/src/Report/SummaryReporter.php new file mode 100644 index 0000000..c10d7df --- /dev/null +++ b/src/Report/SummaryReporter.php @@ -0,0 +1,222 @@ +message = new FindingMessage(); + $this->pluralizer = new Pluralizer(); + } + + /** + * @param list $findings ALL findings, unfiltered + */ + public function render(array $findings): string + { + if ($findings === []) { + return "No chains found.\n"; + } + + $blocks = [ + $this->chainsByLengthBlock($findings), + $this->topLongestChainsBlock($findings), + $this->mostForwardedParametersBlock($findings), + $this->footer($findings), + ]; + + return implode("\n\n", $blocks) . "\n"; + } + + /** + * @param list $findings + */ + private function chainsByLengthBlock(array $findings): string + { + $counts = $this->countsByHopLength($findings); + $labels = $this->hopLengthLabels(array_keys($counts)); + $labelWidth = $this->maxLength($labels); + $maxCount = $this->nonEmptyMax($counts); + + $rows = []; + foreach ($counts as $hops => $count) { + $rows[] = self::INDENT . str_pad($labels[$hops], $labelWidth) . self::COLUMN_GAP + . str_repeat('#', $this->barLength($count, $maxCount)) . ' ' . $count; + } + + return "Chains by length:\n" . implode("\n", $rows); + } + + /** + * @param list $findings + * @return array hop count => number of findings, ascending by hop count + */ + private function countsByHopLength(array $findings): array + { + $counts = []; + foreach ($findings as $finding) { + $counts[$finding->hops] = ($counts[$finding->hops] ?? 0) + 1; + } + ksort($counts); + + return $counts; + } + + /** + * @param list $hopCounts + * @return array hop count => rendered label, e.g. "1 hop"/"2 hops" + */ + private function hopLengthLabels(array $hopCounts): array + { + $labels = []; + foreach ($hopCounts as $hops) { + $labels[$hops] = $hops . ' ' . $this->pluralizer->of($hops, 'hop'); + } + + return $labels; + } + + private function barLength(int $count, int $maxCount): int + { + if ($maxCount <= self::MAX_BAR_LENGTH) { + return $count; + } + + return max(1, (int) round($count * self::MAX_BAR_LENGTH / $maxCount)); + } + + /** + * @param list $findings + */ + private function topLongestChainsBlock(array $findings): string + { + $top = $this->longestFindings($findings); + $hopLabels = array_map( + fn (Finding $finding): string => $finding->hops . ' ' . $this->pluralizer->of($finding->hops, 'hop'), + $top, + ); + $params = array_map(static fn (Finding $finding): string => '$' . $finding->param, $top); + + $hopLabelWidth = $this->maxLength($hopLabels); + $paramWidth = $this->maxLength($params); + + $rows = []; + foreach ($top as $index => $finding) { + $rows[] = self::INDENT . str_pad($hopLabels[$index], $hopLabelWidth) . self::COLUMN_GAP + . str_pad($params[$index], $paramWidth) . self::COLUMN_GAP + . $finding->origin . ' -> ' . $this->message->terminalClause($finding); + } + + return "Top 10 longest chains:\n" . implode("\n", $rows); + } + + /** + * @param list $findings + * @return list sorted by hops desc, then origin asc, then param asc; capped at 10 + */ + private function longestFindings(array $findings): array + { + $sorted = $findings; + usort($sorted, static function (Finding $left, Finding $right): int { + return [-$left->hops, $left->origin, $left->param] + <=> [-$right->hops, $right->origin, $right->param]; + }); + + return array_slice($sorted, 0, self::TOP_CHAINS_LIMIT); + } + + /** + * @param list $findings + */ + private function mostForwardedParametersBlock(array $findings): string + { + $rows = []; + foreach ($this->paramCounts($findings) as $paramCount) { + $rows[] = self::INDENT . $paramCount['count'] . 'x' . self::COLUMN_GAP . '$' . $paramCount['param']; + } + + return "Most-forwarded parameters:\n" . implode("\n", $rows); + } + + /** + * @param list $findings + * @return list sorted by count desc, then param asc + */ + private function paramCounts(array $findings): array + { + $counts = []; + foreach ($findings as $finding) { + $counts[$finding->param] = ($counts[$finding->param] ?? 0) + 1; + } + + $pairs = []; + foreach ($counts as $param => $count) { + $pairs[] = ['param' => $param, 'count' => $count]; + } + + usort($pairs, static function (array $left, array $right): int { + return [-$left['count'], $left['param']] <=> [-$right['count'], $right['param']]; + }); + + return $pairs; + } + + /** + * @param list $findings + */ + private function footer(array $findings): string + { + $total = count($findings); + $atOrOverLimit = count( + array_filter($findings, fn (Finding $finding): bool => $finding->hops >= $this->thresholds->limit), + ); + + return $total . ' ' . $this->pluralizer->of($total, 'chain') . ' total; ' + . $atOrOverLimit . ' at or over the limit (limit: ' + . $this->thresholds->limit . ' ' . $this->pluralizer->of($this->thresholds->limit, 'hop') . ').'; + } + + /** + * @param array $strings key order and identity are + * irrelevant — only lengths matter + */ + private function maxLength(array $strings): int + { + return $this->nonEmptyMax(array_map('strlen', $strings)); + } + + /** + * @param array $values key order and identity are + * irrelevant — only the max value matters + */ + private function nonEmptyMax(array $values): int + { + // render() already guards against an empty $findings, so every caller + // reaches this with at least one value; the check just proves that to + // PHPStan without a magic fallback number. + if ($values === []) { + throw new \LogicException('nonEmptyMax() called with an empty array.'); + } + + return max($values); + } +} diff --git a/src/Report/TextReporter.php b/src/Report/TextReporter.php index d00611b..95268c3 100644 --- a/src/Report/TextReporter.php +++ b/src/Report/TextReporter.php @@ -12,39 +12,64 @@ * label column is at least as wide as the widest fixed label ("terminal") and * the method column is padded to the longest method entry in each finding. */ -final class TextReporter +final class TextReporter implements Reporter { private const INDENT = ' '; private const COLUMN_GAP = ' '; private const MIN_LABEL_WIDTH = 8; + private readonly Pluralizer $pluralizer; + public function __construct( - private readonly int $limit, + private readonly Thresholds $thresholds, + private readonly Paths $paths, private readonly bool $explain = false, ) { + $this->pluralizer = new Pluralizer(); } /** - * @param list $findings already filtered to hops >= limit + * @param list $findings ALL findings, unfiltered */ public function render(array $findings): string { - if ($findings === []) { + $reportable = $this->reportableFindings($findings); + if ($reportable === []) { return 'No tramp data found (' . $this->limitClause() . ").\n"; } - $blocks = array_map($this->renderFinding(...), $findings); + $blocks = array_map( + fn (array $reportableFinding): string => $this->renderFinding(...$reportableFinding), + $reportable, + ); - return implode("\n\n", $blocks) . "\n\n" . $this->summary(count($findings)) . "\n"; + return implode("\n\n", $blocks) . "\n\n" . $this->summary($reportable) . "\n"; } - private function renderFinding(Finding $finding): string + /** + * @param list $findings + * @return list + */ + private function reportableFindings(array $findings): array + { + $reportable = []; + foreach ($findings as $finding) { + $severity = $this->thresholds->severityOf($finding); + if ($severity !== null) { + $reportable[] = [$finding, $severity]; + } + } + + return $reportable; + } + + private function renderFinding(Finding $finding, Severity $severity): string { $hopRows = $this->hopRows($finding); $labelWidth = $this->labelWidth($hopRows, $finding->notes); $methodWidth = $this->methodWidth($hopRows); - $lines = [$this->header($finding)]; + $lines = [$this->header($finding, $severity)]; foreach ($hopRows as $row) { $lines[] = $this->hopLine($row, $labelWidth, $methodWidth); } @@ -110,7 +135,7 @@ private function label(int $index, bool $isTerminal): string private function location(Hop $hop): string { - return $hop->file . ':' . $hop->line; + return $this->paths->relativize($hop->file) . ':' . $hop->line; } /** @@ -161,29 +186,42 @@ private function methodWidth(array $rows): int return max($widths); } - private function header(Finding $finding): string + private function header(Finding $finding, Severity $severity): string { - return 'FINDING $' . $finding->param . ': ' - . $finding->hops . ' pass-through ' . $this->plural($finding->hops, 'hop') - . ' across ' . $finding->classes . ' ' . $this->plural($finding->classes, 'class', 'classes'); - } + $keyword = $severity === Severity::Warning ? 'WARNING' : 'FINDING'; - private function summary(int $count): string - { - return $count . ' ' . $this->plural($count, 'finding') . ' (' . $this->limitClause() . ').'; + return $keyword . ' $' . $finding->param . ': ' + . $finding->hops . ' pass-through ' . $this->pluralizer->of($finding->hops, 'hop') + . ' across ' . $finding->classes . ' ' . $this->pluralizer->of($finding->classes, 'class', 'classes'); } - private function limitClause(): string + /** + * @param list $reportable + */ + private function summary(array $reportable): string { - return 'limit: ' . $this->limit . ' ' . $this->plural($this->limit, 'hop'); + $count = count($reportable); + if ($this->thresholds->warnLimit === null) { + return $count . ' ' . $this->pluralizer->of($count, 'finding') . ' (' . $this->limitClause() . ').'; + } + + $errorCount = count(array_filter($reportable, static fn (array $entry): bool => $entry[1] === Severity::Error)); + $warningCount = $count - $errorCount; + + return $count . ' ' . $this->pluralizer->of($count, 'finding') . ' (' + . $errorCount . ' ' . $this->pluralizer->of($errorCount, 'error') . ', ' + . $warningCount . ' ' . $this->pluralizer->of($warningCount, 'warning') . '; ' + . $this->limitClause() . ').'; } - private function plural(int $count, string $singular, ?string $plural = null): string + private function limitClause(): string { - if ($count === 1) { - return $singular; + $clause = 'limit: ' . $this->thresholds->limit . ' ' . $this->pluralizer->of($this->thresholds->limit, 'hop'); + if ($this->thresholds->warnLimit !== null) { + $clause .= ', warn-limit: ' . $this->thresholds->warnLimit + . ' ' . $this->pluralizer->of($this->thresholds->warnLimit, 'hop'); } - return $plural ?? $singular . 's'; + return $clause; } } diff --git a/src/Report/Thresholds.php b/src/Report/Thresholds.php new file mode 100644 index 0000000..0afa440 --- /dev/null +++ b/src/Report/Thresholds.php @@ -0,0 +1,45 @@ +warnLimit !== null && $this->warnLimit >= $this->limit) { + throw new InvalidArgsException( + "warn-limit ({$this->warnLimit}) must be lower than limit ({$this->limit}).", + ); + } + } + + /** Null = below every threshold: not reported at all. */ + public function severityOf(Finding $finding): ?Severity + { + if ($finding->hops >= $this->limit) { + return Severity::Error; + } + if ($this->warnLimit !== null && $finding->hops >= $this->warnLimit) { + return Severity::Warning; + } + + return null; + } +} diff --git a/tests/Chain/ChainBuilderTest.php b/tests/Chain/ChainBuilderTest.php index 8ccd9a8..0a2aee5 100644 --- a/tests/Chain/ChainBuilderTest.php +++ b/tests/Chain/ChainBuilderTest.php @@ -245,4 +245,149 @@ public function testFindingsFromOneOriginAreSortedByParamName(): void self::assertSame('aaa', $findings[0]->param); self::assertSame('bbb', $findings[1]->param); } + + public function testMethodLevelAttributeOnMiddleHopKillsTheChain(): void + { + $code = 'process($config); } } ' + . 'class ServiceA { #[TrampIgnore] public function process(Cfg $config): void { ' + . '(new ServiceB())->run($config); } } ' + . 'class ServiceB { public function run(Cfg $config): void { $config->x(); } }'; + + self::assertCount(0, $this->build($code)); + } + + public function testParamLevelAttributeOnlyKillsChainsOfThatParam(): void + { + $code = 'take($ignored); (new B())->take($kept); } } ' + . 'class B { public function take(Cfg $p): void { $p->x(); } }'; + + $findings = $this->build($code); + self::assertCount(1, $findings); + self::assertSame('kept', $findings[0]->param); + } + + public function testAttributeOnAPlainFunctionMiddleHopKillsTheChain(): void + { + // Function-level (not method-level) attribute suppression: recordFunction + // must collect it, exactly like recordMethod does for class methods. + $code = 'take($p); } ' + . 'class B { public function take(Cfg $p): void { $p->x(); } }'; + + self::assertCount(0, $this->build($code)); + } + + public function testClassLevelAttributeKillsAllChainsThroughAnyMethodOfTheClass(): void + { + // Two independent origins, each forwarding through a different method of + // the suppressed class ServiceA (process, alt) into Sink. Both chains + // must be dropped: class-level suppression covers every method. + $code = 'take($config); } ' + . 'public function alt(Cfg $config): void { (new Sink())->take($config); } } ' + . 'class Controller { public function handle(Cfg $config): void { (new ServiceA())->process($config); } ' + . 'public function handleAlt(Cfg $config): void { (new ServiceA())->alt($config); } } ' + . 'class Sink { public function take(Cfg $config): void { $config->x(); } }'; + + self::assertCount(0, $this->build($code)); + } + + public function testIgnoreCommentOnForwardingCallSiteKillsTheChain(): void + { + $code = "step(\$p); // phptramp-ignore\n } }\n" + . 'class B { public function step(Cfg $p): void { $p->x(); } }'; + + self::assertCount(0, $this->build($code)); + } + + public function testIgnoreCommentOnDeclarationLineKillsTheChain(): void + { + // Comment suppression targets a hop node's own declaration line. B::step + // must be a mid-chain hop (forwarding on to C), not the chain's terminal, + // since terminal nodes never suppress. + $code = "step(\$p); } }\n" + . "class B { public function step(Cfg \$p): void { (new C())->use(\$p); } } // phptramp-ignore\n" + . 'class C { public function use(Cfg $p): void { $p->x(); } }'; + + self::assertCount(0, $this->build($code)); + } + + public function testIgnoreCommentOnLineAboveDeclarationKillsTheChain(): void + { + $code = "step(\$p); } }\n" + . "class B {\n" + . "// phptramp-ignore\n" + . "public function step(Cfg \$p): void { (new C())->use(\$p); }\n" + . "}\n" + . 'class C { public function use(Cfg $p): void { $p->x(); } }'; + + self::assertCount(0, $this->build($code)); + } + + public function testIgnoreCommentOnUnrelatedLineDoesNotDropTheChain(): void + { + $code = "step(\$p); } }\n" + . "class B { public function step(Cfg \$p): void { \$p->x(); } }"; + + self::assertCount(1, $this->build($code)); + } + + public function testIgnoreMarkerInsideAStringLiteralDoesNotSuppress(): void + { + // The marker text appears inside a string literal on the forwarding + // line, not in an actual PHP comment. It must not be mistaken for a + // suppression - the finding must still be reported. + $code = "step($p); } } ' + . 'class B { public function step(Cfg $p): void { $p->x(); } }'; + + self::assertCount(1, $this->build($code)); + } + + public function testRenamedParamAtNonOriginHopSuppressesOnlyThatParamsChains(): void + { + // The attribute sits on a mid-chain hop's own (differently-named) + // parameter, not the origin's. This proves the per-hop param name used + // for suppression lookup comes from PartialChain::keys (built from + // B::mid's own param name) rather than the origin's param name. The + // sibling param "other", bound positionally to B::mid's un-suppressed + // second param, must still be found. + $code = 'mid($ignored, $kept); } } ' + . 'class B { public function mid(#[TrampIgnore] Cfg $renamed, Cfg $other): void { ' + . '(new C())->takeRenamed($renamed); (new C())->takeOther($other); } } ' + . 'class C { public function takeRenamed(Cfg $p): void { $p->x(); } ' + . 'public function takeOther(Cfg $p): void { $p->x(); } }'; + + $findings = $this->build($code); + self::assertCount(1, $findings); + self::assertSame('kept', $findings[0]->param); + self::assertSame('Demo\C::takeOther', $findings[0]->terminal); + } + + public function testShortNameAttributeMatchesWithoutImportingOurTrampIgnoreClass(): void + { + // No `use PhpTramp\Ignore\TrampIgnore;` here: matching is purely by the + // attribute name's short name, so analyzed codebases are not required + // to autoload our class. + $code = 'step($p); } } ' + . 'class B { #[TrampIgnore] public function step(Cfg $p): void { (new C())->use($p); } } ' + . 'class C { public function use(Cfg $p): void { $p->x(); } }'; + + self::assertCount(0, $this->build($code)); + } } diff --git a/tests/Config/ConfigLoaderTest.php b/tests/Config/ConfigLoaderTest.php new file mode 100644 index 0000000..4d41ccd --- /dev/null +++ b/tests/Config/ConfigLoaderTest.php @@ -0,0 +1,206 @@ +directory = $directory; + } + + protected function tearDown(): void + { + array_map('unlink', glob($this->directory . '/*') ?: []); + rmdir($this->directory); + } + + private function writeConfig(string $filename, string $contents): void + { + file_put_contents($this->directory . '/' . $filename, $contents); + } + + public function testMissingBothFilesReturnsPlainDefaults(): void + { + $options = (new ConfigLoader())->load($this->directory); + + self::assertEquals(new Options(), $options); + } + + public function testReadsPhptrampJson(): void + { + $this->writeConfig('phptramp.json', '{"limit": 5}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame(5, $options->limit); + } + + public function testFallsBackToDistFileWhenPrimaryAbsent(): void + { + $this->writeConfig('phptramp.dist.json', '{"limit": 7}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame(7, $options->limit); + } + + public function testPrefersNonDistFileWhenBothExist(): void + { + $this->writeConfig('phptramp.json', '{"limit": 5}'); + $this->writeConfig('phptramp.dist.json', '{"limit": 7}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame(5, $options->limit); + } + + public function testUnknownKeyThrows(): void + { + $this->writeConfig('phptramp.json', '{"nope": true}'); + + $this->expectException(ConfigException::class); + $this->expectExceptionMessage('unknown config key: nope'); + + (new ConfigLoader())->load($this->directory); + } + + public function testPathsSplitsPhpEntriesIntoFilesAndRestIntoFolders(): void + { + $this->writeConfig('phptramp.json', '{"paths": ["src", "a.php", "tests", "b.php"]}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame(['src', 'tests'], $options->folders); + self::assertSame(['a.php', 'b.php'], $options->files); + } + + public function testPathsMustBeAList(): void + { + $this->writeConfig('phptramp.json', '{"paths": "src"}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testPathsWithNonStringEntryThrows(): void + { + $this->writeConfig('phptramp.json', '{"paths": ["src", 3]}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testWrongTypeForLimitThrows(): void + { + $this->writeConfig('phptramp.json', '{"limit": "three"}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testWrongTypeForWarnLimitThrows(): void + { + $this->writeConfig('phptramp.json', '{"warnLimit": "two"}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testWrongTypeForFormatThrows(): void + { + $this->writeConfig('phptramp.json', '{"format": 3}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testWrongTypeForBaselineThrows(): void + { + $this->writeConfig('phptramp.json', '{"baseline": 3}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testMalformedJsonThrows(): void + { + $this->writeConfig('phptramp.json', '{not valid json'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testTopLevelJsonArrayThrows(): void + { + $this->writeConfig('phptramp.json', '[1, 2, 3]'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testExcludeIsMapped(): void + { + $this->writeConfig('phptramp.json', '{"exclude": ["vendor/*", "*Test.php"]}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame(['vendor/*', '*Test.php'], $options->exclude); + } + + public function testExcludeMustBeAList(): void + { + $this->writeConfig('phptramp.json', '{"exclude": "nope"}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testExcludeWithNonStringEntryThrows(): void + { + $this->writeConfig('phptramp.json', '{"exclude": ["vendor/*", 3]}'); + + $this->expectException(ConfigException::class); + + (new ConfigLoader())->load($this->directory); + } + + public function testFormatAndBaselineAreMapped(): void + { + $this->writeConfig('phptramp.json', '{"format": "json", "baseline": "baseline.json"}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame('json', $options->format); + self::assertSame('baseline.json', $options->baseline); + } + + public function testWarnLimitIsMapped(): void + { + $this->writeConfig('phptramp.json', '{"warnLimit": 2, "limit": 3}'); + + $options = (new ConfigLoader())->load($this->directory); + + self::assertSame(2, $options->warnLimit); + } +} diff --git a/tests/Console/ApplicationTest.php b/tests/Console/ApplicationTest.php index 4f7ed30..6963e5e 100644 --- a/tests/Console/ApplicationTest.php +++ b/tests/Console/ApplicationTest.php @@ -97,15 +97,121 @@ public function testLimitAboveChainLengthReportsNothingAndExitsZero(): void self::assertStringContainsString('No tramp data found', self::contents($this->stdout)); } - public function testUnsupportedFormatExitsWithToolErrorCode(): void + public function testUnknownFormatExitsWithToolErrorCode(): void { $folder = $this->fixtureWithThreeHopChain(); - self::assertSame(2, $this->app->run(['phptramp', '--folder', $folder, '--format', 'json'])); - self::assertStringContainsString('not implemented', self::contents($this->stderr)); + self::assertSame(2, $this->app->run(['phptramp', '--folder', $folder, '--format', 'xml'])); + self::assertStringContainsString('unknown format: xml', self::contents($this->stderr)); self::assertSame('', self::contents($this->stdout)); } + public function testSummaryFormatReportsAllChainsAndExitsOne(): void + { + $folder = $this->fixtureWithThreeHopChain(); + + self::assertSame(1, $this->app->run(['phptramp', '--folder', $folder, '--format', 'summary'])); + self::assertStringContainsString('at or over the limit', self::contents($this->stdout)); + } + + public function testJsonFormatReportsFindingsAndExitsOne(): void + { + $folder = $this->fixtureWithThreeHopChain(); + + self::assertSame(1, $this->app->run(['phptramp', '--folder', $folder, '--format', 'json'])); + + $output = self::contents($this->stdout); + self::assertStringContainsString('"findings"', $output); + self::assertStringContainsString('"severity": "error"', $output); + } + + public function testWarnOnlyRunExitsZeroButPrintsWarning(): void + { + $folder = $this->fixtureWithTwoHopChain(); + + self::assertSame( + 0, + $this->app->run(['phptramp', '--folder', $folder, '--limit', '3', '--warn-limit', '2']), + ); + self::assertStringContainsString('WARNING', self::contents($this->stdout)); + } + + /** + * Thresholds validation must happen before the codebase is indexed, so an + * invalid --warn-limit/--limit combination fails fast: the fixture here is + * a normal, successfully-parsing 1-hop chain, since fail-fast only changes + * *when* the error surfaces, not whether this particular folder would + * parse. + */ + public function testInvalidWarnLimitFailsFastWithThresholdError(): void + { + $folder = $this->fixtureWithOneHopChain(); + + $exitCode = $this->app->run(['phptramp', '--folder', $folder, '--limit', '3', '--warn-limit', '5']); + + self::assertSame(2, $exitCode); + self::assertStringContainsString( + 'warn-limit (5) must be lower than limit (3)', + self::contents($this->stderr), + ); + } + + public function testConfigFileLimitAppliesWithoutCliFlag(): void + { + $directory = sys_get_temp_dir() . '/phptramp-cwd-' . uniqid(); + mkdir($directory); + $this->folders[] = $directory; + + file_put_contents($directory . '/phptramp.json', '{"limit": 1}'); + + $code = 'c = $config; } }'; + file_put_contents($directory . '/Demo.php', $code); + + $previousCwd = getcwd(); + self::assertIsString($previousCwd); + + try { + chdir($directory); + $exitCode = $this->app->run(['phptramp', '--folder', $directory]); + } finally { + chdir($previousCwd); + } + + self::assertSame(1, $exitCode); + self::assertStringContainsString('FINDING', self::contents($this->stdout)); + } + + private function fixtureWithOneHopChain(): string + { + $folder = sys_get_temp_dir() . '/phptramp-app-' . uniqid(); + mkdir($folder); + $this->folders[] = $folder; + + $code = 'c = $config; } }'; + file_put_contents($folder . '/Demo.php', $code); + + return $folder; + } + + private function fixtureWithTwoHopChain(): string + { + $folder = sys_get_temp_dir() . '/phptramp-app-' . uniqid(); + mkdir($folder); + $this->folders[] = $folder; + + $code = 'process($config); } } ' + . 'class ServiceA { public function process(Cfg $config): void { new Mailer($config); } } ' + . 'class Mailer { private Cfg $c; public function __construct(Cfg $config) { $this->c = $config; } }'; + file_put_contents($folder . '/Demo.php', $code); + + return $folder; + } + private function fixtureWithThreeHopChain(): string { $folder = sys_get_temp_dir() . '/phptramp-app-' . uniqid(); diff --git a/tests/Console/ArgvParserTest.php b/tests/Console/ArgvParserTest.php index 91f4f20..0ee8371 100644 --- a/tests/Console/ArgvParserTest.php +++ b/tests/Console/ArgvParserTest.php @@ -173,4 +173,81 @@ public function testMissingValueMessageNamesTheFlag(): void self::assertStringContainsString('--limit', $e->getMessage()); } } + + public function testSeededDefaultSurvivesWhenFlagAbsent(): void + { + $defaults = new Options(limit: 1); + + self::assertSame(1, (new ArgvParser())->parse([], $defaults)->limit); + } + + public function testExplicitFlagOverridesSeededDefault(): void + { + $defaults = new Options(limit: 1); + + self::assertSame(5, (new ArgvParser())->parse(['--limit', '5'], $defaults)->limit); + } + + public function testFirstPathFlagClearsSeededPaths(): void + { + $defaults = new Options(folders: ['configdir'], files: ['configfile.php']); + + $options = (new ArgvParser())->parse(['--folder', 'cli'], $defaults); + + self::assertSame(['cli'], $options->folders); + self::assertSame([], $options->files); + } + + public function testSecondPathFlagAppendsRatherThanClearing(): void + { + $defaults = new Options(folders: ['configdir']); + + $options = (new ArgvParser())->parse(['--folder', 'a', '--folder', 'b'], $defaults); + + self::assertSame(['a', 'b'], $options->folders); + } + + public function testFileFlagClearsSeededPaths(): void + { + $defaults = new Options(folders: ['configdir'], files: ['configfile.php']); + + $options = (new ArgvParser())->parse(['--file', 'cli.php'], $defaults); + + self::assertSame([], $options->folders); + self::assertSame(['cli.php'], $options->files); + } + + public function testFilesFlagClearsSeededPaths(): void + { + $defaults = new Options(folders: ['configdir'], files: ['configfile.php']); + + $options = (new ArgvParser())->parse(['--files', 'a.php,b.php'], $defaults); + + self::assertSame([], $options->folders); + self::assertSame(['a.php', 'b.php'], $options->files); + } + + public function testUnknownFlagWithEqualsSyntaxThrows(): void + { + $this->expectException(InvalidArgsException::class); + $this->parse('--nope=value'); + } + + public function testSeededExcludeSurvivesWhenAbsentFromArgv(): void + { + $defaults = new Options(exclude: ['vendor/*']); + + self::assertSame(['vendor/*'], (new ArgvParser())->parse([], $defaults)->exclude); + } + + public function testSeededNonPathDefaultsSurviveAlongsideExplicitFlags(): void + { + $defaults = new Options(format: 'json', gitBase: 'develop'); + + $options = (new ArgvParser())->parse(['--limit', '5'], $defaults); + + self::assertSame('json', $options->format); + self::assertSame('develop', $options->gitBase); + self::assertSame(5, $options->limit); + } } diff --git a/tests/Discovery/FileLocatorTest.php b/tests/Discovery/FileLocatorTest.php index ea67e6e..4afba1b 100644 --- a/tests/Discovery/FileLocatorTest.php +++ b/tests/Discovery/FileLocatorTest.php @@ -42,7 +42,7 @@ protected function tearDown(): void */ private function locate(Options $options): array { - return (new FileLocator())->locate($options); + return (new FileLocator($this->root))->locate($options); } private function real(string $relative): string @@ -112,4 +112,54 @@ public function testUppercasePhpExtensionIsMatchedCaseInsensitively(): void self::assertContains($this->real('Upper.PHP'), $this->locate(new Options(folders: [$this->root]))); } + + public function testExcludeGlobDropsNestedDiscoveredFile(): void + { + mkdir($this->root . '/vendor/pkg/src', 0777, true); + file_put_contents($this->root . '/vendor/pkg/src/X.php', "locate(new Options(folders: [$this->root], exclude: ['vendor/*'])); + + self::assertNotContains($this->real('vendor/pkg/src/X.php'), $found); + self::assertContains($this->real('a.php'), $found); + } + + public function testExcludeGlobDropsDiscoveredFileByBasenamePattern(): void + { + file_put_contents($this->root . '/sub/bThingTest.php', "locate(new Options(folders: [$this->root], exclude: ['*Test.php'])); + + self::assertNotContains($this->real('sub/bThingTest.php'), $found); + self::assertContains($this->real('a.php'), $found); + } + + public function testExcludeMatchesRawPathWhenFileIsOutsideWorkingDirectory(): void + { + $otherDirectory = sys_get_temp_dir() . '/phptramp-loc-other-' . uniqid('', true); + mkdir($otherDirectory); + + try { + $found = (new FileLocator($otherDirectory))->locate( + new Options(folders: [$this->root], exclude: [$this->real('a.php')]) + ); + + self::assertNotContains($this->real('a.php'), $found); + self::assertContains($this->real('sub/b.php'), $found); + } finally { + rmdir($otherDirectory); + } + } + + public function testExplicitFileBeatsExclude(): void + { + $found = $this->locate(new Options( + folders: [$this->root], + files: [$this->root . '/a.php'], + exclude: ['a.php', '*.php'], + )); + + self::assertContains($this->real('a.php'), $found); + self::assertNotContains($this->real('sub/b.php'), $found); + } } diff --git a/tests/FixtureTest.php b/tests/FixtureTest.php index 7c56e1d..7bc64c0 100644 --- a/tests/FixtureTest.php +++ b/tests/FixtureTest.php @@ -4,24 +4,22 @@ namespace PhpTramp\Tests; -use PhpTramp\Chain\ChainBuilder; -use PhpTramp\Chain\Finding; -use PhpTramp\Chain\Hop; use PhpTramp\Console\Application; -use PhpTramp\Console\Options; -use PhpTramp\Discovery\FileLocator; -use PhpTramp\Index\Indexer; -use PhpTramp\Resolve\CallResolver; -use PhpTramp\Resolve\ClassHierarchy; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; /** * Fixture harness: runs the real pipeline over every tests/fixtures//src. * A case with expected-index.json pins the classifier's `--dump-index` output; a - * case with expected-findings.json pins the chain builder's findings (chain - * flattened to FQMNs — the CLI JSON format arrives in Phase 3). Adding a semantic - * rule means adding a fixture directory, not touching this test. + * case with expected-findings.json pins the real CLI's `--format json` output + * (via `Application::run`), byte-for-byte modulo `file` path normalization. + * Adding a semantic rule means adding a fixture directory, not touching this test. + * + * A findings case runs in one of two modes, chosen by what sits next to its + * expected-findings.json: a `phptramp.json` routes it through config mode + * (chdir into the fixture, no `--folder`); otherwise it runs in folder mode + * (`--folder /src`), with CLI args from an optional `phptramp-args.json` + * or the default `--limit 1`. */ final class FixtureTest extends TestCase { @@ -70,38 +68,193 @@ public function testFindingsMatchExpectation(string $caseDir): void $expectedJson = file_get_contents($caseDir . '/expected-findings.json'); self::assertIsString($expectedJson); $expected = json_decode($expectedJson, true); + self::assertIsArray($expected); - self::assertEquals($expected, $this->findings($caseDir . '/src')); + [$exitCode, $decoded] = $this->runFindingsJson($caseDir); + + self::assertSame($this->expectedExitCode($expected), $exitCode); + self::assertEquals($expected, $decoded); + } + + /** + * A finding at or over `--limit` reports `severity: "error"` and must fail + * the run; anything else (all-warnings, or no findings) must not. + * + * @param array $expected + */ + private function expectedExitCode(array $expected): int + { + $findings = $expected['findings'] ?? []; + self::assertIsArray($findings); + + foreach ($findings as $finding) { + self::assertIsArray($finding); + if ($finding['severity'] === 'error') { + return 1; + } + } + + return 0; + } + + /** + * Routes to config mode (a `phptramp.json` sits in the fixture directory) or + * folder mode (plain `--folder /src`), each producing an + * already-normalized findings document ready to compare against + * expected-findings.json. + * + * @return array{0: int, 1: array} + */ + private function runFindingsJson(string $caseDir): array + { + if (is_file($caseDir . '/phptramp.json')) { + return $this->runConfigMode($caseDir); + } + + return $this->runFolderMode($caseDir); + } + + /** + * Folder mode: `--folder /src` plus either the fixture's + * `phptramp-args.json` extra CLI args or the default `--limit 1`. The + * reporter relativizes `file` against getcwd() (the repo root at test + * time), so JSON `file` values arrive repo-relative, e.g. + * "tests/fixtures//src/Demo.php" — normalize back to case-relative. + * + * @return array{0: int, 1: array} + */ + private function runFolderMode(string $caseDir): array + { + $argv = ['phptramp', '--folder', $caseDir . '/src', '--format', 'json', ...$this->extraCliArgs($caseDir)]; + [$exitCode, $decoded] = $this->runApplication($argv); + + return [$exitCode, $this->normalizeFindingsDocument($decoded, $caseDir)]; + } + + /** + * Config mode: chdir into the fixture directory so its `phptramp.json` + * 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. + * + * @return array{0: int, 1: array} + */ + private function runConfigMode(string $caseDir): array + { + $previousCwd = getcwd(); + self::assertIsString($previousCwd); + + try { + self::assertTrue(chdir($caseDir)); + + return $this->runApplication(['phptramp', '--format', 'json']); + } finally { + chdir($previousCwd); + } + } + + /** + * @return list + */ + private function extraCliArgs(string $caseDir): array + { + $argsFile = $caseDir . '/phptramp-args.json'; + if (! is_file($argsFile)) { + return ['--limit', '1']; + } + + $json = file_get_contents($argsFile); + self::assertIsString($json); + $args = json_decode($json, true); + self::assertIsArray($args); + + $cliArgs = []; + foreach ($args as $arg) { + self::assertIsString($arg); + $cliArgs[] = $arg; + } + + return $cliArgs; + } + + /** + * @param list $argv + * @return array{0: int, 1: array} + */ + private function runApplication(array $argv): array + { + $stdout = fopen('php://memory', 'w+'); + $stderr = fopen('php://memory', 'w+'); + self::assertIsResource($stdout); + self::assertIsResource($stderr); + + $exitCode = (new Application($stdout, $stderr))->run($argv); + + rewind($stdout); + $json = stream_get_contents($stdout); + self::assertIsString($json); + $decoded = json_decode($json, true); + self::assertIsArray($decoded); + + return [$exitCode, $decoded]; } /** - * @return array{findings: list>} + * The reporter relativizes `file` against getcwd() (the repo root at test + * time), so JSON `file` values arrive repo-relative, e.g. + * "tests/fixtures//src/Demo.php". Expected files use case-relative + * paths, e.g. "src/Demo.php" — strip the case-dir prefix, computed relative + * to getcwd(), from every chain entry's `file`. + * + * @param array $decoded + * @return array */ - private function findings(string $srcDir): array + private function normalizeFindingsDocument(array $decoded, string $caseDir): array { - $files = (new FileLocator())->locate(new Options(folders: [$srcDir])); - $index = (new Indexer())->index($files); - $resolver = new CallResolver($index, new ClassHierarchy($index)); - $findings = (new ChainBuilder($resolver))->build($index); + $realCaseDir = realpath($caseDir); + $cwd = getcwd(); + self::assertIsString($realCaseDir); + self::assertIsString($cwd); + + $prefix = null; + if (str_starts_with($realCaseDir, $cwd . '/')) { + $prefix = substr($realCaseDir, strlen($cwd) + 1) . '/'; + } + + $findings = $decoded['findings'] ?? []; + self::assertIsArray($findings); + $decoded['findings'] = array_map( + fn (array $finding): array => $this->normalizeFindingDocument($finding, $prefix), + $findings, + ); - return ['findings' => array_map($this->normalizeFinding(...), $findings)]; + return $decoded; } /** + * @param array $finding * @return array */ - private function normalizeFinding(Finding $finding): array + private function normalizeFindingDocument(array $finding, ?string $prefix): array { - return [ - 'param' => $finding->param, - 'origin' => $finding->origin, - 'terminal' => $finding->terminal, - 'terminalKind' => $finding->terminalKind->value, - 'hops' => $finding->hops, - 'chain' => array_map(static fn (Hop $hop): string => $hop->fqmn, $finding->chain), - 'classes' => $finding->classes, - 'notes' => $finding->notes, - ]; + $chain = $finding['chain'] ?? []; + self::assertIsArray($chain); + + $finding['chain'] = array_map( + function (array $hop) use ($prefix): array { + $file = $hop['file']; + self::assertIsString($file); + if ($prefix !== null && str_starts_with($file, $prefix)) { + $hop['file'] = substr($file, strlen($prefix)); + } + + return $hop; + }, + $chain, + ); + + return $finding; } /** diff --git a/tests/Ignore/SuppressionIndexTest.php b/tests/Ignore/SuppressionIndexTest.php new file mode 100644 index 0000000..a548c23 --- /dev/null +++ b/tests/Ignore/SuppressionIndexTest.php @@ -0,0 +1,71 @@ +suppressesMethod('Demo\A::go')); + self::assertFalse($index->suppressesMethod('Demo\A::other')); + } + + public function testSuppressesParamForExactFqmnAndParamPair(): void + { + $index = new SuppressionIndex([], [['Demo\A::go', 'config']], []); + + self::assertTrue($index->suppressesParam('Demo\A::go', 'config')); + self::assertFalse($index->suppressesParam('Demo\A::go', 'other')); + self::assertFalse($index->suppressesParam('Demo\A::other', 'config')); + } + + public function testSuppressesLineForExactFileAndLine(): void + { + $index = new SuppressionIndex([], [], ['/path/to/File.php' => [10, 12]]); + + self::assertTrue($index->suppressesLine('/path/to/File.php', 10)); + self::assertTrue($index->suppressesLine('/path/to/File.php', 12)); + self::assertFalse($index->suppressesLine('/path/to/File.php', 11)); + self::assertFalse($index->suppressesLine('/other/File.php', 10)); + } + + public function testSuppressesParamForMultipleDistinctPairsIndependently(): void + { + $index = new SuppressionIndex([], [ + ['Demo\A::go', 'config'], + ['Demo\B::step', 'other'], + ], []); + + self::assertTrue($index->suppressesParam('Demo\A::go', 'config')); + self::assertTrue($index->suppressesParam('Demo\B::step', 'other')); + self::assertFalse($index->suppressesParam('Demo\B::step', 'config')); + } + + public function testSuppressesLineAcrossMultipleFilesIndependently(): void + { + $index = new SuppressionIndex([], [], [ + '/path/to/A.php' => [10], + '/path/to/B.php' => [20], + ]); + + self::assertTrue($index->suppressesLine('/path/to/A.php', 10)); + self::assertTrue($index->suppressesLine('/path/to/B.php', 20)); + self::assertFalse($index->suppressesLine('/path/to/B.php', 10)); + } + + public function testEmptyIndexSuppressesNothing(): void + { + $index = new SuppressionIndex([], [], []); + + self::assertFalse($index->suppressesMethod('Demo\A::go')); + self::assertFalse($index->suppressesParam('Demo\A::go', 'config')); + self::assertFalse($index->suppressesLine('/path/to/File.php', 1)); + } +} diff --git a/tests/Report/CheckstyleReporterTest.php b/tests/Report/CheckstyleReporterTest.php new file mode 100644 index 0000000..60f6a28 --- /dev/null +++ b/tests/Report/CheckstyleReporterTest.php @@ -0,0 +1,176 @@ +' . "\n" + . '' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . '' . "\n"; + + $reporter = new CheckstyleReporter(new Thresholds(3, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testRendersWarningSeverityLabel(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + new Hop('Demo\C::sink', 'Demo\C', 'src/C.php', 13, null), + ]; + $finding = new Finding('p', 'Demo\A::go', 'Demo\C::sink', TerminalKind::Used, 2, $chain, 2, [], []); + + $expected = '' . "\n" + . '' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . '' . "\n"; + + $reporter = new CheckstyleReporter(new Thresholds(3, 2), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testGroupsFindingsByOriginFilePreservingFirstSeenAndInputOrder(): void + { + $aFinding = new Finding( + 'p', + 'Demo\A::go', + 'Demo\A::sink', + TerminalKind::Used, + 1, + [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\A::sink', 'Demo\A', 'src/A.php', 9, null), + ], + 1, + [], + [], + ); + $bFinding = new Finding( + 'q', + 'Demo\B::go', + 'Demo\B::sink', + TerminalKind::Used, + 1, + [ + new Hop('Demo\B::go', 'Demo\B', 'src/B.php', 15, 17), + new Hop('Demo\B::sink', 'Demo\B', 'src/B.php', 19, null), + ], + 1, + [], + [], + ); + $secondAFinding = new Finding( + 'r', + 'Demo\A::again', + 'Demo\A::sink', + TerminalKind::Used, + 1, + [ + new Hop('Demo\A::again', 'Demo\A', 'src/A.php', 25, 27), + new Hop('Demo\A::sink', 'Demo\A', 'src/A.php', 29, null), + ], + 1, + [], + [], + ); + + $expected = '' . "\n" + . '' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . '' . "\n"; + + $reporter = new CheckstyleReporter(new Thresholds(1, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$aFinding, $bFinding, $secondAFinding])); + } + + public function testEscapesMessageXmlEntities(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::sink', 'Demo\B', 'src/A.php', 9, null), + ]; + $finding = new Finding('p', 'Demo\A::go', 'Demo\B::sink', TerminalKind::Used, 1, $chain, 1, [], []); + + $expected = '' . "\n" + . '' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . ' ' . "\n" + . '' . "\n"; + + $reporter = new CheckstyleReporter(new Thresholds(1, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testEmptyRunRendersSkeletonWithNoFileElements(): void + { + $expected = '' . "\n" + . '' . "\n" + . '' . "\n"; + + $reporter = new CheckstyleReporter(new Thresholds(3, null), $this->paths()); + + self::assertSame($expected, $reporter->render([])); + } +} diff --git a/tests/Report/FindingMessageTest.php b/tests/Report/FindingMessageTest.php new file mode 100644 index 0000000..034b010 --- /dev/null +++ b/tests/Report/FindingMessageTest.php @@ -0,0 +1,78 @@ +describe($finding), + ); + } + + public function testDescribesSingularHopAndClass(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\A::sink', 'Demo\A', 'src/A.php', 9, null), + ]; + $finding = new Finding('p', 'Demo\A::go', 'Demo\A::sink', TerminalKind::Used, 1, $chain, 1, [], []); + + self::assertSame( + '$p: 1 pass-through hop across 1 class (terminal: Demo\A::sink [used])', + (new FindingMessage())->describe($finding), + ); + } + + public function testDescribesNullTerminalWithKindOnlyClause(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + ]; + $finding = new Finding( + 'cfg', + 'Demo\A::go', + null, + TerminalKind::Truncated, + 2, + $chain, + 2, + ['truncated: 2 implementations'], + [], + ); + + self::assertSame( + '$cfg: 2 pass-through hops across 2 classes (terminal: truncated)', + (new FindingMessage())->describe($finding), + ); + } +} diff --git a/tests/Report/GithubReporterTest.php b/tests/Report/GithubReporterTest.php new file mode 100644 index 0000000..37d6c62 --- /dev/null +++ b/tests/Report/GithubReporterTest.php @@ -0,0 +1,192 @@ +paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testEmitsWarningAnnotationAndOmitsNoticeWhenNoSubsequentHops(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\A::sink', 'Demo\A', 'src/A.php', 9, null), + ]; + $finding = new Finding('p', 'Demo\A::go', 'Demo\A::sink', TerminalKind::Used, 1, $chain, 1, [], []); + + $expected = "::warning file=src/A.php,line=5,title=phptramp%3A%3A\$p%3A 1 pass-through hop across 1 class " + . "(terminal%3A Demo\\A%3A%3Asink [used])\n"; + + $reporter = new GithubReporter(new Thresholds(3, 1), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testOmitsBelowThresholdFindingEntirely(): void + { + $chain = [ + new Hop('Demo\C::go', 'Demo\C', 'src/C.php', 5, null), + ]; + $finding = new Finding('q', 'Demo\C::go', 'Demo\C::go', TerminalKind::Used, 1, $chain, 1, [], []); + + $reporter = new GithubReporter(new Thresholds(3, 2), $this->paths()); + + self::assertSame('', $reporter->render([$finding])); + } + + public function testEscapesPercentCarriageReturnAndNewlineInAnnotationData(): void + { + $chain = [ + new Hop("Demo\\A::go\r\nInjected", 'Demo\A', 'src/100%/A.php', 5, null), + ]; + $finding = new Finding( + 'p', + "Demo\\A::go\r\nInjected", + "Demo\\A::go\r\nInjected", + TerminalKind::Used, + 1, + $chain, + 1, + [], + [], + ); + + $expected = '::error file=src/100%25/A.php,line=5,title=phptramp%3A%3A$p%3A 1 pass-through hop across 1 ' + . "class (terminal%3A Demo\\A%3A%3Ago%0D%0AInjected [used])\n"; + + $reporter = new GithubReporter(new Thresholds(1, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + /** + * A `,` in a property value would otherwise be misread as the separator + * between it and the next `key=value` pair; pins the escapeProperty() + * branch that the percent/CR/LF test above never exercises. + */ + public function testEscapesCommaInAnnotationPropertyValue(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, null), + ]; + $finding = new Finding( + 'p', + 'Demo\A::go', + 'Demo\A, Inc::sink', + TerminalKind::Used, + 1, + $chain, + 1, + [], + [], + ); + + $expected = '::error file=src/A.php,line=5,title=phptramp%3A%3A$p%3A 1 pass-through hop across 1 class ' + . "(terminal%3A Demo\\A%2C Inc%3A%3Asink [used])\n"; + + $reporter = new GithubReporter(new Thresholds(1, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + /** + * Findings are not sorted by hop count, so a below-threshold finding can + * precede an above-threshold one in real input order. This pins that the + * per-finding `continue` (skip this finding, keep scanning) is not a + * `break` in disguise (stop scanning entirely) — a `break` here would + * silently drop every qualifying finding that comes after the first + * skipped one. + */ + public function testStillReportsQualifyingFindingAfterAnEarlierBelowThresholdFindingIsSkipped(): void + { + $belowChain = [ + new Hop('Demo\Z::go', 'Demo\Z', 'src/Z.php', 1, null), + ]; + $belowFinding = new Finding('z', 'Demo\Z::go', 'Demo\Z::go', TerminalKind::Used, 1, $belowChain, 1, [], []); + + $chain = [ + new Hop('Demo\Controller::handle', 'Demo\Controller', 'src/Demo.php', 12, 14), + new Hop('Demo\ServiceA::process', 'Demo\ServiceA', 'src/Demo.php', 18, 20), + new Hop('Demo\ServiceB::run', 'Demo\ServiceB', 'src/Demo.php', 24, 26), + new Hop('Demo\Mailer::__construct', 'Demo\Mailer', 'src/Demo.php', 32, null), + ]; + $aboveFinding = new Finding( + 'config', + 'Demo\Controller::handle', + 'Demo\Mailer::__construct', + TerminalKind::Stored, + 3, + $chain, + 4, + [], + [], + ); + + $expected = "::error file=src/Demo.php,line=12,title=phptramp%3A%3A\$config%3A 3 pass-through hops " + . "across 4 classes (terminal%3A Demo\\Mailer%3A%3A__construct [stored])\n" + . "::notice file=src/Demo.php,line=18,title=phptramp%3A%3Ahop 2 of \$config chain from " + . "Demo\\Controller%3A%3Ahandle\n" + . "::notice file=src/Demo.php,line=24,title=phptramp%3A%3Ahop 3 of \$config chain from " + . "Demo\\Controller%3A%3Ahandle\n"; + + $reporter = new GithubReporter(new Thresholds(3, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$belowFinding, $aboveFinding])); + } + + public function testEmptyRunRendersEmptyString(): void + { + $reporter = new GithubReporter(new Thresholds(3, null), $this->paths()); + + self::assertSame('', $reporter->render([])); + } +} diff --git a/tests/Report/JsonReporterTest.php b/tests/Report/JsonReporterTest.php new file mode 100644 index 0000000..9a9cf3f --- /dev/null +++ b/tests/Report/JsonReporterTest.php @@ -0,0 +1,343 @@ +paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testRendersWarningSeverityAndOmitsBelowThresholdFinding(): void + { + $warningChain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + new Hop('Demo\C::sink', 'Demo\C', 'src/C.php', 13, null), + ]; + $warningFinding = new Finding( + 'p', + 'Demo\A::go', + 'Demo\C::sink', + TerminalKind::Used, + 2, + $warningChain, + 2, + [], + [], + ); + + $belowWarnLimitChain = [ + new Hop('Demo\C::go', 'Demo\C', 'src/C.php', 5, null), + ]; + $belowWarnLimitFinding = new Finding( + 'q', + 'Demo\C::go', + 'Demo\C::go', + TerminalKind::Used, + 1, + $belowWarnLimitChain, + 1, + [], + [], + ); + + $expected = <<<'JSON' + { + "limit": 3, + "warnLimit": 2, + "findings": [ + { + "param": "p", + "severity": "warning", + "origin": "Demo\\A::go", + "terminal": "Demo\\C::sink", + "terminalKind": "used", + "hops": 2, + "classes": 2, + "chain": [ + { + "method": "Demo\\A::go", + "role": "origin", + "file": "src/A.php", + "line": 5, + "forwardLine": 7 + }, + { + "method": "Demo\\B::step", + "role": "hop", + "file": "src/B.php", + "line": 9, + "forwardLine": 11 + }, + { + "method": "Demo\\C::sink", + "role": "terminal", + "file": "src/C.php", + "line": 13, + "forwardLine": null + } + ], + "notes": [] + } + ] + } + + JSON; + + $reporter = new JsonReporter(new Thresholds(3, 2), $this->paths()); + + // Below-threshold finding comes first so an off-by-one on the filter + // (e.g. stopping instead of skipping) would also fail this assertion. + self::assertSame($expected, $reporter->render([$belowWarnLimitFinding, $warningFinding])); + } + + public function testIncludesAllQualifyingFindingsWhenMultiplePresent(): void + { + $firstChain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\A::sink', 'Demo\A', 'src/A.php', 9, null), + ]; + $firstFinding = new Finding('p', 'Demo\A::go', 'Demo\A::sink', TerminalKind::Used, 1, $firstChain, 1, [], []); + + $secondChain = [ + new Hop('Demo\B::go', 'Demo\B', 'src/B.php', 5, 7), + new Hop('Demo\B::sink', 'Demo\B', 'src/B.php', 9, null), + ]; + $secondFinding = new Finding('q', 'Demo\B::go', 'Demo\B::sink', TerminalKind::Used, 1, $secondChain, 1, [], []); + + $expected = <<<'JSON' + { + "limit": 1, + "warnLimit": null, + "findings": [ + { + "param": "p", + "severity": "error", + "origin": "Demo\\A::go", + "terminal": "Demo\\A::sink", + "terminalKind": "used", + "hops": 1, + "classes": 1, + "chain": [ + { + "method": "Demo\\A::go", + "role": "origin", + "file": "src/A.php", + "line": 5, + "forwardLine": 7 + }, + { + "method": "Demo\\A::sink", + "role": "terminal", + "file": "src/A.php", + "line": 9, + "forwardLine": null + } + ], + "notes": [] + }, + { + "param": "q", + "severity": "error", + "origin": "Demo\\B::go", + "terminal": "Demo\\B::sink", + "terminalKind": "used", + "hops": 1, + "classes": 1, + "chain": [ + { + "method": "Demo\\B::go", + "role": "origin", + "file": "src/B.php", + "line": 5, + "forwardLine": 7 + }, + { + "method": "Demo\\B::sink", + "role": "terminal", + "file": "src/B.php", + "line": 9, + "forwardLine": null + } + ], + "notes": [] + } + ] + } + + JSON; + + $reporter = new JsonReporter(new Thresholds(1, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$firstFinding, $secondFinding])); + } + + public function testChainWithNoTerminalNodeGivesLastEntryHopRoleAndNullTerminal(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + ]; + $finding = new Finding( + 'cfg', + 'Demo\A::go', + null, + TerminalKind::Truncated, + 2, + $chain, + 2, + ['truncated: 2 implementations'], + [], + ); + + $expected = <<<'JSON' + { + "limit": 2, + "warnLimit": null, + "findings": [ + { + "param": "cfg", + "severity": "error", + "origin": "Demo\\A::go", + "terminal": null, + "terminalKind": "truncated", + "hops": 2, + "classes": 2, + "chain": [ + { + "method": "Demo\\A::go", + "role": "origin", + "file": "src/A.php", + "line": 5, + "forwardLine": 7 + }, + { + "method": "Demo\\B::step", + "role": "hop", + "file": "src/B.php", + "line": 9, + "forwardLine": 11 + } + ], + "notes": [ + "truncated: 2 implementations" + ] + } + ] + } + + JSON; + + $reporter = new JsonReporter(new Thresholds(2, null), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testRendersEmptyFindingsArrayForEmptyRun(): void + { + $expected = <<<'JSON' + { + "limit": 3, + "warnLimit": null, + "findings": [] + } + + JSON; + + $reporter = new JsonReporter(new Thresholds(3, null), $this->paths()); + + self::assertSame($expected, $reporter->render([])); + } +} diff --git a/tests/Report/PathsTest.php b/tests/Report/PathsTest.php new file mode 100644 index 0000000..2ed1973 --- /dev/null +++ b/tests/Report/PathsTest.php @@ -0,0 +1,56 @@ +relativize('/home/user/project/src/Demo.php')); + } + + public function testRelativizesNestedSubdirectoryPath(): void + { + $paths = new Paths('/home/user/project'); + + self::assertSame( + 'src/Chain/Deep/Nested.php', + $paths->relativize('/home/user/project/src/Chain/Deep/Nested.php'), + ); + } + + public function testLeavesPathOutsideWorkingDirectoryUnchanged(): void + { + $paths = new Paths('/home/user/project'); + + self::assertSame('/var/other/Demo.php', $paths->relativize('/var/other/Demo.php')); + } + + public function testPathEqualToWorkingDirectoryRelativizesToEmptyString(): void + { + $paths = new Paths('/home/user/project'); + + self::assertSame('', $paths->relativize('/home/user/project')); + } + + public function testNormalizesTrailingSlashOnWorkingDirectory(): void + { + $paths = new Paths('/home/user/project/'); + + self::assertSame('src/Demo.php', $paths->relativize('/home/user/project/src/Demo.php')); + } + + public function testDoesNotTreatDifferentDirectoryWithSamePrefixAsUnderIt(): void + { + $paths = new Paths('/home/user/project'); + + self::assertSame('/home/user/project-other/Demo.php', $paths->relativize('/home/user/project-other/Demo.php')); + } +} diff --git a/tests/Report/PluralizerTest.php b/tests/Report/PluralizerTest.php new file mode 100644 index 0000000..7e51eeb --- /dev/null +++ b/tests/Report/PluralizerTest.php @@ -0,0 +1,32 @@ +of(1, 'hop')); + } + + public function testAppendsDefaultSSuffixWhenCountIsNotOne(): void + { + self::assertSame('hops', (new Pluralizer())->of(2, 'hop')); + self::assertSame('hops', (new Pluralizer())->of(0, 'hop')); + } + + public function testUsesExplicitIrregularPluralWhenCountIsNotOne(): void + { + self::assertSame('classes', (new Pluralizer())->of(4, 'class', 'classes')); + } + + public function testExplicitPluralIsIgnoredWhenCountIsOne(): void + { + self::assertSame('class', (new Pluralizer())->of(1, 'class', 'classes')); + } +} diff --git a/tests/Report/ReporterFactoryTest.php b/tests/Report/ReporterFactoryTest.php new file mode 100644 index 0000000..da5aebd --- /dev/null +++ b/tests/Report/ReporterFactoryTest.php @@ -0,0 +1,85 @@ + a single finding at or above every threshold used + * below (limit: 2) + */ + private function qualifyingFinding(): array + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + new Hop('Demo\C::sink', 'Demo\C', 'src/C.php', 13, null), + ]; + + return [new Finding('p', 'Demo\A::go', 'Demo\C::sink', TerminalKind::Used, 2, $chain, 2, [], [])]; + } + + public function testGithubFormatRoutesToGithubReporter(): void + { + $factory = new ReporterFactory('/not/matching'); + $reporter = $factory->create(new Options(format: 'github', limit: 2)); + + self::assertStringContainsString('::error', $reporter->render($this->qualifyingFinding())); + } + + public function testCheckstyleFormatRoutesToCheckstyleReporter(): void + { + $factory = new ReporterFactory('/not/matching'); + $reporter = $factory->create(new Options(format: 'checkstyle', limit: 2)); + + self::assertStringContainsString('render($this->qualifyingFinding())); + } + + public function testSarifFormatRoutesToSarifReporter(): void + { + $factory = new ReporterFactory('/not/matching'); + $reporter = $factory->create(new Options(format: 'sarif', limit: 2)); + + self::assertStringContainsString('"phptramp.trampData"', $reporter->render($this->qualifyingFinding())); + } + + public function testSummaryFormatRoutesToSummaryReporter(): void + { + $factory = new ReporterFactory('/not/matching'); + $reporter = $factory->create(new Options(format: 'summary', limit: 2)); + + self::assertStringContainsString('at or over the limit', $reporter->render($this->qualifyingFinding())); + } + + /** + * ArgvParser::validateFormat already rejects any format outside the six + * valid values before Options ever reaches the factory, so this exercises + * the `default` arm directly — the only way to reach it at all. + */ + public function testUnroutableFormatThrows(): void + { + $factory = new ReporterFactory('/not/matching'); + + $this->expectException(InvalidArgsException::class); + $this->expectExceptionMessage("format 'xml' is not implemented yet."); + + $factory->create(new Options(format: 'xml')); + } +} diff --git a/tests/Report/SarifReporterTest.php b/tests/Report/SarifReporterTest.php new file mode 100644 index 0000000..2988b61 --- /dev/null +++ b/tests/Report/SarifReporterTest.php @@ -0,0 +1,300 @@ +schemaUrl()}", + "runs": [ + { + "tool": { + "driver": { + "name": "phptramp", + "informationUri": "https://github.com/larspohlmann/phptramp", + "rules": [ + { + "id": "phptramp.trampData" + } + ] + } + }, + "results": [ + { + "ruleId": "phptramp.trampData", + "level": "error", + "message": { + JSON; + $expected .= "\n" . ' "text": "$config: 3 pass-through hops across 4 classes ' + . "(terminal: Demo\\\\Mailer::__construct [stored])\"\n"; + $expected .= <<paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testRendersWarningFindingAndOmitsRelatedLocationsWhenNoSubsequentHops(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\A::sink', 'Demo\A', 'src/A.php', 9, null), + ]; + $finding = new Finding('p', 'Demo\A::go', 'Demo\A::sink', TerminalKind::Used, 1, $chain, 1, [], []); + + $expected = <<schemaUrl()}", + "runs": [ + { + "tool": { + "driver": { + "name": "phptramp", + "informationUri": "https://github.com/larspohlmann/phptramp", + "rules": [ + { + "id": "phptramp.trampData" + } + ] + } + }, + "results": [ + { + "ruleId": "phptramp.trampData", + "level": "warning", + "message": { + "text": "\$p: 1 pass-through hop across 1 class (terminal: Demo\\\\A::sink [used])" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/A.php" + }, + "region": { + "startLine": 5 + } + } + } + ] + } + ] + } + ] + } + + JSON; + + $reporter = new SarifReporter(new Thresholds(3, 1), $this->paths()); + + self::assertSame($expected, $reporter->render([$finding])); + } + + public function testOmitsBelowThresholdFindingEntirely(): void + { + $chain = [ + new Hop('Demo\C::go', 'Demo\C', 'src/C.php', 5, null), + ]; + $finding = new Finding('q', 'Demo\C::go', 'Demo\C::go', TerminalKind::Used, 1, $chain, 1, [], []); + + $reporter = new SarifReporter(new Thresholds(3, 2), $this->paths()); + + self::assertStringNotContainsString('"ruleId"', $reporter->render([$finding])); + } + + /** + * Findings are not sorted by hop count, so a below-threshold finding can + * precede an above-threshold one in real input order. This pins that the + * per-finding `continue` (skip this finding, keep scanning) is not a + * `break` in disguise (stop scanning entirely) — a `break` here would + * silently drop every qualifying finding that comes after the first + * skipped one. + */ + public function testStillIncludesQualifyingFindingAfterAnEarlierBelowThresholdFindingIsSkipped(): void + { + $belowChain = [ + new Hop('Demo\Z::go', 'Demo\Z', 'src/Z.php', 1, null), + ]; + $belowFinding = new Finding('z', 'Demo\Z::go', 'Demo\Z::go', TerminalKind::Used, 1, $belowChain, 1, [], []); + + $aboveChain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + new Hop('Demo\C::sink', 'Demo\C', 'src/C.php', 13, null), + ]; + $aboveFinding = new Finding('p', 'Demo\A::go', 'Demo\C::sink', TerminalKind::Used, 2, $aboveChain, 2, [], []); + + $reporter = new SarifReporter(new Thresholds(3, 2), $this->paths()); + + $document = json_decode($reporter->render([$belowFinding, $aboveFinding]), true); + $results = $document['runs'][0]['results']; + + self::assertCount(1, $results); + self::assertSame('warning', $results[0]['level']); + self::assertSame( + '$p: 2 pass-through hops across 2 classes (terminal: Demo\C::sink [used])', + $results[0]['message']['text'], + ); + } + + public function testIncludesBothResultsWhenMultipleFindingsQualify(): void + { + $firstChain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\A::sink', 'Demo\A', 'src/A.php', 9, null), + ]; + $firstFinding = new Finding('p', 'Demo\A::go', 'Demo\A::sink', TerminalKind::Used, 1, $firstChain, 1, [], []); + + $secondChain = [ + new Hop('Demo\B::go', 'Demo\B', 'src/B.php', 5, 7), + new Hop('Demo\B::sink', 'Demo\B', 'src/B.php', 9, null), + ]; + $secondFinding = new Finding('q', 'Demo\B::go', 'Demo\B::sink', TerminalKind::Used, 1, $secondChain, 1, [], []); + + $reporter = new SarifReporter(new Thresholds(1, null), $this->paths()); + + $document = json_decode($reporter->render([$firstFinding, $secondFinding]), true); + $results = $document['runs'][0]['results']; + + self::assertCount(2, $results); + self::assertSame('phptramp.trampData', $results[0]['ruleId']); + self::assertSame('phptramp.trampData', $results[1]['ruleId']); + self::assertSame( + '$p: 1 pass-through hop across 1 class (terminal: Demo\A::sink [used])', + $results[0]['message']['text'], + ); + self::assertSame( + '$q: 1 pass-through hop across 1 class (terminal: Demo\B::sink [used])', + $results[1]['message']['text'], + ); + } + + public function testEmptyRunRendersValidDocumentWithEmptyResults(): void + { + $expected = <<schemaUrl()}", + "runs": [ + { + "tool": { + "driver": { + "name": "phptramp", + "informationUri": "https://github.com/larspohlmann/phptramp", + "rules": [ + { + "id": "phptramp.trampData" + } + ] + } + }, + "results": [] + } + ] + } + + JSON; + + $reporter = new SarifReporter(new Thresholds(3, null), $this->paths()); + + self::assertSame($expected, $reporter->render([])); + } + + private function schemaUrl(): string + { + return self::SCHEMA_URL; + } +} diff --git a/tests/Report/SeverityTest.php b/tests/Report/SeverityTest.php new file mode 100644 index 0000000..a1dd12c --- /dev/null +++ b/tests/Report/SeverityTest.php @@ -0,0 +1,21 @@ +label()); + } + + public function testWarningLabelIsWarning(): void + { + self::assertSame('warning', Severity::Warning->label()); + } +} diff --git a/tests/Report/SummaryReporterTest.php b/tests/Report/SummaryReporterTest.php new file mode 100644 index 0000000..cc4044c --- /dev/null +++ b/tests/Report/SummaryReporterTest.php @@ -0,0 +1,130 @@ +render([])); + } + + /** + * Twelve findings across four hop lengths (1..4), exercising: ascending + * hop-length bars, the Top-10 cap (one of three 1-hop findings is + * excluded), the hops-desc/origin-asc/param-asc tiebreak (the two 4-hop + * findings share an origin and are ordered by param), both terminal-clause + * forms (`-> external`/`-> truncated` vs `-> X [kind]`), a most-forwarded + * count tie broken alphabetically ($alpha and $beta both appear 4x), and + * findings both under and at/over the limit of 3 hops. + */ + public function testRendersMixedHopLengthsWithAllFourBlocks(): void + { + $findings = [ + $this->finding('beta', 4, 'Demo\Kernel::boot', null, TerminalKind::External), + $this->finding('alpha', 4, 'Demo\Kernel::boot', null, TerminalKind::External), + $this->finding('gamma', 3, 'Demo\ServiceA::run', 'Demo\Mailer::__construct', TerminalKind::Stored), + $this->finding('gamma', 3, 'Demo\ServiceB::run', 'Demo\Logger::log', TerminalKind::Used), + $this->finding('gamma', 3, 'Demo\ServiceC::run', null, TerminalKind::Truncated), + $this->finding('alpha', 2, 'Demo\Two::a', 'Demo\Sink::a', TerminalKind::Used), + $this->finding('beta', 2, 'Demo\Two::b', 'Demo\Sink::b', TerminalKind::Stored), + $this->finding('alpha', 2, 'Demo\Two::c', null, TerminalKind::External), + $this->finding('delta', 2, 'Demo\Two::d', null, TerminalKind::Truncated), + $this->finding('alpha', 1, 'Demo\One::x', 'Demo\Sink::x', TerminalKind::Used), + $this->finding('beta', 1, 'Demo\One::y', 'Demo\Sink::y', TerminalKind::Used), + $this->finding('beta', 1, 'Demo\One::z', 'Demo\Sink::z', TerminalKind::Used), + ]; + + $expected = <<<'TXT' + Chains by length: + 1 hop ### 3 + 2 hops #### 4 + 3 hops ### 3 + 4 hops ## 2 + + Top 10 longest chains: + 4 hops $alpha Demo\Kernel::boot -> external + 4 hops $beta Demo\Kernel::boot -> external + 3 hops $gamma Demo\ServiceA::run -> Demo\Mailer::__construct [stored] + 3 hops $gamma Demo\ServiceB::run -> Demo\Logger::log [used] + 3 hops $gamma Demo\ServiceC::run -> truncated + 2 hops $alpha Demo\Two::a -> Demo\Sink::a [used] + 2 hops $beta Demo\Two::b -> Demo\Sink::b [stored] + 2 hops $alpha Demo\Two::c -> external + 2 hops $delta Demo\Two::d -> truncated + 1 hop $alpha Demo\One::x -> Demo\Sink::x [used] + + Most-forwarded parameters: + 4x $alpha + 4x $beta + 3x $gamma + 1x $delta + + 12 chains total; 5 at or over the limit (limit: 3 hops). + + TXT; + + self::assertSame($expected, (new SummaryReporter(new Thresholds(3, null)))->render($findings)); + } + + /** + * Largest bucket (100) forces scaling; the other three buckets each pin + * one otherwise-ambiguous edge of `round()`: 1 exercises the `max(1, …)` + * floor (its raw scaled value rounds down to 0), 13 has a scaled value + * (5.2) where `ceil()` would disagree with `round()`, and 17 has one + * (6.8) where `floor()` would disagree with `round()`. + */ + public function testScalesBarsWhenLargestBucketExceedsFortyHashes(): void + { + $findings = [ + ...$this->findingsWithHops(1, 100), + ...$this->findingsWithHops(2, 1), + ...$this->findingsWithHops(3, 13), + ...$this->findingsWithHops(4, 17), + ]; + + $expected = 'Chains by length:' . "\n" + . ' 1 hop ' . str_repeat('#', 40) . ' 100' . "\n" + . ' 2 hops # 1' . "\n" + . ' 3 hops ##### 13' . "\n" + . ' 4 hops ####### 17'; + + $output = (new SummaryReporter(new Thresholds(3, null)))->render($findings); + $chainsByLengthBlock = explode("\n\n", $output)[0]; + + self::assertSame($expected, $chainsByLengthBlock); + } + + /** + * @return list + */ + private function findingsWithHops(int $hops, int $count): array + { + $findings = []; + for ($index = 0; $index < $count; $index++) { + $findings[] = $this->finding('p', $hops, "Demo\\Scale::m{$index}", null, TerminalKind::External); + } + + return $findings; + } + + private function finding( + string $param, + int $hops, + string $origin, + ?string $terminal, + TerminalKind $terminalKind, + ): Finding { + return new Finding($param, $origin, $terminal, $terminalKind, $hops, [], $hops + 1, [], []); + } +} diff --git a/tests/Report/TextReporterTest.php b/tests/Report/TextReporterTest.php index 5fa00f1..9c230dc 100644 --- a/tests/Report/TextReporterTest.php +++ b/tests/Report/TextReporterTest.php @@ -7,7 +7,9 @@ use PhpTramp\Chain\Finding; use PhpTramp\Chain\Hop; use PhpTramp\Chain\TerminalKind; +use PhpTramp\Report\Paths; use PhpTramp\Report\TextReporter; +use PhpTramp\Report\Thresholds; use PHPUnit\Framework\TestCase; final class TextReporterTest extends TestCase @@ -43,7 +45,9 @@ public function testRendersStoredChainWithAlignedColumns(): void TXT; - self::assertSame($expected, (new TextReporter(3))->render([$finding])); + $reporter = new TextReporter(new Thresholds(3, null), new Paths('/nonexistent-root')); + + self::assertSame($expected, $reporter->render([$finding])); } public function testRendersTruncatedChainWithNoteLineAndNoTerminal(): void @@ -70,11 +74,13 @@ public function testRendersTruncatedChainWithNoteLineAndNoTerminal(): void hop 2 Demo\B::step($cfg) src/B.php:9 note truncated: 2 implementations - 1 finding (limit: 3 hops). + 1 finding (limit: 2 hops). TXT; - self::assertSame($expected, (new TextReporter(3))->render([$finding])); + $reporter = new TextReporter(new Thresholds(2, null), new Paths('/nonexistent-root')); + + self::assertSame($expected, $reporter->render([$finding])); } public function testRendersSingularFormsForOneHopOneClassOneFinding(): void @@ -94,7 +100,9 @@ public function testRendersSingularFormsForOneHopOneClassOneFinding(): void TXT; - self::assertSame($expected, (new TextReporter(1))->render([$finding])); + $reporter = new TextReporter(new Thresholds(1, null), new Paths('/nonexistent-root')); + + self::assertSame($expected, $reporter->render([$finding])); } public function testExplainAppendsTraceLinesUnderHeader(): void @@ -113,11 +121,13 @@ public function testExplainAppendsTraceLinesUnderHeader(): void explain: Demo\A::go: method:step -> Demo\B::step - 1 finding (limit: 3 hops). + 1 finding (limit: 1 hop). TXT; - self::assertSame($expected, (new TextReporter(3, true))->render([$finding])); + $reporter = new TextReporter(new Thresholds(1, null), new Paths('/nonexistent-root'), true); + + self::assertSame($expected, $reporter->render([$finding])); } public function testExplainDisabledOmitsTraceEvenWhenPresent(): void @@ -129,14 +139,116 @@ public function testExplainDisabledOmitsTraceEvenWhenPresent(): void $trace = ['Demo\A::go: method:step -> Demo\B::step']; $finding = new Finding('p', 'Demo\A::go', 'Demo\B::step', TerminalKind::Used, 1, $chain, 2, [], $trace); - self::assertStringNotContainsString('explain:', (new TextReporter(3))->render([$finding])); + $reporter = new TextReporter(new Thresholds(1, null), new Paths('/nonexistent-root')); + + self::assertStringNotContainsString('explain:', $reporter->render([$finding])); } public function testEmptyFindingsRendersReassuringLine(): void { self::assertSame( "No tramp data found (limit: 3 hops).\n", - (new TextReporter(3))->render([]), + (new TextReporter(new Thresholds(3, null), new Paths('/nonexistent-root')))->render([]), + ); + } + + public function testRendersSingleWarningFindingAndFiltersBelowWarnLimit(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + new Hop('Demo\C::sink', 'Demo\C', 'src/C.php', 13, null), + ]; + $warningFinding = new Finding('p', 'Demo\A::go', 'Demo\C::sink', TerminalKind::Used, 2, $chain, 2, [], []); + $belowWarnLimitChain = [ + new Hop('Demo\C::go', 'Demo\C', 'src/C.php', 5, null), + ]; + $belowWarnLimitFinding = new Finding( + 'q', + 'Demo\C::go', + 'Demo\C::go', + TerminalKind::Used, + 1, + $belowWarnLimitChain, + 1, + [], + [], + ); + + $expected = <<<'TXT' + WARNING $p: 2 pass-through hops across 2 classes + origin Demo\A::go($p) src/A.php:5 + hop 2 Demo\B::step($p) src/B.php:9 + terminal Demo\C::sink($p) src/C.php:13 (used) + + 1 finding (0 errors, 1 warning; limit: 3 hops, warn-limit: 2 hops). + + TXT; + + $reporter = new TextReporter(new Thresholds(3, 2), new Paths('/nonexistent-root')); + + self::assertSame($expected, $reporter->render([$warningFinding, $belowWarnLimitFinding])); + } + + public function testRelativizesAbsoluteFilePathUnderWorkingDirectory(): void + { + $chain = [ + new Hop('Demo\A::go', 'Demo\A', '/tmp/project/src/Demo.php', 5, null), + ]; + $finding = new Finding('p', 'Demo\A::go', 'Demo\A::go', TerminalKind::Used, 1, $chain, 1, [], []); + + $reporter = new TextReporter(new Thresholds(1, null), new Paths('/tmp/project')); + + $output = $reporter->render([$finding]); + + self::assertStringContainsString('src/Demo.php:5', $output); + self::assertStringNotContainsString('/tmp/project', $output); + } + + public function testRendersMixedErrorAndWarningFindingsWithCombinedSummary(): void + { + $errorChain = [ + new Hop('Demo\A::go', 'Demo\A', 'src/A.php', 5, 7), + new Hop('Demo\B::step', 'Demo\B', 'src/B.php', 9, 11), + new Hop('Demo\Y::step', 'Demo\Y', 'src/Y.php', 15, 17), + new Hop('Demo\C::sink', 'Demo\C', 'src/C.php', 13, null), + ]; + $errorFinding = new Finding('p', 'Demo\A::go', 'Demo\C::sink', TerminalKind::Used, 3, $errorChain, 3, [], []); + $warningChain = [ + new Hop('Demo\D::go', 'Demo\D', 'src/D.php', 5, 7), + new Hop('Demo\E::step', 'Demo\E', 'src/E.php', 9, 11), + new Hop('Demo\F::sink', 'Demo\F', 'src/F.php', 13, null), + ]; + $warningFinding = new Finding( + 'q', + 'Demo\D::go', + 'Demo\F::sink', + TerminalKind::Used, + 2, + $warningChain, + 2, + [], + [], ); + + $expected = <<<'TXT' + FINDING $p: 3 pass-through hops across 3 classes + origin Demo\A::go($p) src/A.php:5 + hop 2 Demo\B::step($p) src/B.php:9 + hop 3 Demo\Y::step($p) src/Y.php:15 + terminal Demo\C::sink($p) src/C.php:13 (used) + + WARNING $q: 2 pass-through hops across 2 classes + origin Demo\D::go($q) src/D.php:5 + hop 2 Demo\E::step($q) src/E.php:9 + terminal Demo\F::sink($q) src/F.php:13 (used) + + 2 findings (1 error, 1 warning; limit: 3 hops, warn-limit: 2 hops). + + TXT; + + $reporter = new TextReporter(new Thresholds(3, 2), new Paths('/nonexistent-root')); + + self::assertSame($expected, $reporter->render([$errorFinding, $warningFinding])); } } diff --git a/tests/Report/ThresholdsTest.php b/tests/Report/ThresholdsTest.php new file mode 100644 index 0000000..800f76d --- /dev/null +++ b/tests/Report/ThresholdsTest.php @@ -0,0 +1,69 @@ +severityOf($this->findingWithHops(3))); + } + + public function testSeverityOfIsErrorAboveLimit(): void + { + $thresholds = new Thresholds(3, null); + + self::assertSame(Severity::Error, $thresholds->severityOf($this->findingWithHops(5))); + } + + public function testSeverityOfIsWarningBetweenWarnLimitAndLimit(): void + { + $thresholds = new Thresholds(3, 2); + + self::assertSame(Severity::Warning, $thresholds->severityOf($this->findingWithHops(2))); + } + + public function testSeverityOfIsNullBelowWarnLimit(): void + { + $thresholds = new Thresholds(3, 2); + + self::assertNull($thresholds->severityOf($this->findingWithHops(1))); + } + + public function testSeverityOfIsNullBelowLimitWhenWarnLimitIsNull(): void + { + $thresholds = new Thresholds(3, null); + + self::assertNull($thresholds->severityOf($this->findingWithHops(2))); + } + + public function testConstructorThrowsWhenWarnLimitEqualsLimit(): void + { + $this->expectException(InvalidArgsException::class); + + new Thresholds(3, 3); + } + + public function testConstructorThrowsWhenWarnLimitAboveLimit(): void + { + $this->expectException(InvalidArgsException::class); + + new Thresholds(2, 3); + } + + private function findingWithHops(int $hops): Finding + { + return new Finding('p', 'Demo\A::go', 'Demo\A::sink', TerminalKind::Used, $hops, [], 1, [], []); + } +} diff --git a/tests/fixtures/classifier-smoke/expected-findings.json b/tests/fixtures/classifier-smoke/expected-findings.json index 5da1fc0..48a0ff5 100644 --- a/tests/fixtures/classifier-smoke/expected-findings.json +++ b/tests/fixtures/classifier-smoke/expected-findings.json @@ -1,13 +1,21 @@ { + "limit": 1, + "warnLimit": null, "findings": [ { "param": "config", + "severity": "error", "origin": "Demo\\Controller::handle", "terminal": "Demo\\Mailer::__construct", "terminalKind": "stored", "hops": 3, - "chain": ["Demo\\Controller::handle", "Demo\\ServiceA::process", "Demo\\ServiceB::run", "Demo\\Mailer::__construct"], "classes": 4, + "chain": [ + {"method": "Demo\\Controller::handle", "role": "origin", "file": "src/Demo.php", "line": 11, "forwardLine": 13}, + {"method": "Demo\\ServiceA::process", "role": "hop", "file": "src/Demo.php", "line": 19, "forwardLine": 21}, + {"method": "Demo\\ServiceB::run", "role": "hop", "file": "src/Demo.php", "line": 27, "forwardLine": 29}, + {"method": "Demo\\Mailer::__construct", "role": "terminal", "file": "src/Demo.php", "line": 37, "forwardLine": null} + ], "notes": [] } ] diff --git a/tests/fixtures/config-driven/expected-findings.json b/tests/fixtures/config-driven/expected-findings.json new file mode 100644 index 0000000..f9c62fa --- /dev/null +++ b/tests/fixtures/config-driven/expected-findings.json @@ -0,0 +1,20 @@ +{ + "limit": 1, + "warnLimit": null, + "findings": [ + { + "param": "config", + "severity": "error", + "origin": "Demo\\IncludedOrigin::run", + "terminal": "Demo\\IncludedTerminal::consume", + "terminalKind": "used", + "hops": 1, + "classes": 2, + "chain": [ + {"method": "Demo\\IncludedOrigin::run", "role": "origin", "file": "src/Included.php", "line": 11, "forwardLine": 13}, + {"method": "Demo\\IncludedTerminal::consume", "role": "terminal", "file": "src/Included.php", "line": 19, "forwardLine": null} + ], + "notes": [] + } + ] +} diff --git a/tests/fixtures/config-driven/phptramp.json b/tests/fixtures/config-driven/phptramp.json new file mode 100644 index 0000000..39c6f8d --- /dev/null +++ b/tests/fixtures/config-driven/phptramp.json @@ -0,0 +1,5 @@ +{ + "paths": ["src"], + "exclude": ["src/Excl*.php"], + "limit": 1 +} diff --git a/tests/fixtures/config-driven/src/Excluded.php b/tests/fixtures/config-driven/src/Excluded.php new file mode 100644 index 0000000..7aa2130 --- /dev/null +++ b/tests/fixtures/config-driven/src/Excluded.php @@ -0,0 +1,23 @@ +consume($config); + } +} + +class ExcludedTerminal +{ + public function consume(ExcludedCfg $config): void + { + $config->go(); + } +} diff --git a/tests/fixtures/config-driven/src/Included.php b/tests/fixtures/config-driven/src/Included.php new file mode 100644 index 0000000..ca304c6 --- /dev/null +++ b/tests/fixtures/config-driven/src/Included.php @@ -0,0 +1,23 @@ +consume($config); + } +} + +class IncludedTerminal +{ + public function consume(Cfg $config): void + { + $config->go(); + } +} diff --git a/tests/fixtures/interface-multi-impl/expected-findings.json b/tests/fixtures/interface-multi-impl/expected-findings.json index 3ef9099..45bc8a0 100644 --- a/tests/fixtures/interface-multi-impl/expected-findings.json +++ b/tests/fixtures/interface-multi-impl/expected-findings.json @@ -1,13 +1,18 @@ { + "limit": 1, + "warnLimit": null, "findings": [ { "param": "config", + "severity": "error", "origin": "Demo\\Controller::run", "terminal": null, "terminalKind": "truncated", "hops": 1, - "chain": ["Demo\\Controller::run"], "classes": 1, + "chain": [ + {"method": "Demo\\Controller::run", "role": "origin", "file": "src/Demo.php", "line": 32, "forwardLine": 34} + ], "notes": ["truncated: 2 implementations"] } ] diff --git a/tests/fixtures/interface-single-impl/expected-findings.json b/tests/fixtures/interface-single-impl/expected-findings.json index a8dd48a..bc630f3 100644 --- a/tests/fixtures/interface-single-impl/expected-findings.json +++ b/tests/fixtures/interface-single-impl/expected-findings.json @@ -1,13 +1,19 @@ { + "limit": 1, + "warnLimit": null, "findings": [ { "param": "config", + "severity": "error", "origin": "Demo\\Controller::run", "terminal": "Demo\\RealHandler::handle", "terminalKind": "used", "hops": 1, - "chain": ["Demo\\Controller::run", "Demo\\RealHandler::handle"], "classes": 2, + "chain": [ + {"method": "Demo\\Controller::run", "role": "origin", "file": "src/Demo.php", "line": 24, "forwardLine": 26}, + {"method": "Demo\\RealHandler::handle", "role": "terminal", "file": "src/Demo.php", "line": 16, "forwardLine": null} + ], "notes": [] } ] diff --git a/tests/fixtures/recursion/expected-findings.json b/tests/fixtures/recursion/expected-findings.json index 3841b30..95d616f 100644 --- a/tests/fixtures/recursion/expected-findings.json +++ b/tests/fixtures/recursion/expected-findings.json @@ -1,13 +1,18 @@ { + "limit": 1, + "warnLimit": null, "findings": [ { "param": "config", + "severity": "error", "origin": "Demo\\Looper::go", "terminal": null, "terminalKind": "truncated", "hops": 1, - "chain": ["Demo\\Looper::go"], "classes": 1, + "chain": [ + {"method": "Demo\\Looper::go", "role": "origin", "file": "src/Demo.php", "line": 11, "forwardLine": 13} + ], "notes": ["(recursive)"] } ] diff --git a/tests/fixtures/static-function-mix/expected-findings.json b/tests/fixtures/static-function-mix/expected-findings.json index 5f8e923..758f236 100644 --- a/tests/fixtures/static-function-mix/expected-findings.json +++ b/tests/fixtures/static-function-mix/expected-findings.json @@ -1,13 +1,20 @@ { + "limit": 1, + "warnLimit": null, "findings": [ { "param": "config", + "severity": "error", "origin": "Demo\\entry", "terminal": "Demo\\Sink::__construct", "terminalKind": "stored", "hops": 2, - "chain": ["Demo\\entry", "Demo\\Middle::relay", "Demo\\Sink::__construct"], "classes": 2, + "chain": [ + {"method": "Demo\\entry", "role": "origin", "file": "src/Demo.php", "line": 9, "forwardLine": 11}, + {"method": "Demo\\Middle::relay", "role": "hop", "file": "src/Demo.php", "line": 16, "forwardLine": 18}, + {"method": "Demo\\Sink::__construct", "role": "terminal", "file": "src/Demo.php", "line": 26, "forwardLine": null} + ], "notes": [] } ] diff --git a/tests/fixtures/suppressed-chain/expected-findings.json b/tests/fixtures/suppressed-chain/expected-findings.json new file mode 100644 index 0000000..7bec990 --- /dev/null +++ b/tests/fixtures/suppressed-chain/expected-findings.json @@ -0,0 +1,5 @@ +{ + "limit": 1, + "warnLimit": null, + "findings": [] +} diff --git a/tests/fixtures/suppressed-chain/src/Demo.php b/tests/fixtures/suppressed-chain/src/Demo.php new file mode 100644 index 0000000..5fb66da --- /dev/null +++ b/tests/fixtures/suppressed-chain/src/Demo.php @@ -0,0 +1,42 @@ +consume($config); + } +} + +class AttributeSuppressedTerminal +{ + public function consume(Cfg $config): void + { + $config->go(); + } +} + +class CommentSuppressedOrigin +{ + public function run(Cfg $config): void + { + (new CommentSuppressedTerminal())->consume($config); // phptramp-ignore + } +} + +class CommentSuppressedTerminal +{ + public function consume(Cfg $config): void + { + $config->go(); + } +} diff --git a/tests/fixtures/use-breaks-chain/expected-findings.json b/tests/fixtures/use-breaks-chain/expected-findings.json index 3fa5c02..cfb892a 100644 --- a/tests/fixtures/use-breaks-chain/expected-findings.json +++ b/tests/fixtures/use-breaks-chain/expected-findings.json @@ -1,13 +1,19 @@ { + "limit": 1, + "warnLimit": null, "findings": [ { "param": "config", + "severity": "error", "origin": "Demo\\Controller::handle", "terminal": "Demo\\Service::process", "terminalKind": "used", "hops": 1, - "chain": ["Demo\\Controller::handle", "Demo\\Service::process"], "classes": 2, + "chain": [ + {"method": "Demo\\Controller::handle", "role": "origin", "file": "src/Demo.php", "line": 11, "forwardLine": 13}, + {"method": "Demo\\Service::process", "role": "terminal", "file": "src/Demo.php", "line": 19, "forwardLine": null} + ], "notes": [] } ] diff --git a/tests/fixtures/variadic-decorator/expected-findings.json b/tests/fixtures/variadic-decorator/expected-findings.json index 5ea1462..3339d07 100644 --- a/tests/fixtures/variadic-decorator/expected-findings.json +++ b/tests/fixtures/variadic-decorator/expected-findings.json @@ -1,13 +1,20 @@ { + "limit": 1, + "warnLimit": null, "findings": [ { "param": "args", + "severity": "error", "origin": "Demo\\Outer::run", "terminal": "Demo\\Sink::consume", "terminalKind": "used", "hops": 2, - "chain": ["Demo\\Outer::run", "Demo\\Middle::run", "Demo\\Sink::consume"], "classes": 3, + "chain": [ + {"method": "Demo\\Outer::run", "role": "origin", "file": "src/Demo.php", "line": 11, "forwardLine": 13}, + {"method": "Demo\\Middle::run", "role": "hop", "file": "src/Demo.php", "line": 19, "forwardLine": 21}, + {"method": "Demo\\Sink::consume", "role": "terminal", "file": "src/Demo.php", "line": 27, "forwardLine": null} + ], "notes": [] } ] diff --git a/tests/fixtures/warn-vs-error/expected-findings.json b/tests/fixtures/warn-vs-error/expected-findings.json new file mode 100644 index 0000000..2994760 --- /dev/null +++ b/tests/fixtures/warn-vs-error/expected-findings.json @@ -0,0 +1,37 @@ +{ + "limit": 3, + "warnLimit": 2, + "findings": [ + { + "param": "config", + "severity": "warning", + "origin": "Demo\\TwoHopOrigin::run", + "terminal": "Demo\\TwoHopTerminal::consume", + "terminalKind": "used", + "hops": 2, + "classes": 3, + "chain": [ + {"method": "Demo\\TwoHopOrigin::run", "role": "origin", "file": "src/Demo.php", "line": 11, "forwardLine": 13}, + {"method": "Demo\\TwoHopMiddle::relay", "role": "hop", "file": "src/Demo.php", "line": 19, "forwardLine": 21}, + {"method": "Demo\\TwoHopTerminal::consume", "role": "terminal", "file": "src/Demo.php", "line": 27, "forwardLine": null} + ], + "notes": [] + }, + { + "param": "config", + "severity": "error", + "origin": "Demo\\ThreeHopOrigin::run", + "terminal": "Demo\\ThreeHopTerminal::consume", + "terminalKind": "used", + "hops": 3, + "classes": 4, + "chain": [ + {"method": "Demo\\ThreeHopOrigin::run", "role": "origin", "file": "src/Demo.php", "line": 35, "forwardLine": 37}, + {"method": "Demo\\ThreeHopMiddleA::relay", "role": "hop", "file": "src/Demo.php", "line": 43, "forwardLine": 45}, + {"method": "Demo\\ThreeHopMiddleB::relay", "role": "hop", "file": "src/Demo.php", "line": 51, "forwardLine": 53}, + {"method": "Demo\\ThreeHopTerminal::consume", "role": "terminal", "file": "src/Demo.php", "line": 59, "forwardLine": null} + ], + "notes": [] + } + ] +} diff --git a/tests/fixtures/warn-vs-error/phptramp-args.json b/tests/fixtures/warn-vs-error/phptramp-args.json new file mode 100644 index 0000000..f13906c --- /dev/null +++ b/tests/fixtures/warn-vs-error/phptramp-args.json @@ -0,0 +1 @@ +["--limit", "3", "--warn-limit", "2"] diff --git a/tests/fixtures/warn-vs-error/src/Demo.php b/tests/fixtures/warn-vs-error/src/Demo.php new file mode 100644 index 0000000..028beb7 --- /dev/null +++ b/tests/fixtures/warn-vs-error/src/Demo.php @@ -0,0 +1,63 @@ +relay($config); + } +} + +class TwoHopMiddle +{ + public function relay(Cfg $config): void + { + (new TwoHopTerminal())->consume($config); + } +} + +class TwoHopTerminal +{ + public function consume(Cfg $config): void + { + $config->go(); + } +} + +class ThreeHopOrigin +{ + public function run(Cfg $config): void + { + (new ThreeHopMiddleA())->relay($config); + } +} + +class ThreeHopMiddleA +{ + public function relay(Cfg $config): void + { + (new ThreeHopMiddleB())->relay($config); + } +} + +class ThreeHopMiddleB +{ + public function relay(Cfg $config): void + { + (new ThreeHopTerminal())->consume($config); + } +} + +class ThreeHopTerminal +{ + public function consume(Cfg $config): void + { + $config->go(); + } +}