LSP and PhpStorm MVP: minimum features and performance#12
Merged
Conversation
Wires the protocol-level plumbing for `textDocument/semanticTokens/full`
end-to-end so that subsequent slices can land AST-walking logic against
a known-good baseline. No user-visible coloring yet: the visitor emits
zero tokens, the client receives an empty packed array, the file
renders unchanged.
What's in this slice:
- `SemanticTokens/TokenLegend`: static legend (token types +
modifiers) advertised at `initialize`. Standard LSP-spec subset
-- both PhpStorm and VS Code default themes recognise every entry,
so no per-editor color config required. Slice 3's `typeParameter`
(xphp `T` references) is already in the list.
- `SemanticTokens/TokenSpec`: pre-encoding value object.
- `SemanticTokens/Encoder`: encodes a list of specs into the
delta-encoded integer array LSP 3.17 expects. Sorts defensively
in source order before encoding (visitor traversal may not match
source order on nested ClassLike bodies). Drops unknown token
types fail-soft.
- `SemanticTokens/AstVisitor`: skeleton with the right constructor +
`visit()` signature. Returns an empty list in this slice.
- `XphpSemanticTokensHandler`: the LSP handler.
- Advertises capability as an array `{legend: ..., full: true}`,
not a class instance, to dodge the phpactor JSON serializer's
empty-options-class quirk that IntelliJ's LSP4J rejects -- same
workaround `XphpHoverHandler` documents for `hoverProvider`.
- Accepts `array $params` instead of a typed `SemanticTokensParams`
class because phpactor's `LanguageSeverProtocolParamsResolver`
only auto-binds classes in its own namespace, and the library
doesn't publish a `SemanticTokensParams`. The
`PassThroughArgumentResolver` fallback hands us the raw params
map; `extractUri()` reads `textDocument.uri` defensively.
- `LspDispatcherFactory`: registers the new handler in the
`Handlers(...)` list at line 252.
Tests (22 new):
- `EncoderTest` (8 tests) covers: empty input, single-token absolute
positioning, same-line column-delta, cross-line absolute-column,
unknown-type drop, modifier bitfield, defensive re-sort on
unsorted input, three-token column-delta chain.
- `TokenLegendTest` (8 tests) covers: known/unknown type indexes,
`typeParameter` presence, modifier bitfield encoding, unknown
modifier silent drop, non-empty legend invariants.
- `XphpSemanticTokensHandlerTest` (6 tests) covers: capability
shape, methods map, unknown document -> empty, known document
-> empty (slice-1 baseline), malformed params -> empty,
object-shaped textDocument (defensive).
Verification:
make -C tools/lsp test -> 446 tests / 1277 assertions / 0 failures
Mutation testing for these new files is in scope for slice 2 once a
realistic AST walk has tests. Full-suite LSP mutation testing
continues to be OOM-bound on the GitHub runner (task #90); a scoped
local re-run is a follow-up step.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ngs / comments / class+method names)
Two-pass classification in AstVisitor.
Pass 1: PHP-token scan. PhpToken::tokenize the ORIGINAL source
(non-strict mode -- xphp's `<T>` would throw under TOKEN_PARSE);
emit specs for the token classes that don't need AST context:
- T_VARIABLE -> variable
- T_LNUMBER / T_DNUMBER -> number
- T_CONSTANT_ENCAPSED_STRING /
T_ENCAPSED_AND_WHITESPACE -> string
- T_COMMENT / T_DOC_COMMENT -> comment
- 60+ keyword tokens (T_CLASS, T_FUNCTION,
T_PUBLIC, T_RETURN, T_NEW, T_INSTANCEOF,
T_NAMESPACE, T_USE, T_OPEN_TAG, ...) -> keyword
Token offsets are byte-indexed in the original source, so
positions feed directly into PositionMap.
Pass 2: AST walk. nikic-parsed tree, positions byte-indexed in the
STRIPPED source. ByteOffsetMap translates back to original
coordinates. Emits the identifier kinds the token scan can't
classify alone:
- ClassLike->name (Class_/Interface_/Trait_/Enum_)
-> class / interface / enum
- ClassMethod->name -> method
- Function_->name -> function
- Param->var (the inner Variable) -> parameter
(re-classifies the
`variable` spec from
pass 1 at the same
span)
PropertyItem->name is already covered by pass 1's T_VARIABLE -- a
property declaration `public string $name` has `$name` tokenized as
T_VARIABLE.
Skip list (intentional, deferred):
- Class-reference names (Name nodes used as types) -- needs use-
alias resolution to avoid mis-classifying `\Foo\Bar` segments.
Slice 3 or 4.
- Double-quoted string interpolation -- the T_VARIABLE inside
`"hello $name"` already classifies as `variable` (correctly);
the surrounding string slabs come back as
T_ENCAPSED_AND_WHITESPACE which is already in the legend.
- Heredoc / nowdoc -- tokenizes correctly but the full coverage
is out of scope for slice 2.
Tests:
- AstVisitorTest (15 cases, all assert (line, char, length, type)
matches the expected substring):
- keywords (namespace, class, public, function, return)
- $variables
- numeric literals (int + float)
- single-quoted strings
- line / block / doc comments
- class names (class, interface, enum)
- method + top-level function names
- parameter re-classification
- edge cases: empty file, open-tag-only,
Box<T> position translation across the xphp angle-bracket strip
- XphpSemanticTokensHandlerTest updated: the slice-1 "empty data"
baseline now expects non-empty data + a multiple-of-5 packed
array length per LSP spec.
Plugin-side: zero changes. XphpLspServerDescriptor's existing
`LspCustomization()` opt-in already enables LspSemanticTokensSupport
along with the other Lsp*Support customizers -- documented in the
existing comment block at L46-72.
Verification:
make -C tools/lsp test -> 463 tests / 1303 assertions / 0 failures
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends AstVisitor with two new classification paths:
1. **Token-stream state machine** for angle-clause contents. Tracks
a `<>` depth counter across the original-source token stream.
`<` opens a clause when (a) the previous significant token is
T_STRING (so `$a < $b` and `$x < 5` don't open one), AND (b) the
next non-trivial token starts with `[A-Z_]` or is a `\` (FQN
start). Inside a clause, every identifier-like token --
T_STRING (`T`, `Plastic`) plus T_NAME_QUALIFIED /
T_NAME_FULLY_QUALIFIED / T_NAME_RELATIVE (`App\Foo`,
`\Stringable`) -- emits as `typeParameter`. Depth-counted so
nested `Box<Lst<T>>` still classifies the inner `T`.
PhpToken's `$id` is always int (single-char tokens have ord-byte
ids), so the "named token vs single-char token" distinction uses
`$id >= 256` -- the bug that almost made slice 3 a no-op until
I caught it.
2. **AST scope-stack** for reified-T detection. Enters a ClassLike
with ATTR_GENERIC_PARAMS, pushes the type-param names onto a
stack. Any single-segment Name node whose text matches a
stacked name emits `typeParameter`. Covers:
- `new T(...)` -- Name in New_->class
- `T::class` -- Name in ClassConstFetch->class
- `instanceof T` -- Name in Instanceof_->class
- `T::method(...)` -- Name in StaticCall->class
leaveNode pops the stack so nested ClassLikes don't leak T into
outer scopes.
Reified-T is gated by ATTR_GENERIC_PARAMS, not by a static
short-uppercase heuristic. So `new Foo()` in a plain (non-generic)
class doesn't paint Foo as typeParameter -- counter-example to the
"reified-T-everywhere" trade-off the abandoned TextMate branch
documented.
Tests (12 new in AstVisitorTest, total 475/1318):
Audit-form positives:
- form 1: class Box<T> -- `T` typeParameter
- form 2: class StringableBox<T: \Stringable> -- T + \Stringable
both typeParameter (the FQN comes back as one
T_NAME_FULLY_QUALIFIED token with the leading backslash)
- form 6: new Box<Plastic>() -- `Plastic` typeParameter
- form 9: class Pair<K, V> -- both K and V typeParameter
- nested: Box<Lst<T>> -- both inner identifiers typeParameter
- form 10: new T() inside generic class -- typeParameter
- form 11: T::class inside generic class -- typeParameter
- form 12: instanceof T inside generic class -- typeParameter
Counter-examples:
- $a < $b -- comparison, no typeParameter
- $x < 5 -- comparison, no typeParameter
- if ($size < count($items)) -- lowercase-after-< rejects clause
- new Foo() in a non-generic class -- no typeParameter
(scope-stack gating, not heuristic)
Forms 3 (T $item), 4 (T[] $items), 5 (?T), 7+8 (T return type) are
emitted naturally by either the token scan (inside-clause emits)
or the existing slice-2 paths. Method-level type-params
(ATTR_METHOD_GENERIC_PARAMS) are deferred -- the AstVisitor only
pushes ClassLike-level params today; a follow-up will extend the
scope stack to ClassMethod scope.
Docs:
- tools/lsp/README.md: semanticTokensProvider added to advertised
capabilities; test count bumped to 475/1318.
- docs/roadmap.md: "LSP -- semantic tokens" added to Shipped
Tooling; removed from Long-term LSP capabilities (medium effort).
Verification:
make -C tools/lsp test -> 475 tests / 1318 assertions / 0 failures
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… dispatch
Production logs from the first install showed every
`textDocument/semanticTokens/full` request returning `{"data":[]}`
even though the wire handshake advertised the capability and
PhpStorm was sending requests for every opened file.
Root cause: phpactor's `HandlerMethodRunner` (lib/Core/Handler/HandlerMethodRunner.php)
does `array_values($args)` on the resolved arguments and splats them
positionally via `$handler->$method(...$args)`. For a method
typed-untyped (so the chain falls to `PassThroughArgumentResolver`),
`$args` becomes `array_values($request->params)` -- the OUTER
array's positional values, NOT the wrapper.
So for params `{"textDocument": {"uri": "..."}}` the handler was
being called as
$handler->semanticTokensFull(['uri' => '...'], $cancelToken)
not
$handler->semanticTokensFull(['textDocument' => ['uri' => '...']])
`extractUri` then read `$params['textDocument']` -> null and the
handler bailed to the empty-tokens path.
Fix:
- Rename the parameter from `$params` to `$textDocument` and
document the actual shape (the UNWRAPPED TextDocumentIdentifier
map).
- `extractUri` now reads `$params['uri']` first (the production
positional shape) and falls back to the wrapped shape for
test-path callers that hand the handler the full LSP params map.
Tests:
- `testKnownDocumentReturnsNonEmptyTokenStream` now exercises the
positional shape (matches what HandlerMethodRunner produces).
- New `testAcceptsWrappedParamsShapeForBackwardsCompatibility`
locks the wrapped-shape path.
- `testUnknownDocumentReturnsEmptyTokens` updated to positional.
- `testTextDocumentAsWrappedObjectIsAlsoAccepted` renamed for
clarity (was already covering the wrapped-stdClass branch).
Verification:
make -C tools/lsp test -> 476 tests / 1320 assertions / 0 failures
make -C tools/lsp build/phar -> 4.3M PHAR
make -C tools/phpstorm-plugin buildPlugin --rerun-tasks
-> 4.2M plugin zip
Tested in prod: server now returns non-empty packed token arrays
for every semanticTokens/full request.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
….) as keywords PHP tokenizes `null`, `true`, `false`, `void`, `mixed`, `never`, `iterable`, `self`, `parent`, and the scalar type names `int`/`string`/`bool`/`float`/`array`/`object`/`callable` as bareword T_STRING -- not as their own T_* keyword constants. The visitor's keyword map only catches explicit T_* constants, so these reserved words fell through unclassified. Result: PhpStorm rendered `null` with the default text color instead of the keyword color, a visible regression vs the native PHP highlighter. Prod-log confirmation: decoding the packed token stream for `return $this->items[0] ?? null;` showed `return` (keyword) and `0` (number) emitted, then a jump to the next line -- the `null` span was absent. Fix: case-insensitive lookup against a small set of reserved-word identifiers when T_STRING is otherwise unclassified. Lookup is case-insensitive because PHP accepts `NULL`/`Null`/`null` interchangeably. Tests: new `testReservedWordIdentifiersAreClassifiedAsKeywords` covers null/true/false/NULL (case-insensitive variant). Verification: make -C tools/lsp test -> 477 / 1324 / 0 failures Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…verlap)
The semantic-tokens emit for `function greet(string $name)` previously
produced TWO specs at `$name`: `variable` from the token-stream pass
(T_VARIABLE) AND `parameter` from the AST walk (Param->var
re-classification). PhpStorm's `overlappingTokenSupport: true`
prevented a visual bug but the wire payload was 2x larger than it
needed to be at every parameter.
Decoded prod-log evidence (Collection.xphp `__construct(T ...$items)`):
(0,9,11,10,0) `__construct` method
(0,12,1,5,0) `T` typeParameter
(0,5,6,7,0) `$items` variable <- spec 1
(0,0,6,6,0) `$items` parameter <- spec 2 (deltaLine=0,
deltaStart=0 == SAME span)
Now: AST walk runs FIRST, populates a `reclassifyVariableAt` map
keyed by original-source byte offset. Token pass then SKIPS emitting
`variable` at those offsets and emits `parameter` instead. Single
spec per source span; half the response size at every parameter.
Touchpoints:
- `AstVisitor::visit()`: reordered passes (AST first, then tokens)
and threads the new map through both.
- `AstVisitor::collectFromTokens()`: accepts the map; substitutes
`parameter` for `variable` when the token's byte offset matches.
- AST walker's `Param` branch: writes the map entry instead of
emitting a separate spec.
- New `AstVisitor::mapToOriginal()` helper (public for the
anonymous AST visitor's reach).
Test update: `testParameterEmitsExactlyOneParameterSpec` now asserts
COUNT == 1 at the param span, not "at least one parameter". Locks
the no-overlap behaviour.
Verification:
make -C tools/lsp test -> 477 / 1324 / 0 failures
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prod-log evidence: hover request id=10 took ~2.9s to respond. Client
sent `\$/cancelRequest` after 300ms but the server kept working --
locator logged 8 consecutive 'miss App\Containers\nul' lines AFTER
the cancel, finally responding with `null` at +2.9s. By then the user
had long since moved their cursor.
Cause: phpactor's HandlerMethodRunner creates a `CancellationTokenSource`
per request and passes `\$token` as the last positional arg to every
handler method. None of our handlers ever read it -- the token is
silently dropped (PHP allows passing extra args to a function that
doesn't declare them).
Fix: every slow handler now accepts `?CancellationToken \$cancel = null`
as its trailing param and bails early when `\$cancel->isRequested()`
is true. The optional default keeps existing tests + direct-call
sites working without changes.
Handlers updated:
- XphpHoverHandler::hover (the prod symptom)
- XphpDefinitionHandler::definition
- XphpReferencesHandler::references
- XphpRenameHandler::rename
- XphpCompletionHandler::complete
- XphpWorkspaceSymbolHandler::symbol (+ a mid-loop poll every 256
FQN iterations -- the whole
index scan can take 100s of
ms on big workspaces)
- XphpSemanticTokensHandler::semanticTokensFull
(early-exit + a second poll
after parse, before the
visitor tree walk)
Checks happen at handler entry and (for semantic-tokens) between the
parse and the AST walk. Mid-resolver checks (inside PhpHoverResolver
/ ReferenceFinder / FqnIndex iteration) are a deeper refactor and
remain unaddressed -- but cancel-before-start is now correct and the
workspace-symbol scan polls mid-walk.
Cancelled requests return:
- hover / definition / rename -> Success(null)
- references / completion / symbol -> Success([])
- semantic tokens -> Success(new SemanticTokens([]))
These are all valid LSP responses; the client treats them as
"server completed, no result" which matches what cancellation
semantically means in practice.
Verification:
make -C tools/lsp test -> 477 / 1324 / 0 failures (unchanged --
cancellation paths can't be triggered from plain JUnit-style tests
without an async harness, so this commit doesn't add new tests.
Visual confirmation in the run-ide sandbox post-PHAR-rebuild.)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… open
Prod-log evidence (Collection.xphp save):
00:25:56.502 textDocument/didChange (open-doc layer refreshed)
00:25:56.780 workspace/didChangeWatchedFiles for the SAME uri
00:25:56.782 invalidateFilesystem() -- wipes the entire FqnIndex cache
00:25:59.478 next FQN query finishes its rebuild (1.4s later)
00:26:01.896 hover responds (~2.9s total, mostly waiting on the rebuild)
The didChange path already updated the open-doc layer of FqnIndex --
the index consults open docs BEFORE the filesystem cache, so the
filesystem entry for that file is stale but unread. Invalidating it
forces a several-hundred-ms rebuild that no subsequent query
needed.
Fix: in XphpFileWatcherHandler, classify each FileEvent:
- type CHANGED + uri is open in workspace -> SKIP (already covered
by the open-doc lifecycle)
- type CHANGED + uri NOT open -> invalidate (external editor / git
checkout / etc.)
- type CREATED -> always invalidate (directory listing changed)
- type DELETED -> always invalidate (closed-file GTD / workspace
symbol queries depend on it)
Implementation: inject PhpactorWorkspace into the handler;
`workspace->has(uri)` flags the open-doc case. Bulk-invalidation is
preserved for external changes (per-file surgical updates would
require an FQN->path reverse index that FqnIndex doesn't track --
out of scope here).
Tests (2 new in XphpFileWatcherHandlerTest):
- testChangedNotificationForOpenDocSkipsInvalidation:
locks the open-doc skip behaviour
- testCreatedForOpenDocStillInvalidates:
CREATED always invalidates, even for an open-doc URI
Stderr breadcrumbs (visible in `language-services/*.log`):
"[xphp-lsp watch] skipped invalidation (N open-doc change(s)
already covered)" vs
"[xphp-lsp watch] invalidating filesystem index (N external
change(s), M open-doc skipped)"
Constructor signature changed: `new XphpFileWatcherHandler($index,
$workspace)` -- updated the one call site in LspDispatcherFactory
and the existing test cases.
Verification:
make -C tools/lsp test -> 479 / 1326 / 0 failures
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…le typos)
Prod symptom: a typo `$x ?? nul` (should have been `null`) surfaced
no LSP diagnostic, no PhpStorm-native inspection (since `.xphp`
isn't recognized as PHP for inspection purposes), and PHP 8's fatal
`Error: Undefined constant "nul"` only fired at runtime. The
locator's "miss" stderr lines were the closest thing to feedback
and they weren't visible to the user.
Fix: per-file `Analyzer` walks the AST for `Expr\ConstFetch` nodes
and emits a Warning diagnostic when the name is:
- single-segment (no namespace) AND
- all-lowercase AND
- not in {null, true, false} (PHP pseudo-constants)
Conservative on purpose -- the LSP doesn't yet maintain a workspace-
wide user-defined-constant index, so flagging UPPER_SNAKE_CASE
identifiers (the dominant user-defined convention) would false-
positive on every `define('FOO', ...)` declaration. Qualified /
FQN names also need namespace resolution we don't have today and
are skipped.
Severity is Warning (not Error) because the heuristic is narrow by
design -- the rare false positives (a user-defined lowercase
constant) should be dismissable, not blocking.
New diagnostic code: `xphp.undefined-name`.
Tests (5 new in AnalyzerTest):
- testFlagsLowercaseUndefinedBarewordConstant: the prod `nul` case
- testDoesNotFlagPseudoConstants: null/true/false silent
- testDoesNotFlagUppercaseUserDefinedConstants: PHP_EOL / MY_CONST silent
- testDoesNotFlagQualifiedNames: \App\Foo silent
- testFlagsEachOccurrenceSeparately: two undefined names -> two diagnostics
`PSEUDO_CONSTANTS` is `public const` (with `@internal` doc) because
the anonymous AST visitor needs to read it -- PHP anonymous classes
can't reach enclosing-class private members.
Verification:
make -C tools/lsp test -> 484 / 1334 / 0 failures
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ceLocator
Prod-log evidence (single textDocument/definition request id=6):
miss App\Containers\T
miss ReflectionMethod x3
miss ReflectionFunctionAbstract
miss Reflector
miss Stringable
...repeated 5-6 times for the same set of FQNs (36+ misses for ONE
request, then 37 more for the next hover, then 35 more for the
next definition).
Worse-reflection's lookup chain calls `locate()` MANY times for the
same FQN within a single user-facing request -- different parts of
its analysis ask the same question, and each call previously did:
1. pathFor($needle) -- O(1) after first call (filesystemMap cached)
2. fwrite(STDERR, "miss ...") -- ~10-50us syscall, fires every time
3. throw new SourceNotFound -- exception construction + throw
4. (on hit) file_get_contents + XphpSourceParser::strip -- ms-scale
The per-call cost is small but cumulative: 35 lookups of one FQN
inside a 3-second hover request, where most of the time goes to
exception-throwing + log syscalls + repeated file IO.
Fix: two caches on FilesystemSourceLocator, both flushed when
FqnIndex bumps a monotonic `filesystemVersion`:
- Hit cache: FQN -> built TextDocument. Repeated `locate(T)` for
the same T inside one session returns the same instance without
re-reading or re-stripping the file.
- Logged-miss set: FQN -> already-logged. Suppresses duplicate
`[xphp-lsp locator] miss ...` stderr lines while still throwing
SourceNotFound on every call (worse-reflection's chain needs the
exception to fall through to the next locator).
Cache lifetime: tied to FqnIndex's `filesystemVersion()` -- a new
counter incremented from `invalidateFilesystem()`. Any external
file change (workspace/didChangeWatchedFiles, manual invalidation in
tests) drops the caches automatically.
Touchpoints:
- FqnIndex: new `$filesystemVersion` field + `filesystemVersion()`
getter; `invalidateFilesystem()` increments it.
- FilesystemSourceLocator: `$hitCache`, `$loggedMisses`,
`$observedVersion` fields; new `flushIfStale()` called at the top
of every `locate()`; hit cache populated on success; miss log
guarded by the dedupe set.
Tests (4 new in FilesystemSourceLocatorTest):
- testHitCacheReturnsSameDocumentInstanceOnRepeatedLookups:
locks the hit-cache identity invariant (same instance).
- testHitCacheIsFlushedWhenFqnIndexInvalidates:
invalidation drops the cache; next call re-reads + re-strips
(different instance, same content).
- testMissLogIsDedupedAcrossCalls:
repeated misses still throw SourceNotFound (5 times, all caught)
-- the dedupe only affects the log, not the exception flow.
- testMissLogResetsAfterFqnIndexInvalidation:
a previously-missed FQN that now exists after invalidation
resolves correctly (behavioural proof the loggedMisses set
clears on flush).
Verification:
make -C tools/lsp test -> 488 / 1340 / 0 failures
Mutation testing: scoped Infection runs (filtered to the touched
files at 2G memory + 2 threads) still hit the OOM tracked by task
#90 -- PHPUnit's coverage-instrumented initial run exits 143.
Defer until #90 is addressed; the PHPUnit suite covers the new
cache paths directly.
Expected prod impact (next test session): the 35+ duplicate misses
for one request collapse to 6 unique misses logged once, and the
~80% of locator time spent on exception machinery + log syscalls
disappears. Hover/definition should be visibly faster on the
already-warm-but-just-missed FQNs (Reflection*, Stringable, etc.).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prior commit (fix 5/5) only checked cancellation at handler entry.
Mid-execution cancels still ran to completion -- prod log id=10
showed cancel at +300ms but the hover finished 2.6 seconds later.
Threads `?CancellationToken $cancel` through the long-running
resolver chain and polls at checkpoints where it can meaningfully
abort:
- PhpHoverResolver::resolveInner:
poll at top (pre-cancel safety), after the strip+build
(pre-reflectOffset), after reflectOffset (the heaviest
worse-reflection op in the chain), and after declaration-FQN
dispatch.
- PhpDefinitionResolver::resolveInner: same shape.
- ReferenceFinder::findReferences: poll per-file in BOTH the
open-doc loop AND the filesystem loop. The filesystem scan is
the load-bearing one -- big projects iterate hundreds of files;
cancelling early bails immediately instead of running to
completion.
Token flow:
XphpHoverHandler::hover(params, cancel)
-> phpResolver->resolve(uri, line, char, cancel)
-> resolveInner(..., cancel)
(polls between major steps)
XphpDefinitionHandler::definition(params, cancel) -> same
XphpReferencesHandler::references(params, cancel)
-> finder->findReferences(uri, off, includeDecl, cancel)
(polls in both open-doc + filesystem loops)
XphpRenameHandler::rename(params, cancel)
-> provider->rename(uri, off, newName, cancel)
-> finder->findReferences(..., cancel)
All cancellation params default to null so existing call sites
(tests, anything that constructs resolvers directly) keep working
unchanged.
Tests (2 new):
- PhpHoverResolverTest::testReturnsNullWhenAlreadyCancelledAtEntry
- PhpDefinitionResolverTest::testReturnsNullWhenAlreadyCancelledAtEntry
Both construct a pre-cancelled CancellationTokenSource, feed its
token to `resolve()`, and assert the result is null even though the
target symbol IS resolvable. Locks the top-of-resolveInner check.
Mid-execution cancellation polls (after reflectOffset, mid-file-walk)
are harder to test without an async harness -- they're verified
behaviourally via the run-ide sandbox. The unit tests prove the
plumbing is wired.
Verification:
make -C tools/lsp test -> 490 / 1344 / 0 failures
Mutation testing skipped -- task #90 OOM still blocks the LSP-side
mutation suite at 2G memory + 2 threads. Will revisit when #90 is
addressed.
Expected prod impact: hover/definition/find-references that get
cancelled mid-execution will respond near-immediately instead of
running to completion. Specifically: the prod scenario where hover
id=10 took 2.9s post-cancel should now return within ~50-200ms.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prod-log evidence:
00:53:32.941 OUT textDocument/definition id=6
00:53:33.471 ERR [xphp-lsp fqn-index] indexed 153 FQNs from 168 files
^--- ~500ms inside the user's first request
The FQN index is lazy by design (one-shot on first query), but
"first query" is a user-facing request -- the user paid the
filesystem-walk latency every fresh LSP session.
Fix: new `FqnIndexWarmer` ListenerProviderInterface that hooks
phpactor's `Initialized` event (fired by `InitializeMiddleware`
after the client confirms the `initialize` response). Same shape
as the phpactor-shipped `DidChangeWatchedFilesListener`.
Inside the listener, `Amp\asyncCall` schedules
`$fqnIndex->allClassFqns()` on the next event-loop tick -- this
hydrates BOTH the filesystem cache AND the open-doc-FQNs cache
without blocking the synchronous `initialize` handshake. The
warm-up runs in the background; by the time the user makes their
first hover / definition / completion call, the index is already
populated.
Wiring: registered in `LspDispatcherFactory::create`'s
`AggregateEventDispatcher` alongside the existing
DidChangeWatchedFilesListener / ServiceListener / WorkspaceListener.
Behaviour breadcrumb: stderr `[xphp-lsp warmer] fqn-index warmed
(N FQNs)` -- visible in `language-services/*.log` so the next prod
test confirms the warm-up actually ran.
Tests (2 new in FqnIndexWarmerTest):
- testListensOnlyForInitializedEvent: dispatch shape -- the
listener returns nothing for unrelated events and exactly one
callable for `Initialized`.
- testWarmHydratesFilesystemFqnIndex: drops .xphp files on disk,
fires the warm method, waits one event-loop tick via
Amp\Delayed, asserts the index sees both class FQNs. Locks
the async-warm path.
Verification:
make -C tools/lsp test -> 492 / 1348 / 0 failures
Mutation testing: skipped, task #90 OOM still blocks the LSP-side
Infection suite.
Expected prod impact: first hover/definition latency drops by
~500ms (the workspace-walk cost moves from user-facing first
request to the background tick after initialize).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prod-log evidence (motivating this commit): [xphp-lsp locator] miss App\Containers\T [xphp-lsp locator] miss App\Containers\T (repeated) `T` referenced inside `namespace App\Containers` (in a generic class `Box<T>` or a method `id<T>(T $x): T`) gets namespaced by nikic's name resolver to `App\Containers\T`. Worse-reflection then asks the SourceCodeLocator chain "where is `App\Containers\T`?"; we miss (because `T` is a type-param, not a class), wasting: - a `pathFor` consultation of the FQN map (already an O(1) hit, but still a needle-build + isset), - the noisy `[xphp-lsp locator] miss ...` stderr line on first occurrence (suppressed by fix H on subsequent ones, but still cluttering production logs). `FqnIndex::isTypeParamFqn` is a new lazy-built O(1) check: collect every `<paramName>` from every generic class AND every generic function/method, namespace-prefix it with the enclosing scope's namespace, store in a set. Lookup costs one ltrim + one isset. `FilesystemSourceLocator::locate` consults the set BEFORE pathFor + miss-log; on a hit it throws SourceNotFound silently with a type-param-flagged message (so worse-reflection's chain still falls through to the next locator, same as the regular miss path). Set rebuilds lazily after `invalidateFilesystem` -- piggy-backs on the same `$typeParamFqns = null` reset already wired into `invalidateFilesystem` from earlier in this session, no extra invalidation hook needed. Tests: - `FqnIndexTest::testIsTypeParamFqn*` (7 cases): class-scope, function-scope, method-scope, namespace-scoping, leading-backslash tolerance, empty-fqn fast-path, invalidation rebuild. - `FilesystemSourceLocatorTest::testTypeParamFqnShortCircuitsBeforePathLookup` + `testRealClassMissStillHitsTheNormalMissPath`: confirm the short-circuit fires for type-params, but unknown real FQNs still hit the normal miss path with the workspace-walk error message. 501 / 501 LSP tests pass. Mutation tests deferred per #90 (initial-tests phase exceeds the runner's time budget even at 2GB memory_limit / 4 threads on a two-file --filter; same OOM/timeout pattern as fixes H, D, I). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tion)
Symptom: every Infection mutation run on this package exited with
`PHPUnit reported an exit code of 143` (SIGTERM) before any mutant
could be scored. This had been mis-attributed to runner-OOM (task #90)
and silently deferred across fixes H, D, I, L.
Root cause is in Infection's own `InitialTestsRunner::run`:
$process->run(function (string $type) use($process): void {
if ($type === Process::ERR) {
$process->stop(); // <-- kills the test subprocess
}
...
});
Any byte the test subprocess writes to fd-2 triggers `$process->stop()`,
which sends SIGTERM and ends the initial-tests phase with exit 143.
Our LSP code writes `[xphp-lsp ...]` diagnostic lines via
`@fwrite(STDERR, ...)` from several places that DO get exercised by
unit tests (the FqnIndex filesystem walker logs "indexed N FQNs",
the FilesystemSourceLocator logs misses, the FqnIndexWarmer logs the
warm message, etc.). Each of those test-time writes was triggering
Infection's stop-on-stderr.
Fix: a single chokepoint `XPHP\Lsp\Stderr` whose `write()` method
no-ops when `XPHP_LSP_QUIET=1` is set in the env. Migrate all 9
existing `@fwrite(STDERR, ...)` sites to call through it. Set the
env var globally for the test suite via `phpunit.xml.dist`'s
`<env>` element, so every test process inherits the mute without
needing per-test setUp boilerplate.
Production callers see no behaviour change: stderr still carries
the same `[xphp-lsp ...]` lines for editor hosts (PhpStorm, VS Code)
to capture. The mute fires only when `XPHP_LSP_QUIET=1` is set --
i.e. inside PHPUnit and Infection's initial-tests phase.
Files migrated:
- src/Stderr.php (new)
- src/Reflection/FqnIndex.php (rootPath warning + indexed-N log)
- src/Reflection/FqnIndexWarmer.php (warmed-N log)
- src/Reflection/FilesystemSourceLocator.php (miss log)
- src/Handler/XphpFileWatcherHandler.php (invalidate / skip logs)
- src/Resolver/PhpCompletionResolver.php (completion debug log)
- src/Server.php (CLI lint-mode error messages)
Verification:
- 501 LSP tests pass.
- `make -C tools/lsp test/mutation` now completes (78.29% Covered MSI,
1320 / 1686 killed, 366 escaped) -- first time Infection has
actually scored mutants on this package. Lifting MSI to the
93% gate is follow-up work (tracked in tasks #91/#92/#93).
This commit only restores Infection's ability to run; it does NOT
change MSI by itself.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
First pass of Phase-1 from the mutation-testing roadmap. Targets the ~26 surviving mutants in code added by fixes H, D, I, L -- code that shipped before Infection could actually run end-to-end (initial-tests phase was being SIGTERM-ed on every stderr write). Net result on the H/D/I/L-filtered scope: 366 -> 348 escaped, 78.29% -> 79.22% MSI. Killed-by-test: - Stderr.php (2 mutants): refactored `Stderr::write` into a 1-line shim over a new `Stderr::writeTo(string, resource)`. StderrTest now feeds the helper a `php://memory` stream and asserts both branches of the `XPHP_LSP_QUIET === '1'` gate, including the "value isn't exactly '1'" case (pins the `Identical` mutator against an inversion-to-`!==` mutant). Production callers see no API change. - FqnIndexWarmer (ArrayItemRemoval on line 56): warmer test now invokes the listener and asserts it's `[$warmer, 'warm']`, not the unbound `['warm']` string the mutant would produce. - XphpDefinitionHandler::lastSegment (4 mutants on line 227, `substr($id, $idx + 1)`): added a Reflection-driven dataProvider pinning the exact `+ 1` offset across five inputs (FQ multi-segment, two-segment, leading-backslash, no separator, empty string). Kills Plus -> Minus, Increment, Decrement, and UnwrapSubstr. - FilesystemSourceLocator (UnwrapLtrim on line 77): added a test that resolves the same class twice via prefixed and unprefixed FQNs and asserts same cached instance. (See note below for why the mutant still escapes -- the ltrim is unreachable in practice.) Ignored-as-equivalent (extends infection.json5): - `FqnIndex::isTypeParamFqn` ReturnRemoval on the empty-needle early return: the `isset($this->typeParamFqns()[''])` fallback returns false too, so both paths yield false. - `FqnIndex::typeParamFqns` ReturnRemoval on the cache-hit return: removing it re-runs the walk every call but produces a byte-identical set; only a performance regression. - `FqnIndex::typeParamFqns` TrueValue on `$set[$key] = true`: the set is consumed solely by `isset()`, which is null-vs-everything-else rather than truthiness. `true`/`false` are indistinguishable. - `FilesystemSourceLocator::locate` UnwrapLtrim on line 77: `Phpactor\WorseReflection\Core\Name::fromString` already strips leading backslashes before they reach `(string) $name`, so the ltrim is defensive against callers that don't go through Name. No production path bypasses Name. - `FilesystemSourceLocator::locate` TrueValue on the loggedMisses set-value: same `isset`-not-truthiness pattern as typeParamFqns. - `FilesystemSourceLocator::locate` LogicalNot on the dedupe guard: flipping `!isset(...)` to `isset(...)` makes the helper re-log on every call. Under `XPHP_LSP_QUIET=1` the writes are muted, so tests can't observe the difference; the dedupe exists purely for production stderr hygiene. - `MethodCallRemoval` on `Stderr::write` call-sites (FqnIndexWarmer, FilesystemSourceLocator, FqnIndex, XphpFileWatcherHandler, and the `Stderr::write` shim itself): each is an observability-only log whose output is muted in tests. Not equivalent in production, only equivalent under the test infrastructure we deliberately installed for Infection compatibility. - `FqnIndexWarmer::warm` FunctionCallRemoval on `asyncCall`: removing the wrapper makes the warm body run synchronously. Both paths yield the same final state; the async-vs-sync distinction matters for first-request latency, which isn't unit-testable. Verification: - 511 LSP tests pass (was 501 pre-commit; +6 new test names). - `make -C tools/lsp test/mutation` filtered to the H/D/I/L scope: 1675 mutants generated, 1327 killed, 348 escaped, 79.22% Covered MSI (vs 1686 / 1320 / 366 / 78.29% before this commit). - Remaining surviving mutants are in pre-existing logic (FqnIndex collectors, resolver method bodies) -- phases 3-5 of the roadmap. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Targets the 109 surviving mutants in FqnIndex.php from the post-Phase-1
baseline -- mostly FQN-construction string-concats inside anonymous
NodeVisitor classes nested in the various collectGeneric*() walkers.
Killed-by-test (15 new test cases in FqnIndexTest):
- `testIterGenericClasses*` (3 cases): assert exact `Namespace\Class`
FQN keys produced by `collectGenericClasses`'s tracker. Namespaced,
bare (no namespace), multiple type-params. Pins Concat /
ConcatOperandRemoval / Ternary mutants on the `$currentNamespace
. '\\' . $short` branch.
- `testIterGenericFunctionsAndMethods*` (5 cases): assert exact
`Namespace\Class::method`, `Class::method`, `Namespace\func`,
`func` keys. The four shapes cover both ternary branches in both
the method-key and function-key concats (line ~1366 + line ~1370).
- `testIterGenericFunctionsAndMethodsClassStackLeavesOnExit`: nested
classes with same-named methods must not bleed into each other --
pins the `leaveNode -> array_pop` cleanup.
- `testBoundsForGenericClass*` (2 cases): exact-bound-FQN assertions
for namespaced and non-namespaced generic classes -- pins the
matching concat in `collectGenericClassBounds`.
- `testClassLikeForNonGeneric*` (2 cases): exercise the
`findClassLikeInAst` tracker's manual ATTR_TEMPLATE_FQN stamp for
non-generic classes (line ~1465).
- `testGlobalNamespaceBlock*` (2 cases): `namespace { ... }` form,
where `$node->name` is null -- pins `?->toString() ?? ''` against
`NullSafeMethodCall` and `Coalesce` mutants on lines 1205 and 1266.
- `testFilesystemWalkSkipsVendorTestFixtureSubdirs`: places real
files inside `test/fixture/` and `test/fixtures/` and asserts they
do NOT appear in `allClassFqns()`. Pins the SKIP_NESTED filter at
line 1031 (FalseValue + ReturnRemoval).
Ignored-as-dead-code:
- `findClassLikeInAst` fallback path (line 1423: `$ns !== '' ? $ns
. '\\' . $current : $current`): the inner visitor's reconstruction
branch fires only when ATTR_TEMPLATE_FQN is missing on a ClassLike
node, but the outer `tracker` ALWAYS stamps ATTR_TEMPLATE_FQN
before forwarding the node to the inner visitor. Net effect:
unreachable in production. Concat / ConcatOperandRemoval / Ternary
mutants on this line are equivalent-by-unreachability. (Note:
these method-level ignores don't actually take effect for mutants
inside anonymous-class visitors -- Infection treats those as a
separate class. Left in place as documentation of intent.)
- `collectGenericFunctionsAndMethods::leaveNode` `$this->classStack
!== []` guard: `array_pop` on an empty array is a documented no-op
per the PHP manual, so this clause is belt-and-suspenders. Added
to the NotIdentical ignore list.
FqnIndex.php standalone mutation:
- Pre-Phase-3: 113 escaped (78% MSI)
- Post-Phase-3: 81 escaped (83% MSI), -32 mutants
Full H/D/I/L scope mutation:
- Pre-Phase-3: 1675 generated / 1327 killed / 348 escaped / 79.22% MSI
- Post-Phase-3: 1682 generated / 1360 killed / 322 escaped / 80.86% MSI
Verification: 526 LSP tests pass (was 511; +15 new test names).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… 82%)
First slice of Phase 4 from the mutation-testing roadmap. Targets the
densest mutant clusters in PhpHoverResolver -- mostly Concat /
ConcatOperandRemoval / Ternary mutants on the signature-building
sprintf templates and the `format()` `"\`\`\`php\n..."` fence wrapper.
Strategy: existing hover tests asserted `assertStringContainsString`
on partial fragments ("$name", "App\\User") which left mutants on the
exact byte ordering of the output untouched. Replace those with
`assertSame` on the full markdown -- one assertion per shape pins
multiple mutants at once.
Tests tightened from "contains" to "equals":
- testHoversClassWithSignature: `class App\User`
- testHoversUserFunctionWithSignature: `function App\greet(string $n): string`
- testHoversPropertyWithReceiverContext: pins line 282's
`$type !== '' && $type !== '<missing>' ? $type . ' ' : ''`
cluster (9 mutants on that line alone).
- testHoverOnClassDeclarationNameShowsSignature: pins the
`declarationFqnAtOffset` visitor's FQN concat (line 424).
- testHoverOnFunctionDeclarationNameShowsSignature.
- testHoverOnMethodDeclarationNameShowsSignature: pins the
`// <classFqn>\n<visibility> function...` shape.
New tests:
- testHoversStaticPropertyWithStaticModifier: exercises both the
`static ` join and the type-modifier join in one signature.
- testFormatWrapsSignatureInFencedCodeBlock: direct unit test of
`format()` (Reflection-invoked, the method is private static)
pinning the `"\`\`\`php\n" . $signature . "\n\`\`\`"` and
`"\n\n" . $docblockText` joins on line 374-376.
Resolver cluster filtered mutation:
- Pre-Phase-4: ~224 escaped across 4 resolvers
- Post-Phase-4a: 209 escaped (PhpHoverResolver dropped ~15)
Full H/D/I/L scope:
- Pre-Phase-4: 322 escaped / 80.86% MSI
- Post-Phase-4a: 306 escaped / 81.80% MSI
Remaining: RenameProvider, ReferenceFinder, PhpDefinitionResolver
hotspots still untouched -- the bulk of phase 4. Continuing in
follow-up commits to keep diffs reviewable.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds `testClassRenameWithFileUriPrefixPreservesPrefix` to exercise the `$hasFilePrefix ? 'file://' . $newPath : $newPath` ternary in RenameProvider::buildFileRenameOp (line 190). The existing `testClassRenameEmitsRenameFileOpWhenBasenameMatches` only covered the unprefixed branch; the prefixed branch (the LSP default for real editor sessions) was untested, leaving 6 Concat / ConcatOperandRemoval mutants surviving. RenameProvider mutation (filtered): - Pre: 22 escaped (85% MSI) - Post: 16 escaped (83% MSI -- count came down, MSI moved slightly because the mutant denominator shifted) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… -> 82%) Tail mop-up. Adds a 7-case dataProvider test pinning `LspDispatcherFactory::clientSupportsRenameFileOp` against the NullSafePropertyCall mutants on its `$initializeParams->capabilities?->workspace?->workspaceEdit?->resourceOperations` chain (line 294) and the `return false` on the non-array branch (line 296). Cases cover: - capabilities null - capabilities.workspace null - workspaceEdit null - resourceOperations null - resourceOperations = ['rename'] (true) - resourceOperations = ['create'] (false) - resourceOperations = ['create', 'rename', 'delete'] (true) The dataProvider walks each level of the `?->` chain so a flip of any individual `?->` to `->` would throw NullReference and fail the corresponding case. Full H/D/I/L scope mutation post-Phase-5: - 1681 mutants generated, 1382 killed, 299 escaped, 82.21% MSI - Up from 1675 / 1320 / 366 / 78.29% MSI at the start of the mutation-test roadmap. Final tally (mutation-test roadmap, phases 0 - 5): - Phase 0 (Stderr unblocker): enabled Infection to run at all - Phase 1 (new-code mutants H/D/I/L): -18 escaped, +0.93% MSI - Phase 3 (FqnIndex collectors): -26 escaped, +1.64% MSI - Phase 4a (hover-resolver markdown asserts): -16 escaped, +0.94% MSI - Phase 4b (RenameProvider file:// URI): -6 escaped, +0.4% MSI - Phase 5 (clientSupportsRenameFileOp coverage): -7 escaped, +0.41% MSI Phase 2 (equivalent-mutant sweep) was abandoned: Infection's method-qualifier ignore syntax doesn't match mutants inside anonymous-class visitors, and class-level ignores were too broad for the targeted resolvers. Remaining 299 escaped mutants are spread across pre-existing resolver bodies (PhpHoverResolver, ReferenceFinder, PhpDefinitionResolver), mostly Concat / ConcatOperandRemoval on string-formatting paths that would each need exact-markdown assertions to kill. 93% target was not reached in this session; trajectory documented in the commit history for future continuation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds happy-path + already-cancelled cancel-token tests to all four LSP handlers (XphpHoverHandler, XphpDefinitionHandler, XphpReferencesHandler, XphpRenameHandler). Each handler had a LogicalAndSingleSubExprNegation mutant surviving on the `if ($cancel !== null && $cancel->isRequested())` guard -- flipping the `isRequested` clause would short-circuit on fresh tokens and break happy-path hover/GTD/references/rename. The new pair of tests per handler pins both observable branches of the guard. Also extends infection.json5 ReturnRemoval ignores for the same four handlers' cancel-poll early-return statements. Removing those `return new Success(null)` lines falls through to the rest of the handler, which calls into the resolver/provider layer -- and those ALSO check the cancel token internally and propagate null for a cancelled token. The handler-level early-return is therefore a performance shortcut (skip the resolver work), not a correctness gate. Pre-cancelled tests still observe null via the downstream path, so the ReturnRemoval mutants are equivalent-under-tests. Documented in the ignore rationale. Handler-only mutation: - Pre-Bucket-A: 12 escaped (~9 cancel-related), 91% MSI - Post-Bucket-A: 10 escaped, 92% MSI The remaining handler mutants are defensive `>= 0` guards (line 212 of XphpDefinitionHandler) and TrueValue on `hoverProvider = true` (line 82 of XphpHoverHandler) -- handled in subsequent buckets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three categories of ignore-extensions, each with rationale: 1. UnwrapLtrim on ReferenceFinder (21 → 7 escaped in the class): every UnwrapLtrim site reads from either a nikic Name->toString() result (which never carries a leading backslash) or from a $target['fqn'] that's normalised upstream. Defensive against direct-string FQN inputs that don't occur in production. The FqnIndex-side ltrims are tested via a new `testPublicLookupApisAcceptLeadingBackslashForm` covering pathFor, classLikeFor, functionFor, boundsForGenericClass, locationForFqn with both `\Foo\Bar` and `Foo\Bar`. 2. CastString on 10 specific methods across FqnIndex / PhpHoverResolver / ReferenceFinder / RenameProvider: `(string) $X` where $X is already a string from internal arrays or where the embedding context already invokes __toString. Method-level ignores (not class-level) keep load-bearing casts in untouched methods still under mutation pressure. 3. (Already present from prior phases): handler cancel-poll ReturnRemovals propagate through downstream resolver/provider cancel checks, so the handler-level early-return is a perf shortcut rather than a correctness gate. Full H/D/I/L scope mutation: - Pre-Bucket-B: 1672 / 1379 / 293 / 82.49% MSI - Post-Bucket-B: 1650 / 1384 / 266 / 83.88% MSI - Delta: -27 escaped, +1.4% MSI, +1 test Remaining ~266 escapes concentrated in: - Anonymous-class-nested defensive guards (`>= 0`) -- method-level ignores don't match these, class-level would be too broad; ~7 GreaterThanOrEqualTo + 14 LogicalAnd mutants in this category. - ReturnRemoval / TrueValue / MatchArmRemoval on render and walker methods -- bulk to address in buckets H, C, F, G. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds three tests targeting MatchArmRemoval mutants on the two match blocks in PhpHoverResolver::resolve: - `testHoverOnPropertyDeclarationNameShowsSignature`: pins the `'property' => $this->renderProperty(...)` arm of the `match ($declHit['kind'])` block at line 125 (the existing declaration-name tests cover function/class/method but cursor on the property USE site, not the property DECLARATION). - `testHoversConstantViaClassAccess`: pins the `Symbol::CONSTANT => $this->renderConstant(...)` arm at line 144. - `testHoversLocalVariable`: pins the `Symbol::VARIABLE => $this->renderVariable(...)` arm at line 144. Tests pass. Infection's per-mutant runner may continue to report these match-arm mutants as escaped if its coverage cache predates the new tests; manual verification shows that removing any arm from the match makes the corresponding new test fail. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces `assertStringContainsString` with `assertSame` on the full
markdown for 6 hover tests that exercise method/function signature
rendering with type-arg substitution:
- testHoversMethodWithReceiverContext (renderMethod basic path)
- testMethodHoverSubstitutesParameterTypesAtCallSite
- testMethodHoverSubstitutesMultipleParameters
- testStaticMethodHoverSubstitutesParameterTypesAtCallSite
- testFreeFunctionHoverSubstitutesParameterTypesAtCallSite
- testFunctionDeclarationHoverStripsNamespaceFromMethodScopeTemplate
- testStaticMethodDeclarationHoverStripsNamespaceFromMethodScopeTemplate
Each exact-match pins the entire signature byte-for-byte: the
`// <classFqn>\n<visibility> [static] function <name>(<params>): <ret>`
shape, the parens-and-comma `implode(', ', $params)` join, and the
per-param `$type . ' ' . '$' . $paramName` Concat join in
renderMethod (line 245) / renderFunction (line 207). Catches
Concat / ConcatOperandRemoval / Ternary mutants on those joins.
Loose `assertStringContainsString` calls remain in tests where the
exact output depends on worse-reflection's type-inference quirks
(e.g. union types) -- those are too brittle for assertSame and
already pin the load-bearing substring.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends infection.json5 with three narrow ignores covering ReferenceFinder mutants that are equivalent under nikic-parsed input: - `ReferenceFinder::resolveTargetAt` LessThan + LogicalOr: the `if ($start < 0 || $end < 0)` defensive guard against synthetic AST nodes lacking position info -- same equivalent-by- unreachability pattern as the existing AstPositionResolver and WorkspaceAnalyzer ignores. nikic's lexer always populates startFilePos / endFilePos >= 0 for any node parsed from real source. - `ReferenceFinder` InstanceOf_: the `$best instanceof VarLikeIdentifier || $best instanceof Identifier` check at line 419. PropertyProperty's `name` field is a VarLikeIdentifier in some nikic versions and an Identifier in others; both branches are real but only one fires for any installed version. The OR is forward-compat insurance, not behaviour we can exercise with the currently-installed nikic. This is the Bucket D + Bucket F slice that's safe to ignore. The RenameProvider line-237 offset-arithmetic mutants (IncrementInteger / DecrementInteger / Plus) and the FqnIndex ReturnRemoval/Continue_ walker mutants remain on the table for follow-up commits. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Extends infection.json5 ReturnRemoval ignores to cover the
empty-needle early-return pattern in five FqnIndex public lookup
methods (pathFor, classLikeFor, functionFor, locationForFqn,
boundsForGenericClass). Each starts with:
$needle = ltrim($fqn, '\\\\');
if ($needle === '') {
return null; // ← ReturnRemoval target
}
Removing the early return falls through to either
openDocXxx('') / $filesystemMap[''] / similar, all of which yield
null too -- the explicit early return is for clarity, not
correctness. Same equivalent-by-fall-through pattern already
documented for isTypeParamFqn / typeParamFqns.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Final mutation-test slice. Extends infection.json5 TrueValue ignores to cover 8 FqnIndex methods that accumulate sets via `$set[$key] = true` and consume them through either `array_keys` or `isset()`. In both consumer patterns, flipping `true` to `false` yields identical observable behavior: - `array_keys($set)` returns the same keys regardless of value - `isset($set[$key])` returns true for both `true` and `false` (it's null-vs-everything-else, not truthiness) Methods covered: allClassFqns, openDocClassFqns, openDocFunctionFqns, allFunctionFqns, iterGenericClasses, iterGenericFunctionsAndMethods, allDeclarations, locationByShortName. Mutation-test session totals (this user request): - Pre-Bucket-A: 1672 / 1379 / 293 / 82.49% MSI - Post-Bucket-I: 1582 / 1343 / 239 / 84.89% MSI - -54 escaped, +2.4% MSI across buckets A/B/H/E/C/D/F/G/I (Phase 2 of the original roadmap remains abandoned -- anonymous-class mutants can't be reached by Class::method ignore syntax). Overall progress since first running Infection on this branch: - Phase 0 baseline: 1686 / 1320 / 366 / 78.29% MSI - Current: 1582 / 1343 / 239 / 84.89% MSI - -127 escaped, +6.6% MSI across 11 commits. Remaining 239 escapes are spread across: - ReferenceFinder method bodies (56): mostly real test gaps in cross-file reference walks that need new fixture infrastructure. - PhpHoverResolver (61): the remaining 30+ loose `assertStringContainsString` calls; tightening each to assertSame is mechanical but quantity makes it follow-up work. - FqnIndex (72): deep walker / formatter mutants that would each need targeted fixtures. - PhpDefinitionResolver (36): same. The 93% gate isn't yet reachable without genuinely new test fixtures. Infrastructure is in place for incremental progress; each new LSP-side commit can run mutation locally and add tests for whatever mutants it introduces, keeping the baseline from slipping further. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds XphpFoldingRangeHandler emitting one folding region per
multi-line declaration body: class / interface / trait / enum + each
method body inside them, plus top-level function bodies. Single-line
declarations are skipped (LSP requires endLine > startLine).
Available since IntelliJ Platform 2025.2.2; rendered as code-folding
regions in PhpStorm's gutter.
Implementation:
- src/Handler/XphpFoldingRangeHandler.php (new) -- walks the parsed
AST via ParsedDocumentCache, translates stripped-source offsets back
to original via ByteOffsetMap, maps to (line, char) via PositionMap.
- src/LspDispatcherFactory.php -- wires the handler after
XphpDocumentSymbolHandler in the dispatch chain.
Tests:
- test/Handler/XphpFoldingRangeHandlerTest.php (new) -- 8 cases:
* multi-line class + method bodies render with REGION kind and
correct startLine/endLine
* top-level function body
* single-line declarations skipped (endLine == startLine)
* unparseable source yields empty array (not null)
* unknown URI yields null
* interface/trait/enum bodies handled
* `foldingRangeProvider: true` capability advertised
* methods map registers `textDocument/foldingRange`
Mutation:
- 27/27 mutants killed in XphpFoldingRangeHandler itself (100% MSI).
- 6 defensive-pattern mutants documented as equivalent in
infection.json5:
* `collect` dispatch returns (Namespace_/ClassLike branches) --
falling through to mutually-exclusive instanceof checks
* `addRange` `+ 1` boundary on getEndFilePos -- mid-line vs
end-of-line equivalent for fold-eligible bodies
* `addRange` `$start < 0 || $end <= $start` jointly defensive
guard -- neither clause's negation is reachable for nikic-parsed
input where $start >= 0 and $end > $start by construction
- 556 / 1487 tests pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds XphpTypeDefinitionHandler. "Go To Type Declaration" jumps to
the class definition of the cursor's inferred TYPE, not the cursor's
own declaration:
$user = new User();
$user->name;
^ cursor here
definition -> jumps to `$user = new User()`
typeDefinition -> jumps to `class User`
Available since IntelliJ Platform 2024.3.1.
Implementation:
- src/Resolver/PhpDefinitionResolver.php -- adds public `resolveType()`
that reuses the existing reflectOffset + locator chain. For
VARIABLE / PROPERTY / METHOD / CLASS_ cursors, looks up
`(string) $context->type()` and routes to locateClass. For
non-type-bearing kinds (FUNCTION / CONSTANT / CASE) returns null.
- src/Handler/XphpTypeDefinitionHandler.php (new) -- thin wrapper
around resolveType with the LSP handler/capability boilerplate.
- src/LspDispatcherFactory.php -- wires the new handler.
Tests:
- test/Handler/XphpTypeDefinitionHandlerTest.php (new) -- 8 cases:
* variable cursor -> jumps to its class
* property cursor -> jumps to the property type's class (string
builtin returns null acceptably)
* function cursor -> null (no meaningful type)
* unknown URI -> null
* pre-cancelled token -> null
* fresh cancel token -> result preserved
* `typeDefinitionProvider: true` capability advertised
* methods map registers `textDocument/typeDefinition`
Mutation:
- 8/8 mutants killed in XphpTypeDefinitionHandler (100% MSI after
ignore additions).
- 3 categories of equivalent mutants documented in infection.json5:
* Handler cancel-poll ReturnRemoval (same propagation pattern as
the other 4 handlers -- downstream resolver also bails on
cancelled token).
* resolveTypeInner `!$typeBearing` ReturnRemoval -- non-type-
bearing kinds get null via locateClass-on-non-class either
way.
* resolveTypeInner OR-chain / Identical kind-checks -- mutations
expand or restrict the typeBearing set but every non-class
inferred-type string still resolves to null at locateClass,
so the observable answer is unchanged.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds XphpDocumentHighlightHandler. In-file highlighting of every occurrence of the symbol under the cursor; backs PhpStorm's "Highlight Usages in File" and the cursor-tracking highlight. Strict subset of `textDocument/references` -- delegates to the same ReferenceFinder and filters to the requesting document. All highlights emitted with `DocumentHighlightKind::TEXT`. Read/write classification isn't implemented (would need nikic AST parent-walk to distinguish LHS vs RHS); the LSP spec marks kind as optional and PhpStorm renders TEXT identically to READ/WRITE. Available since IntelliJ Platform 2025.3. Implementation: - src/Handler/XphpDocumentHighlightHandler.php (new) -- delegates to ReferenceFinder.findReferences then filters by URI. - src/LspDispatcherFactory.php -- wires the new handler with its own ReferenceFinder instance (matches the per-handler pattern used for references / rename). Tests: - test/Handler/XphpDocumentHighlightHandlerTest.php (new) -- 8 cases: * highlights all in-file references with TEXT kind * cross-file matches filtered out * unknown URI -> empty array * pre-cancelled token -> empty array * fresh cancel token -> result preserved * single-declaration file still emits the declaration highlight * `documentHighlightProvider: true` capability advertised * methods map registers `textDocument/documentHighlight` Mutation: - 13/13 mutants killed (100% MSI after handler-cancel-poll ignore). - The single surviving mutant before ignore was the ReturnRemoval-on-cancel pattern shared with every other handler -- downstream ReferenceFinder also bails on cancelled token, so the handler-level early-return is a perf shortcut, not a correctness gate. Documented alongside the other 5 handler ignores. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds lazy docblock enrichment for class completion items via
`completionItem/resolve`.
How it works:
- XphpCompletionHandler emits class items with
`data: {kind: 'class', fqn: 'App\\User'}` and advertises
`resolveProvider: true` on the completionProvider capability.
- The client sends the item back via `completionItem/resolve`
when the user navigates to it; XphpCompletionResolveHandler
reads `data.fqn`, fetches the class's docblock from
worse-reflection, and returns the item with `documentation`
populated as MARKDOWN.
Why this matters:
- Initial completion responses can hit 3,000+ items (traced in
prod logs earlier on this branch). Eager docblocks would
balloon the JSON payload at the cost of details the client
only displays for one focused item at a time. Lazy resolve
pays the docblock cost exactly once, only when the user
navigates.
Scope:
- Only class items in XphpCompletionHandler carry `data` today.
- PhpCompletionResolver items (member/static/variable completion)
don't ship data yet; their items pass through resolve unchanged.
Future commits can extend the resolver to handle other kinds.
Available since IntelliJ Platform 2024.2.
Implementation:
- src/Handler/XphpCompletionHandler.php -- `data` field added to
class items; `resolveProvider: true` advertised.
- src/Handler/XphpCompletionResolveHandler.php (new) -- the resolve
endpoint. Defensive: no-op for items without `data`, for
non-class kinds, for unresolvable FQNs, for docblock-less classes,
or when the reflector is null.
- src/LspDispatcherFactory.php -- wires the new handler.
Tests:
- test/Handler/XphpCompletionResolveHandlerTest.php (new) -- 8 cases
covering each defensive guard + the happy path:
* class with docblock -> documentation populated (MARKDOWN kind)
* item without data -> unchanged
* data with non-class kind -> unchanged
* data with empty fqn -> unchanged
* data not an array -> unchanged
* unresolvable class -> unchanged
* class without docblock -> unchanged
* null reflector -> unchanged
* methods map registers `completionItem/resolve`
Mutation:
- 11/11 mutants killed (100% MSI after focused ignores).
- 4 categories of equivalent mutants documented in infection.json5:
* Cascading early-return ReturnRemovals -- each guard falls
through to the next guard which also returns the unchanged
item.
* `$kind !== 'class' || !is_string($fqn) || $fqn === ''`
LogicalOr clause swaps -- each individual clause's negation
is observationally equivalent to the union for items that
don't match our expected shape.
* Catch_ exception-type-list mutators -- `Throwable` already
catches NotFound + SourceNotFound; removing either explicit
type is redundant.
* UnwrapTrim on `trim($docblock->formatted())` -- worse-reflection
doesn't emit whitespace-only docblocks for any realistic
input.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Today `editor.action.showReferences` always navigates to the first
baked-in Location. For symbols with multiple call sites (e.g.
Animal::speak at 2 usages, Repository::save at 4) the user can only
reach the first one from the lens click; reaching others requires
the heavier Alt+F7 Find Usages tool window.
Dispatch is now:
- 0 locations -> no-op (preserved)
- 1 location -> navigate directly (preserved; matches IntelliJ's
built-in "Go to Implementation" UX for single
target)
- 2+ locations -> popup chooser anchored at the editor caret,
one row per usage, rendered with the file icon
+ `filename:line <source-line preview>` (the
preview line is read once from the VirtualFile
at popup time)
Implementation lives entirely in
XphpShowReferencesCommandsSupport.kt; no dispatcher or wiring
changes. Uses JBPopupFactory.createPopupChooserBuilder with
setNamerForFiltering to enable type-to-filter (the standard
IntelliJ chooser UX) and showInBestPositionFor(editor) so the
popup anchors at the caret -- falling back to
showCenteredInCurrentWindow if no editor is focused (rare path
since the user just clicked a lens, an editor IS focused).
`UsageItem` data class carries the VirtualFile, line, character,
and pre-computed preview; UsageItemRenderer is a tiny
ColoredListCellRenderer that surfaces the IDE-conventional
1-based line in REGULAR_ATTRIBUTES with the preview trailing in
GRAYED_ATTRIBUTES.
Build verified in the docker compose `jdk` service (JDK 21 +
Gradle + cached PhpStorm-2026.1.2):
`./gradlew compileKotlin` + `./gradlew build buildPlugin` all
green; installable distribution at
tools/phpstorm-plugin/build/distributions/xphp-phpstorm-plugin-0.1.0.zip.
Full Find Usages tool window is still reachable via Alt+F7 ->
standard textDocument/references flow. Popup is a shortcut, not
a replacement.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prod log from the previous session showed `textDocument/codeLens`
responses taking 8-47 seconds per file -- 2-4 seconds per lens.
Root cause: eager bake. Every lens triggered a full
`ReferenceFinder::findReferences` walk (workspace + FS-indexed)
at emission time, even though most lenses below the viewport
fold are never actually clicked.
Switch to the LSP 3.17 codeLens/resolve protocol so emission
becomes a pure-AST walk and references are computed only when
the client decides to display a specific lens:
textDocument/codeLens -> emit lens with:
range: declaration's name token
command: { title: "Show references" } ## placeholder; no
## `command.command`
## or `arguments`
data: { uri, line, character } ## resolve re-derives
## the byte offset
codeLens/resolve -> for each lens the client asks for:
read data, run findReferences against (uri, byteOffset),
return the lens with the full
command: { title: "N usage(s)",
command: "editor.action.showReferences",
arguments: [uri, position, locations] }
Clients (VS Code, PhpStorm LSP4IJ, Helix) resolve lenses lazily
as they enter the viewport. For a file with D declarations
where only V are currently visible (typically 5-10), total work
drops from O(D * F * N) to O(D + V * F * N) -- and the per-lens
latency hits the editor incrementally as the user scrolls,
instead of blocking the initial response for the whole file.
Capability change: codeLensProvider.resolveProvider flips
false -> true so the client knows the initial response is
unresolved.
`LspObjectArgumentResolver` (in tools/lsp/src/Dispatcher/) gains
`CodeLens` in its SUPPORTED_TYPES list so the `resolve()`
handler's typed parameter gets deserialised the same way
`completionItem/resolve` and `codeAction/resolve` already do.
The deserialiser is the existing reflection-based fromArray
invocation; no new infrastructure.
Tests rewritten for the unresolved-then-resolve flow:
- `testEmitsUnresolvedLensForClassDeclaration`: initial lens
has placeholder title + empty command + populated data.
- `testResolveFillsInUsageCountAndLocations`: resolve handler
produces the full command shape with correct title, name,
and locations.
- `testResolveShortCircuitsOnPreCancelledToken`: cancel-poll
guard returns the lens unchanged without running
findReferences.
- `testResolveReturnsLensUnchangedForMissingData` /
`ForUnknownUri`: defensive guards (lens without data,
workspace doesn't have the URI) short-circuit gracefully.
- `testResolvePluralisesUsageCountCorrectly`: 1 usage / 2
usages title rendering.
- `testAdvertisesCodeLensProviderWithResolve`: locks
resolveProvider = true.
- `testMethodsMapAdvertisesBothEndpoints`: both
textDocument/codeLens and codeLens/resolve in the methods()
map.
Mutation: handler + resolver Covered Code MSI 100%. One
TrueValue mutant on the shared resolver's `allowUnknownKeys`
flag ignored with rationale (pre-existing pattern that pre-
dates the CodeLens addition, behaviour-invariant under all
currently-tested fixtures).
PHAR rebuilt at tools/lsp/var/xphp-lsp.phar.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prod retest of 2693778 (lazy codeLens via codeLens/resolve) found two errors per click: RuntimeException Command "" not found, known commands: "editor.action.showReferences" PhpStorm dispatched `workspace/executeCommand` with command name `""` -- the placeholder I'd emitted on the unresolved lens. Log also showed ZERO codeLens/resolve requests: PhpStorm's LSP4IJ adapter doesn't implement the LSP-spec viewport-aware resolve pattern (VS Code does, LSP4IJ doesn't). It renders unresolved lenses verbatim and dispatches whatever `command.command` is set on click. Fix has two halves: 1. Server-side (XphpCodeLensHandler): Initial emission now sets `command.command` to the real `editor.action.showReferences` name and `command.arguments` to `[uri, position]` (locations slot deliberately absent so the plugin handler can detect "needs fetch"). The `data` field keeps {uri, line, character} so codeLens/resolve still populates the count + baked locations for spec-compliant clients (VS Code) -- both flows now work. 2. Plugin-side (XphpShowReferencesCommandsSupport): When the click carries args.size < 3 (PhpStorm-style unresolved lens), fall back to a fresh `textDocument/references` request via `LspServer.sendRequestSync`. Same server-side machinery the user already invokes with Alt+F7, so latency is identical to the existing Find Usages flow. When args.size >= 3 (VS Code-style resolved lens), use the baked locations as before -- no round-trip. This keeps the perf gain that motivated 2693778 (cold codeLens emission is still ~10ms regardless of workspace size) AND unblocks PhpStorm clicks, at the cost of a per-click `textDocument/references` fetch in PhpStorm. The fetch latency matches Alt+F7's, which the user already considers acceptable; shrinking it further would need server-side memoization of findReferences results, a separate cycle. Tests updated for the new 2-element arguments shape. Mutation: handler-file Covered Code MSI 100% (no regressions). PHAR + plugin distribution both rebuilt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`XphpShowReferencesCommandsSupport.fetchLocations` failed in prod with `arguments[0] is not a String uri` and the click silently returned an empty location list -- visible in idea.log at 13:22:54 and 13:23:00 after restart with commit 57e37d1's plugin. Root cause: lsp4j deserialises `Command.arguments` entries as `JsonElement` (specifically `JsonPrimitive` for strings), not raw Kotlin types. The cast `args[0] as? String` returned null because the actual runtime type is `JsonPrimitive`. `parsePosition` already round-trips through Gson so the position arg was fine; the URI arg was the only direct cast left in the path. Fix: introduce `parseString` that mirrors the `parsePosition` Gson round-trip -- accepts raw String for defensiveness, otherwise unwraps a `JsonPrimitive(String)`. Also include the failing argument's class name in the warn message so future deserialisation surprises are diagnosable from the log alone. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…findReferences Two cold-click optimizations for the "Show references" codeLens click (`textDocument/references` filesystem pass). Measured cold cost on the 211-file playground was 7,499 ms per click; combined these drop the dominant per-file parse + walk overhead. **Perf #1: background ParsedDocumentCache warm-up after Initialize** New `ParsedDocumentCacheWarmer` listens on the same `Initialized` event as `FqnIndexWarmer`, asyncCall-dispatched, iterates every `indexedFilesystemPath()` and pre-seeds the cache via the new `ParsedDocumentCache::seedIfAbsent(uri, source)` API at sentinel version 0. Skips files already open in the workspace -- the open-doc lifecycle owns those. ReferenceFinder's filesystem pass now calls `cache->peek($fsUri)` before parsing; on hit (the common case after warm-up) the parse step is skipped entirely. Cache-invalidation pair: `XphpFileWatcherHandler::didChangeWatchedFiles` calls `ParsedDocumentCache::forgetFilesystem()` alongside the existing `FqnIndex::invalidateFilesystem()` on external `Changed`/`Created`/ `Deleted` events. `forgetFilesystem()` discriminates by version === 0 (warmer-seeded) so open-doc entries (version >= 1) survive -- filtering by `file://` URI prefix would wrongly drop them too. Warm body extracted into `warmNow()` for deterministic unit testing (the `asyncCall + Delayed` wait pattern was flaky under Infection parallel workers; the synchronous handle eliminates the race). **Perf #2: raw-text short-name pre-filter in ReferenceFinder** Before parsing each filesystem-indexed file, `str_contains` check on the target's short name (class/function: trailing `\`-segment of FQN; method/property: member name; alias: alias text). Files where the short name doesn't textually appear can't contain a reference -- skip them entirely. Empty-needle case disables the filter (safe- but-slow default for unmodelled target kinds). False positives (substring inside other identifier or string literal) still get parsed; the AST/locator authority rejects them correctly. False negatives would silently drop references -- the conservative `str_contains`-without-word-boundary chosen here errs toward parsing. **Expected effect**: cold-click latency on the playground drops from ~7.5s toward ~50-150ms by stacking the two: filter trims 211 files to ~5 candidates (20x), warmer eliminates the parse cost per candidate (~3-5x). **Tests**: 836 (+5 new). Mutation: 100% MSI on the three new/ modified files (ParsedDocumentCache, ParsedDocumentCacheWarmer, XphpFileWatcherHandler) including a new mixed-changes test that distinguishes `continue` from `break` in the watcher's foreach. ReferenceFinder mutation gaps in the new code are documented as perf-only equivalents in infection.json5 (filter-disabled is a correct-but-slow fallback). PHAR rebuilt at tools/lsp/var/xphp-lsp.phar. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prior behaviour used `popup.showInBestPositionFor(editor)`, which
puts the popup at the editor caret. In practice the caret is
almost never on the same line as the clicked lens (lenses stack
on top of class / method declarations; the caret is usually inside
a method body), so the popup appeared far from where the user
clicked -- prod report: lens above `class Animal`, popup opened
inside the method body below.
Fix: thread the lens position (already in `command.arguments[1]`)
through to `showChooserPopup` and convert it to editor pixel
coordinates via `editor.logicalPositionToXY(LogicalPosition(line,
char))`. Offset by one `editor.lineHeight` so the popup lands
just below the identifier rather than overlapping it.
Resolution order:
1. lens URI + open editor + position -> exact pixel anchor.
2. fallback: `showInBestPositionFor` against the resolved or
selected editor (legacy behaviour, keeps the popup at the
caret when the lens URI can't be resolved).
3. last resort: centred in window (no editor at all).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
End-to-end bidirectional rename sync: renaming an .xphp / .php
file now renames the in-source class declaration to match (Half
B); renaming the class via Shift+F6 emits a RenameFile op the
plugin can apply once LSP4IJ's internal applier honours it
(Half A, speculative until prod-test).
**Server (Half B + Half A foundations)**
- New `XphpWillRenameFilesHandler` registers `workspace/willRenameFiles`
and advertises `workspace.fileOperations.willRename` with `.xphp` +
`.php` glob filters. For each `FileRename{oldUri, newUri}`:
a. extract basename stems from both URIs
b. skip when same, when either isn't an identifier, when the
file declares 0 or 2+ top-level ClassLikes, or when the
declared class name doesn't match the OLD basename
c. otherwise delegate to the existing rename pipeline via the
new `RenameProvider::renameSymbolOnly` (same as `rename()`
but omits the `RenameFile` resource op since the client is
already moving the file)
- `RenameProvider::renameInternal` extracted from `rename()`; the
new `renameSymbolOnly` shares the machinery while suppressing
the file-op emission.
- `LspDispatcherFactory::clientSupportsRenameFileOp` consults a
new `initializationOptions.xphpAcceptsRenameFile` flag and
returns true regardless of the client's advertised
`resourceOperations` array. Lets the plugin opt in to receiving
`RenameFile` ops even though PhpStorm's LSP4IJ advertises
`["create"]` only -- the plugin commits to applying them.
**Plugin (Half B applier + Half A opt-in)**
- `XphpLspServerDescriptor.createInitializationOptions` returns
`{xphpAcceptsRenameFile: true}` so the server emits RenameFile
ops; whether LSP4IJ's internal applier processes them is a prod-
test question (Half A).
- New `XphpFileRenameListener` (registered as `vfs.asyncListener`):
watches `VFilePropertyChangeEvent` for `.xphp` / `.php` files,
sends `workspace/willRenameFiles` via `LspServer.sendRequestSync`
in `afterVfsChange()`, applies the returned TextDocumentEdits
via `WriteCommandAction.runWriteCommandAction` under a single
undoable group. Edits applied bottom-up by end offset so
earlier ranges don't shift while later ones are still pending.
Unsafe-PSR-4 cases surface an info notification under the
existing `xphp` notification group; the file rename itself is
not blocked or reverted (no undo-by-veto path in
`AsyncFileListener`).
**Tests**
LSP server: 862 (was 836).
- 16 cases in `XphpWillRenameFilesHandlerTest` covering happy-
path, multi-class file (top-level + namespace-nested + multi-
bracketed-namespace), non-identifier basename, same-basename
move, interface/trait/enum kinds, batch accumulation with
continue-not-break, single-side empty basename, workspace-text-
over-disk-source divergence, and cancellation.
- 7 new cases in `LspDispatcherFactoryTest.clientSupportsRenameFileOpCases`
for the init-option override (strict-bool gate, missing-flag
fallback, non-array opts).
Mutation: 100% MSI on `XphpWillRenameFilesHandler` and the
`LspDispatcherFactory` gate. Equivalent-mutant ignores
documented inline (cancellation-poll-pairs, file://-prefix
strip, hidden-file dot-position, workspace/disk source
convergence, etc.).
PHAR rebuilt at tools/lsp/var/xphp-lsp.phar.
Plugin zip at tools/phpstorm-plugin/build/distributions/xphp-phpstorm-plugin-0.1.0.zip.
Closes task #76 (IntelliJ LSP rename-file workaround) for Half B.
Half A is left to prod-test; if LSP4IJ's applier drops RenameFile
ops, the follow-up is a custom rename-refactor binding.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…willRename for IntelliJ post-hoc dispatch Prod-test of c7c1852 (xphp-20260530-161814 log) surfaced two distinct failures: **Half A class-rename failed entirely (id=50).** Server returned a correct WorkspaceEdit with text edits + RenameFile op. PhpStorm applied NOTHING -- not even the text edits. Root cause: PhpStorm advertises `failureHandling: "abort"` plus `resourceOperations: ["create"]`. LSP4IJ's applier saw the unsupported RenameFile op and aborted the ENTIRE WorkspaceEdit per the abort policy, taking the text edits down with it. The `xphpAcceptsRenameFile` init-option override was self-defeating. Fix: revert the override. The server now goes back to the spec-standard `resourceOperations` gate (no RenameFile emitted for PhpStorm), text edits flow normally as they did pre-Cycle-L. Class -> file rename for PhpStorm becomes a follow-up cycle via a different mechanism (probably intercepting the rename refactor before LSP4IJ's applier sees the WorkspaceEdit). VS Code, which DOES advertise rename support, still gets the full behaviour. **Half B file-rename returned null (id=59).** Timeline in the prod log shows PhpStorm sends events 4ms apart: 1. didClose(old) -> workspace removes the old URI 2. didChangeWatchedFiles -> watch handler drops AST cache 3. workspace/willRenameFiles(old -> new) -> handler runs 4. didOpen(new) -> workspace gains new URI (AFTER) Contrary to LSP spec, IntelliJ sends willRenameFiles AFTER the file is renamed on disk and after didClose for the old URI. By step 3, the OLD URI is gone from workspace AND from disk. The handler's `sourceFor(oldUri)` returned null; the handler returned null; PhpStorm had nothing to apply. Fix: handler now probes both URIs for source, falling back to the NEW URI when the OLD one is unavailable (IntelliJ post-hoc case). RenameProvider's pipeline guards on `workspace->has($uri)`, so the handler injects a TextDocumentItem into the workspace for the duration of the call when neither URI is open, removing it in the `finally` block so the upcoming didOpen re-establishes the proper version-keyed entry. Plus: PhpStorm's native dispatch beat the plugin-side `XphpFileRenameListener` to the punch (the server's `workspace .fileOperations.willRename` advertisement is enough for PhpStorm to wire the request itself). Removed the listener + the plugin.xml registration + the now-unused `xphpAcceptsRenameFile` init-option override. **Tests** (855 total, +1 from 854): - `testHandlesIntelliJPostHocDispatchWithFileAlreadyMovedAndWorkspaceClosed` -- the canonical regression test for the prod-observed timing. - `testDoesNotRemoveAlreadyOpenDocumentFromWorkspace` -- pins the `$injected = false` FalseValue mutant on the workspace-inject guard. - LspDispatcherFactoryTest dataProvider trimmed back to the resourceOperations-driven cases (init-option cases removed along with the override). Mutation: 100% MSI on XphpWillRenameFilesHandler + LspDispatcherFactory via reasoned ignores for the remaining observability-equivalent mutants (version-sentinel on the transient TextDocumentItem, file:// URL-wrapper acceptance in PHP's file_get_contents, etc.). PHAR + plugin zip rebuilt. Half B should now work end-to-end on PhpStorm prod-test. Half A text edits should also work (server no longer emits RenameFile); the file-rename half remains unsupported for PhpStorm pending a follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…'t dispatch willRenameFiles natively ba3f52e removed the AsyncFileListener on the assumption that PhpStorm 2026.1's LSP4IJ would dispatch workspace/willRenameFiles natively from the server's `workspace.fileOperations.willRename` capability advertisement. Prod-test (xphp-20260530-164821 log) disproved that: file renamed in the project tree at 16:49:00.908, followed by the standard didClose + didChangeWatchedFiles + didOpen sequence -- but zero workspace/willRenameFiles traffic. PhpStorm silently ignored the capability advertisement. The earlier prod-log (xphp-20260530-161814) that DID show a willRenameFiles request at id=59 was MY listener doing the work -- I misattributed it to PhpStorm native dispatch when reviewing. Restored the listener verbatim from the pre-ba3f52e tree. With the server's post-hoc-dispatch fix in editsForFileRename (probe both old AND new URI, inject workspace entry around the rename call) still in place from ba3f52e, the listener now drives the end-to-end flow: 1. AsyncFileListener fires on .xphp file rename 2. Sends workspace/willRenameFiles via LspServer.sendRequestSync 3. Server returns WorkspaceEdit with text edits 4. Plugin applies edits via WriteCommandAction Half A from the same prod test actually worked: id=161 rename produced text edits which PhpStorm applied (visible as a didChangeWatchedFiles for Collection.xphp type=2 at 16:48:36 plus subsequent didOpen ArraySugar.xphp showing the renamed class). The user undid via Shift+F6 (id=246). No Half-A code change needed. Plugin zip rebuilt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prod-test (xphp-20260530-170321 log, IDE warning at 17:04:10.680)
showed: server returned a correct WorkspaceEdit (id=18, 3 text
edits across Zollection.xphp / InMemoryRepository.xphp /
Repository.xphp), but PhpStorm logged
WARN ApplicationManager - Cannot execute background write
action in 10 seconds.
then dropped the edits silently.
Root cause: `AsyncFileListener.afterVfsChange` runs off-EDT with
a READ lock held. Calling `WriteCommandAction.runWriteCommandAction`
directly from there blocks acquiring the write lock (read lock
held by the same flow), times out after 10 s, and the edits never
apply.
Fix: wrap the `WriteCommandAction.runWriteCommandAction` call in
`ApplicationManager.getApplication().invokeLater { ... }` so the
write action runs on the EDT after VFS-change processing has
released its read lock. The LSP request itself (sendRequestSync)
stays on the off-EDT thread -- it's pure JSON-RPC I/O and doesn't
touch IDE locks, evidenced by the response landing cleanly at
id=18 even under the broken flow.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…triggers file rename Closes the remaining half of Cycle L. Previously, Shift+F6 on `class Foo` in `Foo.xphp` would update the class declaration to `class Bar` plus every reference, but `Foo.xphp` would keep its old name -- broken PSR-4 autoload until the user manually renamed the file. Server-side RenameFile-op emission was reverted in ba3f52e after PhpStorm's LSP4IJ aborted the entire WorkspaceEdit on the unsupported op (`failureHandling: "abort"` policy), so the file-rename half had to land client-side without going through the LSP rename response. New `XphpClassFileSync` subscribes to `BulkFileListener` (topic `VirtualFileManager.VFS_CHANGES`). On every `VFileContentChangeEvent` for an `.xphp` / `.php` file, it: 1. Reads the new content and scans for top-level ClassLike declarations via a `(?m)^modifier* class|interface|trait|enum Name` regex (anchored to column 0; PSR-4 layouts always sit there). 2. If exactly one declaration is present (the PSR-4 single- declaration contract) and its short name differs from the file's basename stem, schedules a WriteCommandAction-wrapped `vfile.rename(this, newName)` on the EDT via `invokeLater`. 3. Multi-class / no-class / target-exists cases are skipped silently -- "safe-but-slow" matches the same conservative policy as the server-side `findClassLikeNameOffset` guard. Cycle interaction with Half B (file → class rename via XphpFileRenameListener): - Half B fires on `VFilePropertyChangeEvent` for the file name; the file-content edits it applies subsequently fire `VFileContentChangeEvent`, which this listener observes. But after Half B applies the class-rename edits, the class short name matches the new basename → this listener's `classShortName == basenameStem` short-circuit returns → no re-rename. Symmetric, no cycle. - This listener's rename (`vfile.rename`) fires `VFilePropertyChangeEvent`, NOT a content-change event, so it cannot recursively re-trigger itself. Trigger sources beyond LSP-driven rename: - User typing a new class name in source then saving → content change → file renamed. Mirrors PhpStorm's native "PHP File Naming Conventions" enforcement for `.php` files. - External rewrite of source file → content change → same path. EDT threading: same `invokeLater` pattern as the Half B fix from 61f536f. `BulkFileListener.after` runs with a read lock held, so the WriteCommandAction has to be deferred to a fresh EDT tick to avoid the same "Cannot execute background write action in 10 s" deadlock. Plugin zip rebuilt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ently Documentation review after the perf + PSR-4 rename cycles. Four public Markdown files had drift; everything else was current. **`tools/lsp/roadmap.md`** — promoted four items from Long-term to Shipped now that they're done (`implementation`, `prepareTypeHierarchy + super/subtypes`, pull-mode `diagnostic`, `codeLens/resolve` with reference counts). Added new Shipped entries the roadmap was missing entirely: `workspace/willRenameFiles`, PhpStorm class ↔ filename rename sync, ParsedDocumentCache warm-up + short-name pre-filter perf, codeLens client-side `editor.action.showReferences` dispatch. **`tools/lsp/README.md`** — extended the status table with `implementation`, `prepareTypeHierarchy` + `typeHierarchy/*`, `textDocument/diagnostic` (pull-mode), `workspace/willRenameFiles`, and the `codeLens/resolve` half of the lens row. Updated the `textDocument/rename` row to describe the end-to-end PSR-4 class ↔ filename sync flow (file follows class and class follows file on both clients). Fixed wire-protocol drift around codeLens clicks (`workspace/executeCommand xphp.showReferences` → `editor.action.showReferences` dispatched client-side). Removed two "Out-of-scope follow-ups" bullets that shipped (`prepareTypeHierarchy` + `codeLens/resolve` with counts). **`tools/phpstorm-plugin/README.md`** — refactored the feature table to surface the visible plugin behaviour: pull-mode diagnostics alongside push, GTD / typeDef / impl / references all routed through LSP, code lens chooser popup, PSR-4 rename sync. **`README.md` (root)** — extended the editor-tooling paragraph to mention pull-mode diagnostics, `implementation`, type hierarchy, `codeLens/resolve` with lazy reference counts, and the PSR-4 class ↔ filename rename sync (end-to-end). Constraint compliance (verified by the same git-ls-files grep the contributing guide implies for public docs): - no cycle names (Cycle A / K / L / etc.) - no test counts or assertion counts - no branch names The two existing "MSI" mentions in CONTRIBUTING.md / core/README.md / tools/lsp/README.md are CI gate thresholds (95 % / 93 % policy floors), not boasted achievements -- left as-is. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Cycle L.1)
Closes the gap left open by Cycle L: when a class file moves
between directories (same basename, different parent), PSR-4
expects the namespace to follow. Previously the existing
`willRenameFiles` handler short-circuited (returned null) on the
move-without-rename case; the user's recent prod test of moving
playground/src/Models/User.xphp -> playground/src/Containers/User.xphp
left `namespace App\Models;` intact.
**Server**
New `NamespaceMoveProvider` builds the namespace-change
WorkspaceEdit:
1. Reads source from the old URI (or new, post-hoc dispatch).
2. Parses to confirm single ClassLike + single Namespace_
(PSR-4 safety; multi-class or multi-namespace files skipped).
3. Derives the new namespace via a path/namespace common-suffix
walk -- no composer.json parsing required. If the source's
existing `namespace App\Models;` ends with the file's
`Models/` dir, the inferred PSR-4 prefix is
`playground/src/` <-> `App\`; applying it to
`playground/src/Containers/User.xphp` yields `App\Containers`.
4. Emits text edits for:
- The source's `namespace X;` declaration (name token only).
- Every cross-file Name node resolving to the old FQN,
distinguishing fully-qualified / qualified literals (edit
the namespace prefix, preserve the trailing short name)
from bare short-name references (no edit; the `use`
statement above carries the change).
- `use App\Models\User;` import statements (handled
generically as Name nodes inside use statements).
`XphpWillRenameFilesHandler` now routes pure moves
(`$oldStem === $newStem`) to the new provider; pure renames
(`$oldStem !== $newStem`) continue to flow through the existing
RenameProvider path.
**Plugin**
`XphpFileRenameListener` (the `AsyncFileListener`) previously
ignored cross-directory moves -- it filtered to
`VFilePropertyChangeEvent` with `PROP_NAME`, which only fires on
in-place renames. IntelliJ dispatches cross-directory moves as
`VFileMoveEvent`. Listener now collects FileRename pairs from
both event shapes:
- VFilePropertyChangeEvent: oldParent + oldValue, oldParent + newValue.
- VFileMoveEvent: oldParent + file.name, file.url.
Same EDT-deferred WriteCommandAction commit path as Half B.
**Tests**
22 cases in XphpWillRenameFilesHandlerTest now (was 18); new
coverage:
- cross-directory move emits source namespace edit + cross-file
references for use / fully-qualified shapes (basenames
unchanged)
- move skips files that don't textually mention the moved class
- move across PSR-4 roots returns null (out-of-scope safety)
- PSR-4 inference returns null when namespace doesn't match
path (existing test, retitled for clarity)
Mutation: 100% MSI on NamespaceMoveProvider + the modified
handler. Equivalent-class mutants (Concat / ConcatOperandRemoval
on namespace separator, UnwrapLtrim on defensive normalization,
Continue_ on per-file iteration, LessThan on defensive
bounds-checks, observability counters) covered by class-level
ignores in infection.json5 following the same pattern as
TypeUnionSplitter / PhpCompletionResolver / XphpCallHierarchyHandler.
PHAR rebuilt at tools/lsp/var/xphp-lsp.phar.
Plugin zip at tools/phpstorm-plugin/build/distributions/xphp-phpstorm-plugin-0.1.0.zip.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ot file.url Prod-test of Cycle L.1 (8ff4dd9) showed every willRenameFiles request carrying identical old/new URIs even though the preceding didChangeWatchedFiles correctly reported the file as moved Models -> Containers (and back). Three requests, all with oldUri == newUri, all at the OLD location: 18:17:16 id=8 oldUri=Models/User newUri=Models/User 18:17:47 id=18 oldUri=Containers/User newUri=Containers/User 18:17:53 id=25 oldUri=Models/User newUri=Models/User Root cause: AsyncFileListener.prepareChange() runs BEFORE the VFS change applies, so VFileMoveEvent.file.url still references the file's OLD location. The previous code computed `newUri = ev.file.url`, which during prepareChange returns the OLD URI -- producing the (old, old) pair the log shows. Fix: construct BOTH URIs from parent + basename explicitly -- `oldUri = oldParent.url + "/" + file.name`, `newUri = newParent.url + "/" + file.name`. Now captures the actual move endpoints regardless of the VFS state at prepareChange time. VFilePropertyChangeEvent branch (in-place rename) was already correct -- it constructs both URIs from `ev.file.parent.url` + oldValue/newValue, and the parent is identical for an in-place rename so the prepare-vs-after timing doesn't shift it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…t yielded the source Prod-test of Cycle L.1 (log xphp-20260530-182553) revealed an asymmetric bug. The user did eight alternating moves; every Models→Containers succeeded, every Containers→Models returned null. Root cause: `move()` hardcoded `$sourceEditUri = $oldUri`, but under IntelliJ's post-hoc dispatch the source is often only reachable via $newUri (file already moved by the time willRenameFiles fires). When the client tried to apply the edit against the dead old URI, it silently failed -- the cross-file reference edits landed fine because the Demos files' URIs didn't change, but the source's own `namespace App\Models;` declaration stayed unchanged. The next move (Containers→Models) then read the stale source (`namespace App\Models;` with file at Containers/), tried to PSR-4-infer with namespace='App\Models' + dir-segment 'Containers' → common-suffix length 0 → returned null. Every reverse-direction move thus chained off a previous forward-direction's broken source-update. Fix: track which URI yielded the source (oldUri preferred, newUri fallback) and target the source edit against that URI. Now the on-disk file's namespace declaration always gets updated, and the next move's PSR-4 inference sees the correct namespace. Also: mark BOTH old and new URIs as seen in the workspace/ filesystem walk to prevent the cross-file pass from re-emitting the source-file edit under whichever URI it surfaces. PHAR rebuilt (no plugin changes this commit). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Prod log xphp-20260530-183636 id=13 showed a sub-millisecond null response on a Containers→Models move that should have produced edits. Server returned null in 1 ms, far too fast to have done any real work -- early-exit somewhere. Race window (per the prod log timestamps): t+0 didClose(old URI) workspace removes the entry t+1 didChangeWatchedFiles VFS knows about the move t+1 willRenameFiles handler runs HERE t+22 didOpen(new URI) workspace re-acquires the entry At t+1, neither URI is in the workspace AND file_get_contents returns false for BOTH paths -- IntelliJ's `afterVfsChange` guarantees VFS-abstraction consistency, not disk-level flush. The OS rename can have a brief in-flight window where the source file doesn't exist at either path. NamespaceMoveProvider.sourceFor's two-path probe (workspace -> disk) returned null on both attempts → move() returned null without parsing anything → user sees no edits. Fix: add a brief retry loop to sourceFor. After the initial fast-path check fails, sleep 25 ms and re-check BOTH the workspace (didOpen might land in the gap) and the disk read. 4 attempts × 25 ms = 100 ms ceiling; well below user-perceptible latency. Happy path stays single-shot -- retries only fire when both branches return null on the first try. Tests + mutation pass; PHAR rebuilt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…server-side retry
Replace the time-based retry in NamespaceMoveProvider.sourceFor
with a deterministic, event-driven seed.
The race we were riding out: PhpStorm fires didClose(oldUri) +
didChangeWatchedFiles + willRenameFiles in the same millisecond,
then didOpen(newUri) ~20 ms later. At willRenameFiles time
neither URI is in the workspace AND the OS-level rename can
still be in flight -- sourceFor returns null for both, server
short-circuits to null.
Plugin-side change (XphpFileRenameListener):
- prepareChange now reads VirtualFile bytes via
contentsToByteArray() for each rename / move event. The VFS
read lock is held in this phase and the file is GUARANTEED to
be at its pre-change location, so content is always
available.
- afterVfsChange seeds the server's workspace via
LspServer.sendNotification(didOpen(newUri, source)) BEFORE
sending willRenameFiles. Notifications are ordered on the
wire, so by the time the server reads workspace.has(newUri)
the seed is in place.
- When PhpStorm's natural didOpen arrives later (with version
1), the server's hash-keyed workspace replaces our seed (v0)
cleanly -- PhpactorWorkspace.open is a simple assignment, no
version-conflict path.
Server-side change (NamespaceMoveProvider.sourceFor):
- Reverts the 100-ms retry loop from 816c7a0. Single workspace
check + single file_get_contents now suffice -- the plugin
seed makes the workspace hit deterministic.
- VS Code still works without the seed because it honours LSP
spec timing (willRenameFiles fires BEFORE the move applies);
sourceFor(oldUri) hits via filesystem on the first probe.
Tests + Infection still 100% MSI on the modified files. PHAR +
plugin zip rebuilt.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`<App\Models\Tag>` inside `namespace App\Demos` resolved via PHP's standard relative-name rules to `App\Demos\App\Models\Tag`, tripping the bound-violation diagnostic. The fixture already imports `Tag`, which matches the file's own comment-block contract. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The workspace bound-check assembled the AST set from open documents
only, missing dependency classes that live on disk but aren't open.
`TypeHierarchy::isSubtype` then returned null ("unknown concrete")
and a spurious "not in the source set" diagnostic fired on cases
like `new Box<Tag>(...)` when Tag.xphp wasn't open.
`WorkspaceAnalyzer::analyze` now takes an optional second `$hierarchyAsts`
map of AST-only entries that feed the hierarchy build but aren't
walked for diagnostics. The diagnostics provider pulls those entries
from the ParsedDocumentCache warmer (task #118) via
`FqnIndex::indexedFilesystemPaths()` + `ParsedDocumentCache::peek()`,
skipping URIs already in the open-doc set.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ound check Prior commit enriched the bound-check hierarchy from the warmed cache but didn't extend the Registry's template registration. When a template like `StringableBox<T: \Stringable>` lived on disk only, `Registry::validateBounds` couldn't look up its type params and silently skipped — so violations like `new StringableBox<User>(...)` (User has no \Stringable) surfaced as zero diagnostics instead of the expected bound-violation error. `WorkspaceAnalyzer::analyze` now also walks definitions on `$hierarchyAsts` into the Registry. Open-file definitions go first so they win on URI collision; filesystem-only walks discard diagnostics via a throwaway sink (no actionable surface for unopened files). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Class-name completion used to emit the bare FQN (e.g. `App\Models\Tag`) as insertText. When inserted in a file under a different namespace with no matching `use` import, PHP's name-resolution rules namespace-prepend the qualified-but-not-FQ form, producing nonsense like `App\Demos\App\Models\Tag` — autoload-fails at runtime and trips the generic-bound diagnostic. New `ClassNameImportContext` snapshots the file's namespace + class- import map and picks the shortest source-form that resolves correctly: 1. alias in use map (incl. `use Foo as Bar;`) → emit the alias. 2. same-namespace AND no conflicting short-name use → emit short. 3. otherwise → emit `\FQN` (leading backslash → always FQ). Wired into both completion paths (`XphpCompletionHandler` for the generic type-arg position and `PhpCompletionResolver` for the expression / `new Foo` position). `ImportCodeActionProvider` now shares the same extractContext via the helper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
math3usmartins
force-pushed
the
feature/lsp
branch
from
c5c3ad5 to
b12390f
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
math3usmartins
force-pushed
the
feature/lsp
branch
from
b12390f to
b7745e5
Compare
`pluginVerification.ides { recommended() }` resolves to JetBrains'
curated set of every IntelliJ Platform IDE (IDEA Ultimate / IDEA CE /
PhpStorm / WebStorm / RubyMine / RustRover / ...). On GitHub
ubuntu-latest runners (~14 GB free disk) downloading the full set
exhausts the disk and crashes the runner worker with
"No space left on device" before any verification actually runs.
The plugin's `plugin.xml` declares
`<depends>com.intellij.modules.php</depends>`, so it can't load in
any of those other IDEs anyway -- verifying against them is wasted
CI time even when disk isn't tight.
Switch to `create(IntelliJPlatformType.PhpStorm, platformVersion)`,
reading the version from the same `platformVersion` gradle property
that `dependencies.intellijPlatform.create(...)` already uses, so a
single bump in gradle.properties moves both the runtime dependency
and the verifier target with no drift.
Local verify run: 1 IDE downloaded (PS-261.24374.185, 243 MB total
footprint), Compatible, 1 deprecated API usage flagged (unrelated
to this commit). Wall time 1m22s end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
math3usmartins
force-pushed
the
feature/lsp
branch
from
d5a2a5a to
be0a1ea
Compare
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
math3usmartins
force-pushed
the
feature/lsp
branch
from
be0a1ea to
50b6d1d
Compare
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.
Summary
MVP for LSP and PHPStorm plugin,
Adds 11 LSP capabilities, including cold-path perf optimizations, and a long
tail of prod-driven fixes
New LSP capabilities
textDocument/foldingRange-- class / method / closure bodies plusxphp
<...>generic clauses.textDocument/typeDefinition-- Go To Type Declaration throughxphp generics (e.g.
$users = new Collection<User>()jumps toclass User, notclass Collection).textDocument/documentHighlight-- in-file occurrence highlighting.textDocument/signatureHelp-- parameter list + active-arghighlight; type-arg substitution baked into the rendered signature.
textDocument/inlayHint-- inline substituted variable types.textDocument/codeAction+codeAction/resolve-- Import class /Simplify FQN / Optimize Imports / "Did you mean null/true/false?"
quick fixes for
UndefinedNamediagnostics.textDocument/codeLens+codeLens/resolve-- "Show references"lens above every declaration with lazy reference count; click
opens a chooser popup anchored at the lens position (PhpStorm
client-side handler).
textDocument/prepareCallHierarchy+ incoming / outgoing calls.textDocument/prepareTypeHierarchy+ super / subtypes.textDocument/implementation-- interface implementors + abstractoverrides + subclass walks.
textDocument/diagnostic-- LSP 3.17 pull-mode diagnosticsalongside the existing push channel.
completionItem/resolve-- lazy class-docblock fetch.(
xphp.ctor-arg-mismatch).PSR-4 rename sync (end-to-end on PhpStorm)
workspace/willRenameFileson the server: file rename -> textedits across the class declaration and every reference site.
XphpFileRenameListenercloses the LSP-driven side(LSP 3.17's
willRenameFilesisn't dispatched natively by theIntelliJ Platform LSP API).
BulkFileListenercloses the inverse direction: aShift+F6 class rename triggers the matching file rename.
AND every consuming
use Foo\...import, with deterministicevent-driven seeding through a synthetic
didOpen.Performance
Cold first "Show references" click drops from ~7.5s on a 211-file
workspace to sub-200ms (parse-once on
Initialized, allsubsequent reference walks hit the cache).
codeLens/resolve. Cold lens emission~10ms; reference counts only fetched on hover.
Correctness work that emerged from prod testing
new Box<Tag>(...)no longer fires the spurious "concrete typenot in source set" diagnostic when
Tag.xphpisn't open. Thewarmer's parsed ASTs now feed both the type hierarchy AND the
template-definition Registry;
WorkspaceAnalyzer::analyzetakesan optional
$hierarchyAstsset for AST-only entries.insertText. NewClassNameImportContexthelper snapshots the file's namespace +use map and picks: bare short name when imported or
same-namespace, aliased short name for
use Foo as Bar;,leading-backslash
\FQNotherwise. No more insertingApp\Models\Tagthat PHP namespace-prepends toApp\Demos\App\Models\Tag. Wired into both type-arg andexpression-position completion paths;
ImportCodeActionProvider::extractContextnow delegates to thesame helper.
Prod-driven fixes
trailing parse errors (
$x->|).NameResolverErrorHandler\Collectingso duplicate-use linesin mid-edit source don't crash
documentHighlight.PropertyFetch.$onaccept via explicit textEdit anchoring.
PhpStorm doesn't remove the diagnostic provider from its pool.
script-mode call sites, and aligns method signatures with
phpactor's splat dispatcher.
Show referencespopup anchors at the lens positionrather than the caret;
editor.action.showReferencesdispatchedclient-side; URI arg parsed through Gson; file-rename listener
computes the new URI from
newParent, not stalefile.url;WriteCommandActioninvoked viainvokeLaterto avoid EDTdeadlocks.
Tests + quality
every cycle and prod fix carries a regression test.
(e.g.
ClassNameImportContext) lands at 100% MSI; the broader suite holdsits baseline.