0.2.5#12
Merged
Merged
Conversation
A nullsafe access on a generic result lost its nullability: the inferred type of `$users->first()?->name` (where `first(): ?T` => ?User and `User::$name: string`) came back as `string` instead of `?string`. The `?->` operator short-circuits to null when the receiver is null, so the expression is always `<memberType>|null`. resolvePropertyFetch and resolveMethodCall now OR the member's declared nullability with the nullsafe operator: a `NullsafePropertyFetch` / `NullsafeMethodCall` result is nullable regardless of the member's own type. A regular `->` access is unchanged (a null receiver there is a runtime error, not a widened type). NullsafeMethodCall is now also routed through inferType and handleAssign so `$x?->method()` is tracked. This feeds every consumer of resolveVariable (hover, inlay hints, completion). Covers the property and the non-nullsafe (no-widen) cases in GenericResolverTest plus a hover behat scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`$users->first()?->bestFriend?->name` resolved to null (unresolved) instead of `?string`. Two gaps: 1. inferType didn't dispatch PropertyFetch / NullsafePropertyFetch receivers, so a property access used as a chain receiver collapsed to null. It now delegates to resolvePropertyFetch (which recurses into its own receiver, terminating at a Variable / New_ / *Call leaf). 2. A property typed as a bare class name (`?User`) resolved to an unqualified TypeRef (the resolver runs no NameResolver), so the next hop's class lookup failed. New qualifyAgainstNamespace qualifies a bare member-type name against the declaring class's namespace, accepting the candidate only when the lookup confirms it -- an unconfirmable name is left bare so a chain degrades to null rather than fabricating a wrong FQN. As a bonus, terminal bare-class property hovers now render the qualified FQN. Per-hop nullsafe nullability already propagates, so deep chains end up nullable. Covers deep/triple-hop chains, the qualified-FQN terminal case, union-intermediate and $this degradation, plus a hover behat scenario. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…name context)
Foundation for resolving bare class names in a class's member types against
the file where that class is DECLARED (use-map + namespace), including
cross-namespace `use` imports -- not the referencing call site.
- New ClassLikeContext value object {classLike, useMap, namespace}.
- ClassLikeLookup gains findWithContext(); implemented in Workspace
(reuses the cached AST), Filesystem (new FqnIndex::classLikeAstFor that
returns the declaring file's AST, both branches routed through
ParsedDocumentCache so chains don't re-parse per hop), and Composite
(delegates).
- GenericResolver::useMapAndNamespaceFor made public so the lookups can
derive the context.
Pure structural addition -- find() is unchanged and nothing consumes
findWithContext yet (wired in the next commit). Full suite green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…-map W2: resolvePropertyFetch now resolves a bare class name in a property type against the DECLARING file's use-map + namespace (via findWithContext), not just the declaring class's own namespace. A property typed with a cross-namespace `use` import (`use App\Other\Profile; public ?Profile`) now resolves through chains -- `$users->first()?->profile?->bio` => ?string -- and renders the real FQN (App\Other\Profile) instead of collapsing. Replaces the same-namespace-only qualifyAgainstNamespace with a use-map aware qualifyTypeRef (reuses resolveNameWithUseMap), keeping the find()-confirmed-only guard so an unconfirmable name stays bare and the chain degrades to null rather than fabricating a wrong FQN. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
W3: resolveMethodCall no longer bails to null on a non-generic receiver (empty paramMap). It now qualifies the return type against the method's declaring file (W1/W2) and OWNS the result -- so chains continue, e.g. `$users->first()?->mirror()?->name` => ?string where `mirror(): ?User` is a plain method on the non-generic User. Conservative gate to limit the shift from worse-reflection: for a non-generic method, only return non-null when the result is a relative type (static/self/parent) or a lookup-confirmed class. Scalars and unconfirmable names still return null so terminal hovers and single plain calls keep worse-reflection's richer view. The independent bail in resolveMethodCallSubstitutionAt is left intact (no generic inlay on plain methods). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
W4: findEnclosingMethodCallNameAt now matches NullsafeMethodCall too, so go-to-definition resolves a method reached via `?->` and through a chain whose intermediate hop is a non-generic method (`$users->first()?->mirror()?->mirror()` -> User::mirror, via W3). Both call sites (resolveMethodCallSubstitutionAt, resolveMethodDeclarationAt) already use only ->var/->name, which NullsafeMethodCall also exposes. Covered by a chained-method GTD phpunit test. (A behat scenario was omitted: overriding a Background fixture leaves the warmed FQN index stale, unrelated to the fix.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…eiver
Add a conservative `xphp.null-deref` diagnostic (Warning) that flags a
plain-`->` property/method access whose immediate receiver is a statically
nullable chained sub-expression -- e.g. `$users->first()->name` where
`first(): ?T`. The shape guarantees no inline guard is syntactically possible
and no bare variable to flow-narrow, so the inferred nullability is reliable.
Deliberately narrow to keep false positives near zero:
- only plain `->` accesses (a `?->` access is already null-safe);
- the receiver must itself be a `->`/`?->` method-call or property-fetch
(bare-variable receivers like `$x->y` are deferred to a future narrowing
pass);
- unmodellable receivers infer null and nothing fires.
`GenericResolver::findNullDerefSites` walks the AST and reuses the existing
scope/binding machinery (`bindingsAt` promoted to public static) + `inferType`.
The provider gains an OPTIONAL `?GenericResolver` ctor param, so resolver-less
unit contexts skip the diagnostic unchanged; `LspDispatcherFactory` passes the
session resolver. Sites carry stripped-source offsets, mapped back to the
buffer via `ParseResult::$byteOffsetMap` then to LSP via the cached PositionMap.
Also backfill behat coverage gaps for prior work (W8): a cross-namespace
property-type chain hover (W2) and go-to-definition through a nullsafe method
chain (W4, self-contained fixtures to dodge warmed-index staleness), plus the
two new diagnostics scenarios.
serverInfo/version intentionally left at 0.2.4 (owner cuts the release).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Every range a same-document handler emits must sit inside the analyzed
buffer; a strict client annotator (PhpStorm) throws "Range must be inside
element being annotated" when one lands past EOF mid-edit. The handlers
already self-clamp via PositionMap::offsetToPosition, so this is a test-only
sweep that fails loudly if a future edit regresses -- no production change.
Adds, for the six same-document range emitters (semantic tokens, folding,
document symbol, code lens, inlay hint, document highlight):
- one phpunit `...WithinDocumentBounds` method per handler test, via the
existing AssertsRangeWithinDocument trait. Semantic tokens decode the
delta stream to (line,char)..(line,char+length); folding checks the line
span; document symbol recurses range + selectionRange + children; inlay
checks its position as a zero-width range.
- six reusable behat steps (`every <kind> ... is within the bounds of
:path`) wired into one representative scenario each, using
World::rangeWithinDocument (clamp-and-compare) and lengthInUtf16.
Cross-file navigation handlers (definition/references/type-definition/
implementation/type- and call-hierarchy) are intentionally out of scope:
their ranges live in target files, not the annotated buffer.
W5 (array-access receiver inference) is parked: it needs array element-type
tracking that exists nowhere -- the parser lowers `T[]`/`Name[]` to a bare
`array` hint and the compiler keeps no element type (PHP has no typed
arrays). Documented in .claude/plans for if it's ever needed.
phpunit 1084, behat 104 scenarios / 653 steps, green. serverInfo stays
0.2.4 (owner cuts the release).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
serverInfo.version 0.2.4 -> 0.2.5 for the new release, which carries the nullable/chain type-inference work (W1-W4), the xphp.null-deref diagnostic (W6), and the range-bounds invariant test sweep (W7). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.