Feature: xphp check#19
Closed
math3usmartins wants to merge 16 commits into
Closed
Conversation
Introduce the XPHP\Diagnostics namespace: a string-backed Severity enum (Error/Warning/Notice with isFailing()), a DiagnosticSource enum (xphp/phpstan), a SourceLocation (file/line/optional column), the immutable Diagnostic, and a mutable DiagnosticCollector (add/all/hasErrors/count). Pure value objects with no pipeline wiring yet — the foundation for the forthcoming `xphp check` command's structured, collect-all diagnostics. Unit tests cover severity gating, collector ordering/error detection, and defaults (100% mutation score over the new files). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tics sink Thread an optional DiagnosticCollector through the bound-validation path (Registry ctor -> recordInstantiation -> validateBounds -> checkBounds). When absent (xphp compile) violations throw exactly as before, byte-identical; when present each violation is appended as a Diagnostic -- located at the instantiation site captured from the AST node in RegistryCollector -- and recording continues so all violations surface in one run. The user-facing message now comes from a single shared boundViolationMessage() builder so the throw text and the diagnostic text can never drift. Tests cover collect-vs-throw, byte-identical and exact message text, multi-violation collection, and AST-derived source line (100% mutation score over the diff). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add Compiler::check(): parse, build the type hierarchy, collect definitions, validate defaults-against-bounds, collect instantiations (bounds + missing type arguments), and report instantiations of undefined templates -- gathering every error into a DiagnosticCollector and halting before specialization/emit, so a partially-invalid registry never reaches the fixed-point loop. Extends the optional-collector seam to the padding path (missing required type argument), validateDefaultsAgainstBounds (per-parameter, continue-on-error), and a new collectUndefinedTemplates pass. Each reused message is built by a single shared helper so the throw (compile) and diagnostic (check) text stay byte-identical. The parse loop is factored into parseAll(), reused by compile() and check(); compile()'s undefined-template throw now routes through the shared builder. Duplicate-definition is intentionally not part of the seam: RegistryCollector's already-recorded guard makes the class-template path unreachable and surfacing it would change compile-mode semantics -- deferred. Variance and method-level generic checks remain fail-fast (not yet part of the check pass). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…hase Move the variance-position check out of the parser into a Registry phase (validateVariancePositions) over collected definitions, wired into compile() (fail-fast, byte-identical first-violation throw) and check() (collects every violation across all definitions, each located at the offending member). VariancePositionValidator now accumulates violations behind a static assertPositions facade that throws the first when no collector is given or emits a diagnostic per violation when one is. The parser-level variance-position tests move to a dedicated VariancePositionPhaseTest (compile-mode throws via data provider + check-mode collect/location), and the check integration suite gains a variance_violation fixture covering the compile-throw and check-collect paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extract the inner-variance composition walk out of Registry into a dedicated InnerVarianceValidator (mirroring VariancePositionValidator): it accumulates violations behind a static assertComposition facade that throws the first when no collector is given (compile, byte-identical) or emits a diagnostic per violation (check), each located at the offending member. Registry's validateInnerVariance is now a thin delegate. To avoid double-reporting a direct +T/-T misuse, the position check now returns which definitions it flagged and the inner-variance pass skips them -- matching compile-mode, where the position check fails fast before inner-variance runs. Both passes are wired into Compiler::check(); compile() is unchanged. Adds inner-variance check fixture + collect-mode, gating, and null-file tests (100% mutation score over the new validator and the diff). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run the variance-position check before the defaults-vs-bounds check so a class with both surfaces the variance error first in compile-mode — the order it surfaced when the check lived in the parser. Merge the stacked docblocks on the two variance delegate methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a DiagnosticRenderer interface and three implementations for `xphp check` output: TextRenderer (human-readable blocks), JsonRenderer (a stable documented JSON contract), and GithubRenderer (Actions workflow-command annotations with proper escaping). Unit tests pin each format exactly, including the JSON shape and GitHub escaping (100% mutation score over the renderers). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wire a CheckCommand (`xphp check <source> [--format=text|json|github]`) into the console alongside compile, sharing one Compiler. It runs the validate-only pass, renders diagnostics in the chosen format, and exits 0 (clean) / 1 (errors) / 2 (bad source dir or unknown format). Compiler::check() now parses each file in its own try/catch: a file that fails to parse (PHP syntax error or an xphp-specific parse rejection) is reported as a diagnostic and skipped, so the remaining files are still checked. Tests drive the command via CommandTester across all formats/exit codes, and a parse_error fixture proves a valid file's bound violation is still reported alongside two unparseable files. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move the file read out of check()'s per-file try so an I/O failure surfaces as itself rather than being mislabeled xphp.parse_error; only parsing is treated as a recoverable per-file diagnostic. Clarify the parse-error line comment re nikic's -1 sentinel. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an `xphp check` section to the errors reference (formats, exit codes, per-file parse resilience, and the stable diagnostic codes for the json/github formats) and a short pointer from the README quick start. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…pile` Spell out the scope consequence: a clean `check` does not guarantee a clean `compile`, because method/function/closure-level generic checks (and the specialization-loop guards, by design) are not run by `check` yet. Advise keeping `compile` in the build pipeline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run GenericMethodCompiler in a new validate-only mode from Compiler::check(): process(emit: false) walks the call sites for their bound/missing-arg checks and the duplicate-function / $this-capture / static-closure rejections, threading the optional DiagnosticCollector + source locations through the (already collector-aware) Registry::checkBounds/padArgsWithDefaults and the in-process throws, while suppressing the specialize/strip/finalize side-effects. xphp compile is unchanged (default emit: true, no collector -> byte-identical fail-fast). This makes `xphp check` a validation-superset of `xphp compile`: a class-level and a method-level generic error are now both reported in one run. New diagnostic codes xphp.duplicate_generic_function / xphp.closure_this_capture / xphp.static_closure; bound + missing-arg reuse the existing codes. Fixtures + CheckPassIntegrationTest cover each new collected diagnostic (with file:line), the both-passes-in-one-run guarantee, and byte-identical-compile guards. Docs updated: check now covers all generic validation; only the specialization-loop guards (depth cap, hash collision) remain compile-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a closure_static fixture + check-collect and compile-throws tests for the generic static-closure rejection (xphp.static_closure), matching the symmetry of the other method-level checks (the collect path was previously untested). Add the three new method-level codes to the errors-doc table, and correct the validate-only comments (the discarded per-file AST may carry in-place call-site rewrites; templates are deep-cloned so nothing shared is mutated). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Bump phpstan.neon from level 7 to level 9 and make src/ clean at it: - CompileCommand / CheckCommand: narrow getArgument()/getOption() (typed mixed) to string via is_string() instead of a blind (string) cast — the inputs are always strings (required argument / option with a string default), so behavior is unchanged; level 9 just rejects casting mixed. - Specializer: annotate the ATTR_GENERIC_ARGS array as list<TypeRef> so array_map infers the callback's parameter type (level-9 callable-variance check). Full suite green; src/ clean at level 9. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The check command is unit-tested in-process via CommandTester, but nothing exercised the real binary: its autoload wiring, the process exit code the shell sees, or the rendered github/json/text output on stdout. The released PHAR was only smoke-tested with `list`, never `check`. Add test/smoke/check.sh — a parameterized POSIX script (XPHP_BIN selects the binary) that runs `check` against the clean and multi_error fixtures and asserts the 0/1/2 exit contract plus that every renderer emits and json stays well-formed. Wire it in: - Makefile: `test/check` target. - ci-core.yml: a dedicated `xphp check (self-test)` job running `make test/check` against bin/xphp. - release.yml: a post-build step running the same script against dist/xphp.phar, so a packaged binary that can't gate fails the release before upload. No src/ changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
No description provided.