v0.1.1#5
Merged
Merged
Conversation
A duplicate template declaration was only flagged on the second-seen file, and the pull provider forces the edited file first -- so it was treated as canonical and the duplicate never surfaced on the file the editor was looking at. Flag the duplicate on ALL colliding open declarations via a deterministic cross-file pass (each diagnostic naming the others), independent of iteration order. Registry recording is split out (recordDefinitions) and keeps swallowing the duplicate throw for its first-wins state. Only open files are considered, so an open file is never flagged against its own on-disk copy. Flips the @todo duplicate-template Behat scenario green; updates the WorkspaceAnalyzer test (both files now carry it) and adds a provider test that pulling either file returns the duplicate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GTD on `$users->first()` where `$users: Collection<User>` returned null: worse-reflection (Path 3) can't reflect the receiver's generic body (`T[]`, reified `T`), so the method lookup failed. Resolve it xphp-natively instead, reusing the inlay machinery: GenericResolver infers the receiver class (resolveMethodDeclarationAt) and a new FqnIndex::methodLocation locates the ClassMethod name span (open-doc and filesystem, mapped through ByteOffsetMap). Wired as a new definition path before the worse-reflection fallback; plain receivers still use Path 3. Flips the @todo generic-method-jump Behat scenario green; adds a handler unit test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Update features/README -- no @todo scenarios remain; every scenario runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Extend the post-monomorphization argument-type checker (V1, constructors only) to the remaining call shapes -- instance method calls `$obj->m(…)`, static calls `C::m(…)`, and free-function calls `fn(…)` -- under a new `xphp.arg-mismatch` diagnostic code. The constructor surface keeps its `xphp.ctor-arg-mismatch` code and behaviour byte-for-byte. Inference stays conservative and adds "simple locals": a `$var` assigned exactly once, before use, in the same function scope from a literal or `new` flows into both argument typing and method-call receiver typing (`$users = new Collection<User>(); $users->add(42);`). Reassigned, forward-assigned, or non-inferable variables are skipped -- no false positives. A known scalar argument against a class-typed parameter is now a definite mismatch (a PHP scalar never satisfies a class hint). ConstructorArgumentChecker is renamed/generalised to CallArgumentChecker, reusing its existing renderType/isSatisfied/substitution machinery over the in-pass `$files` ASTs (no per-URI resolver), so the workspace pass stays self-contained and unit-testable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A single workspace pass already computes diagnostics for every open document, but the provider previously returned only the linted file's -- so editing `Box.xphp` left a stale `Use.xphp` until the user touched it. The provider now folds the per-file syntax + cross-file passes into one `computeAllOpenDiagnostics()` that yields a per-URI map, and on the push path pushes `textDocument/publishDiagnostics` for every OTHER open document whose diagnostics changed since it was last published. A per-URI signature guard avoids re-publishing unchanged dependents on edit storms; the linted document itself is left to the engine. No dependency index is needed -- the pass already visits the whole open set. The ClientApi is injected (nullable) so pull-mode / unit contexts skip the broadcast. Behat drives the real push path deterministically: the harness builds the tester with a 0ms debounce, starts the diagnostics engine service, edits a dependency, and pumps the cooperative loop until the dependent's re-publish lands in the transmitter. Unit tests pin the precise push accounting (broadcast fires, unchanged dependents aren't re-published, the linted doc isn't self-broadcast) against a real ClientApi. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`xphp.bound` diagnostics were dead-ends -- no fix offered. They now carry a structured `data` payload (added to the neutral Diagnostic + plumbed through DiagnosticTranslator into LSP `Diagnostic.data`, which clients round-trip on codeAction). The WorkspaceAnalyzer computes it at the point of the violation, where the full context is available: which type-param / bound was violated (via Registry::definition + the hierarchy), the offending concrete type, the source range of the offending type-argument (generic clauses strip to equal-length whitespace, so offsets are 1:1 with the original text), workspace types that satisfy the bound, and -- for an editable open class -- where to add an `implements` clause. A new data-driven BoundErrorCodeActionProvider turns those facts into two quick-fixes: "Change type argument to <Candidate>" (one per bound- satisfying workspace type, works for scalar concretes too) and "Add implements \Bound to <Concrete>" (a cross-file edit, when the concrete is an editable open class). Registered alongside the existing code-action providers. Behat pulls the real diagnostic and feeds it back through codeAction so the data round-trip is exercised end-to-end; unit tests drive the whole analyzer -> translator -> provider chain incl. the multi-type-param case (the fix targets the correct argument). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two long-standing gaps in the semantic-token emitter, called out in the class docblock: Non-ASCII (the real fix): emit() reported token lengths in BYTES, but LSP measures them in UTF-16 code units. A token covering a multi-byte char (e.g. a "café" string literal) came out one unit too long, so the highlight over-ran. emit() now converts the covered byte span to its UTF-16 length via the same conversion offsetToPosition already uses (exposed as PositionMap::lengthInUtf16). For the all-ASCII common case this is a no-op, so existing behaviour is unchanged. Interpolation: already handled -- PHP's tokenizer decomposes a double-quoted "… $x …" into literal T_ENCAPSED_AND_WHITESPACE slabs and the inner T_VARIABLE, which the token pass classifies as `string` and `variable` respectively, so the variable highlights inside the string. The class docblock claimed it was emitted "as a single span"; that was stale. This corrects the docblock and locks the behaviour with a test + acceptance scenario. Unit tests pin the UTF-16 length (a `"café"` literal is 6 units not 7 bytes; a 4-byte emoji counts as 2 surrogate units) and the interpolation decomposition; a Behat scenario covers the interpolated-variable case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
documentHighlight reported every occurrence as DocumentHighlightKind::TEXT, so clients that colour a symbol's definition / writes differently from its reads (e.g. VS Code) couldn't. Each occurrence is now classified via a parent-aware AST walk (DocumentHighlightKindResolver): a declaration name or an lvalue (assignment LHS, foreach target, ++/--, property/static- property write) is WRITE; every use site is READ. Location ranges are in original-source coordinates while AST offsets are in the xphp-stripped source, so the resolver maps stripped->original via the document's ByteOffsetMap to line the keys up; unresolved occurrences default to READ so a highlight is never dropped. Unit tests cover the lvalue rules (variable read/write, foreach, inc, property write vs read, declaration vs use); the handler test asserts the class declaration is a WRITE and its two uses are READs; a Behat scenario covers the same end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add an authoritative FQN -> list-of-declaring-records map (`filesystemDecls`) alongside the existing single-value maps, plus a `nearestDecl()` selector that picks the declaration nearest a requesting document (longest shared directory prefix; ties and a null origin fall back to shortest-path-then-alphabetical). Resolution methods (pathFor, classLikeFor, functionFor, locationForFqn, methodLocation, boundsForGenericClass, locationByShortName) gain an optional `?string $origin = null`. With origin null they reduce to the prior global tiebreak, so this is behavior-identical until callers start threading the requesting URI (next phase) -- the full suite (914) and all FqnIndex tests stay green. New tests lock the proximity selection: duplicate FQNs resolve to the requester's own package; null origin keeps shortest-path; locationForFqn returns the near copy's position; open docs still win over any on-disk copy. This is the groundwork for indexing fixture trees without reintroducing the wrong-file go-to-definition the test/fixture skip prevents. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a per-request origin anchor on FqnIndex (withOrigin/currentOrigin) and an OriginTrackingMiddleware that sets it from each request's textDocument.uri (clearing it for workspace-wide requests). selectDecl consults the anchor when no explicit origin is passed. This is how proximity reaches the layers that have no document context: the static GenericResolver inference chain and the worse-reflection SourceCodeLocator both call origin-less pathFor/classLikeFor, which now fall back to the anchor -- so duplicate-FQN resolution becomes proximity-aware end-to-end without threading an origin parameter through every resolver and call site. The explicit ?origin params from the prior commit remain as direct overrides. Safe because requests dispatch one-at-a-time on the loop and the xphp handlers resolve synchronously; the async diagnostics path doesn't use anchor-based resolution. Middleware unit tests cover set-from-request, clear-on-workspace-request, notifications, and chain delegation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The bound-check hierarchy previously absorbed every warmed filesystem file. Once fixture trees are indexed, the same FQN is declared in many packages, and TypeHierarchy::fromAstPerFile keys ancestry by FQN -- so duplicate declarations collide and a class's ancestry could come from the wrong copy. A filesystem file now enters the hierarchy only when it is the NEAREST declarer (to the document being linted) of a class it defines -- `pathFor(fqn, currentUri)` is the proximity oracle, and open buffers win. Pure usage sites (no class declaration) contribute no ancestry and are skipped. Each FQN is therefore single-sourced from its nearest definition. A new test pins both directions: a `Box<Tag>` instantiation in pkgA (whose Tag implements \Stringable) is clean, while the identical instantiation in pkgB (whose same-FQN Tag does not) is flagged -- the hierarchy used each file's local Tag. Exposes WorkspaceAnalyzer::classFqnsIn for the provider. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
With proximity-aware resolution in place, the blunt `test/fixture` / `tests/fixtures` walk exclusion is no longer needed -- duplicate FQNs are now disambiguated by nearness to the requesting document instead of by hiding whole subtrees. Remove SKIP_NESTED and its iterator() filter branch (SKIP_DIRS for .git/vendor/node_modules/var/build stays). This is what fixes the reported prod gap: editing a file inside a fixture tree (e.g. the xphp compiler's own test/fixture/compile/**) now sees its closed sibling declarations -- go-to-definition resolves Box/Tag without opening them, and the bound check no longer fires the spurious "not in the source set" error. The two tests that locked the skip now assert the inverse: fixture-tree declarations are indexed and resolve to the copy nearest the requester. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Spell out what the acceptance suite intentionally does NOT cover and why: the harness runs with no rootUri (100% in-memory, no disk, parallel-safe), so the filesystem layer -- the FQN index, proximity-aware resolution + the origin middleware, closed-file GTD/hover, the warmers, watcher invalidation -- plus non-ASCII semantic-token length are unit-tested rather than driven here. Also notes the in-memory-drivable gaps left unscripted (lifecycle notifications, codeAction/resolve round-trip). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`new X(...)` is a call to the constructor, but the source text is the class name, never `__construct` -- so the reference finder's member-name match missed every instantiation. Find Usages, the "Show references" code lens, and document-highlight on a `__construct` declaration therefore reported no instantiation sites. The method-target collector now also matches `New_` nodes whose instantiated class is (or inherits the constructor from) the target class, yielding the class-name token as a constructor reference. The filesystem pre-filter learns the class short name for `__construct` targets so files that say `new X()` (and never `__construct`) aren't skipped. Rename is unaffected: RenameProvider already drops any reference whose covered text differs from the member name, so renaming `__construct` never touches the `X` in `new X()`. (Go-to-definition on `new X()` still lands on the class, matching PhpStorm's native behavior.) Covered by unit tests (instantiations counted across files; the reference spans the class-name token) and Behat scenarios for both Find Usages and the constructor code-lens count. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three roadmap items shipped on this branch but were still listed as pending: argument-type checker V2 and cross-file diagnostic broadcast (Planned) and bound-error fix-its (Exploratory). Move them out of those sections, refresh the Shipped lane, and add a "Recently shipped" record that also notes the refinements that landed (proximity-aware FQN resolution + fixture indexing, constructor usages of `new X()`, semantic-token interpolation/non-ASCII, document-highlight read/write). Mirror the same changes in the README overview mindmap. Planned now holds exactly the still-pending small capabilities (prepareRename, selectionRange, documentLink). 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.