Repository navigation
Phase 1 — Usage classifier + index - #2
Merged
Merged
Conversation
Parse located files into a whole-project MethodIndex via php-parser (NameResolver + ParentConnectingVisitor). Records FQMNs (Ns\Class::method for methods, Ns\func for functions), structural param facts (name, position, by-ref, variadic, declared type), and per-class hierarchy facts (parent, interfaces, traits). Usage classification (fate/forwards) lands in Task 1.4; params are placeholder-classified Unused for now. Beyond the plan's named file list this adds three internal helpers the typed design needs: CalleeKind enum, ClassInfo DTO for the class-facts, IndexingVisitor, and a ParseException for the exit-2 parse-error path.
Classify each parameter as PureForward / Used / ByRefTerminated / Unused per the frozen core semantics: pure whole-argument forwarding is a hop, any other occurrence (property/method access, assignment, return, interpolation, closure use() or arrow-fn capture) is a terminal use. By-ref receipt and promoted/stored constructor properties terminate. Variadic spread forwarding stays a hop; non-variadic spread is a conservative use. Named and positional forward arg keys are recorded, and forward sites are kept even on Used params (Phase 4 needs the call-site lines). Classification is deferred to after the whole-project traversal so call targets inside bodies are already fully qualified by NameResolver; the IndexingVisitor now collects pending nodes and the Indexer classifies them. Adds an internal ForwardCollector scope-aware visitor.
Wire the Phase 1 pipeline into Application: parse args -> locate files -> index/classify -> render the classified method index as JSON under --dump-index (methods with no params omitted; forwards rendered as kind:callee@argKey). Bad args and parse errors report to stderr and exit 2; default chain reporting still returns the Phase 1 not-implemented notice. Refresh the help text to match the real flag surface. Add the data-driven FixtureTest harness: it runs the real CLI over every tests/fixtures/<case>/src and compares normalized --dump-index output to the case's expected-index.json, plus the classifier-smoke fixture (the README 3-hop chain Phase 2 will reuse for the first end-to-end finding).
Two quality cleanups from a review pass, both behavior-preserving: - Connect AST parents once per method body in classify() instead of re-running ParentConnectingVisitor for every parameter; the per-param body walks now share the connected tree. - Collapse the four repeated 7-argument ParamInfo constructions in classifyParam() into a single tail return that computes fate/forwards in a branch.
Document the composer scripts that mirror the CI jobs (cs/stan/md/test via composer check) and, separately, the diff-scoped Infection mutation gate (composer infection:diff, minMsi 80) that composer check does not cover and that gates PRs — run it before opening a PR.
Adapt the code-style contract from the simple-feed-reader house style: intent-revealing names, small single-purpose functions, few parameters and no behaviour-selecting boolean flags, guard clauses, immutable final value objects, typed namespaced exceptions, DRY, tests-as-production-code, and the mechanically enforced PSR-12 / PHPStan-max / PHPMD-codesize rules.
The diff-scoped Infection PR gate (minMsi 80) failed at 78%. The escaped mutants were genuine test-suite gaps, now covered: - ArgvParser: assert every flag actually sets its Options field (the untested match arms for warn-limit/format/git-base/baseline/ generate-baseline/explain/changed-only/dump-index/help/version), plus error-message content. - Options: pin every constructor default (new OptionsTest). - FileLocator: an existing file passed as --folder throws; uppercase .PHP is matched case-insensitively. - UsageClassifier/ForwardCollector: static, nullsafe, plain-method, and function forwards record the right callee/receiver; dynamic function and dynamic new terminate as Used; first positional forward records key 0; new records the resolved class; abstract (bodyless) method param is Unused; union-typed param has null type; and a standalone classify() test proves the classifier connects its own parents. - Indexer: parse-error message names the file and reason. Also drop ParamInfo's now-unused constructor defaults (the classifier always passes every argument), removing dead defaults the mutator flagged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements Phase 1 of
docs/plan.md. Closes #1.Deliverable:
phptramp --folder <dir> --dump-indexprints the classified method index as JSON — the semantic heart (the parameter usage classifier) shipped and testable before any chain logic exists.What landed (strict fixture-first TDD, one task per commit)
ArgvParser+ immutableOptions+InvalidArgsException(--flag valueand--flag=value, validation → exit 2)FileLocator— recursive*.php, deduped, sorted, realpath-normalizedIndexer→MethodIndexvia php-parser (NameResolver + ParentConnectingVisitor): FQMNs, structural param facts, per-class hierarchy facts, parse-error → exit 2UsageClassifier— classify each paramPureForward/Used/ByRefTerminated/Unusedper the frozen core semantics--dump-indexwiring inApplication+ data-drivenFixtureTestharness +classifier-smokefixture (the README 3-hop chain)Plus a follow-up
refactor:commit from a cleanup review pass (connect AST parents once per method; collapse repeatedParamInfoconstruction).Quality
composer check(PHPCS PSR-12 + PHPStan level max + PHPMD codesize + PHPUnit) green on every commit — 46 tests, 151 assertions.Notes / judgment calls (also in commit messages)
b(...$p)where$pisn't variadic) is classified as a conservativeUsed, not a forward — the plan's algorithm only names variadic spread as a hop.CalleeKind,ClassInfo,IndexingVisitor,ForwardCollector,ParseException.receiverHint,ClassInfosupertypes) were deliberately deferred to the Phase 2 design (CallResolver/ClassHierarchy) rather than abstracted speculatively here.