0.2.4

features/understand/hover.feature

@@ -103,3 +103,82 @@ Feature: Hover
     And the FQN index has been warmed on initialize
     When I request "textDocument/hover" on "fresh" at line 3 of "/Use.xphp"
     Then the hover contents contain "fresh(int $v): App\Builder<int>"
+
+  Scenario: Hover over a generic property substitutes the type parameter
+    Given the file at "/Plastic.xphp" contains the following lines:
+      """
+      <?php
+      namespace App;
+      class Plastic {}
+      """
+    And the file at "/User.xphp" contains the following lines:
+      """
+      <?php
+      namespace App;
+      class User {}
+      """
+    And the file at "/Pair.xphp" contains the following lines:
+      """
+      <?php
+      namespace App;
+      class Pair<A, B>
+      {
+          public function __construct(public A $first, public B $second) {}
+          public function swap(): Pair<B, A> { return new Pair::<B, A>($this->second, $this->first); }
+      }
+      """
+    And the file at "/Use.xphp" contains the following lines:
+      """
+      <?php
+      use App\Pair;
+      use App\Plastic;
+      use App\User;
+      $pair = new Pair::<Plastic, User>(new Plastic(), new User());
+      echo $pair->first;
+      """
+    And the FQN index has been warmed on initialize
+    When I request "textDocument/hover" on "first" at line 5 of "/Use.xphp"
+    Then the hover contents contain "App\Plastic $first"
+
+  Scenario: Hover over a property through a chained generic method call substitutes the type parameter
+    Given the file at "/Plastic.xphp" contains the following lines:
+      """
+      <?php
+      namespace App;
+      class Plastic {}
+      """
+    And the file at "/User.xphp" contains the following lines:
+      """
+      <?php
+      namespace App;
+      class User {}
+      """
+    And the file at "/Map.xphp" contains the following lines:
+      """
+      <?php
+      namespace App;
+      class Map<K, V> {}
+      """
+    And the file at "/Pair.xphp" contains the following lines:
+      """
+      <?php
+      namespace App;
+      class Pair<A, B>
+      {
+          public function __construct(public A $first, public B $second) {}
+          public function swap(): Pair<B, A> { return new Pair::<B, A>($this->second, $this->first); }
+      }
+      """
+    And the file at "/Use.xphp" contains the following lines:
+      """
+      <?php
+      use App\Map;
+      use App\Pair;
+      use App\Plastic;
+      use App\User;
+      $nested = new Pair::<Map<string, int>, Pair<Plastic, User>>(new Map(), new Pair::<Plastic, User>(new Plastic(), new User()));
+      echo $nested->swap()->first;
+      """
+    And the FQN index has been warmed on initialize
+    When I request "textDocument/hover" on "first" at line 6 of "/Use.xphp"
+    Then the hover contents contain "Pair<App\Plastic, App\User> $first"

features/validate/diagnostics.feature

@@ -25,6 +25,20 @@ Feature: Diagnostics
     When I analyze "/Broken.xphp" for diagnostics
     Then a "xphp.parse" diagnostic is reported saying "Syntax error"
 
+  Scenario: An EOF-anchored syntax error stays within document bounds
+    # Regression for the PhpStorm "Range must be inside element being annotated"
+    # crash: an unterminated block comment yields an EOF-anchored error whose raw
+    # end column lands one past EOL. The emitted range must be clamped.
+    Given the file at "/Unterminated.xphp" contains the following lines:
+      """
+      <?php
+      /* unterminated
+      """
+    And the FQN index has been warmed on initialize
+    When I analyze "/Unterminated.xphp" for diagnostics
+    Then a "xphp.parse" diagnostic is reported saying "Syntax error"
+    And every reported diagnostic range is within document bounds
+
   Scenario: Warn about an undefined bareword
     Given the file at "/Typo.xphp" contains the following lines:
       """

src/Analyzer/Analyzer.php

@@ -104,6 +104,13 @@ private static function buildParseErrorDiagnostic(
         }
         $startLine = PositionMap::lspLineFromNikic($e->getStartLine());
         $endLine = PositionMap::lspLineFromNikic($e->getEndLine());
+        // nikic columns are 1-based BYTE columns and, for an EOF-anchored error,
+        // `endColumn` is `lineLength + 1` (one past EOL). Clamp the whole range
+        // into the buffer so we never hand the client a range outside the
+        // document it can render -- a strict LSP annotator (PhpStorm) throws on
+        // an out-of-bounds range mid-edit.
+        [$startLine, $startCharacter, $endLine, $endCharacter] =
+            $positionMap->clampRange($startLine, $startCharacter, $endLine, $endCharacter);
         return new Diagnostic(
             startLine: $startLine,
             startCharacter: $startCharacter,

src/Diagnostics/XphpDiagnosticsProvider.php

@@ -11,7 +11,10 @@
 use Phpactor\LanguageServer\Core\Server\ClientApi;
 use Phpactor\LanguageServer\Core\Workspace\Workspace as PhpactorWorkspace;
 use Phpactor\LanguageServerProtocol\Diagnostic as LspDiagnostic;
+use Phpactor\LanguageServerProtocol\Position;
+use Phpactor\LanguageServerProtocol\Range;
 use Phpactor\LanguageServerProtocol\TextDocumentItem;
+use XPHP\Lsp\Analyzer\Diagnostic;
 use XPHP\Lsp\Analyzer\ParsedDocumentCache;
 use XPHP\Lsp\Analyzer\WorkspaceAnalyzer;
 use XPHP\Lsp\Reflection\FqnIndex;
@@ -115,10 +118,16 @@ private function computeAllOpenDiagnostics(TextDocumentItem $textDocument): arra
         // workspace pass, covering the current document plus every OTHER open
         // document. Documents that fail to parse contribute their syntax
         // diagnostics but are kept out of the workspace pass (unusable AST).
+        // Per-URI (version, source) so the workspace-diagnostics pass below can
+        // clamp its ranges against the same buffer the diagnostics were computed
+        // from (reusing the cached PositionMap).
+        $docMeta = [$currentUri => ['version' => $textDocument->version, 'source' => $textDocument->text]];
         $perFileByUri = [
-            $currentUri => array_map(
-                static fn ($d): LspDiagnostic => DiagnosticTranslator::toLsp($d),
+            $currentUri => $this->toLspClamped(
                 $currentResult->diagnostics,
+                $currentUri,
+                $textDocument->version,
+                $textDocument->text,
             ),
         ];
         $parsedFiles = [];
@@ -130,9 +139,12 @@ private function computeAllOpenDiagnostics(TextDocumentItem $textDocument): arra
                 continue;
             }
             $otherResult = $this->cache->getOrParse($uri, $item->version, $item->text);
-            $perFileByUri[$uri] = array_map(
-                static fn ($d): LspDiagnostic => DiagnosticTranslator::toLsp($d),
+            $docMeta[$uri] = ['version' => $item->version, 'source' => $item->text];
+            $perFileByUri[$uri] = $this->toLspClamped(
                 $otherResult->diagnostics,
+                $uri,
+                $item->version,
+                $item->text,
             );
             if ($otherResult->ast !== null) {
                 $parsedFiles[$uri] = ['ast' => $otherResult->ast, 'source' => $item->text];
@@ -176,16 +188,51 @@ private function computeAllOpenDiagnostics(TextDocumentItem $textDocument): arra
 
         $result = [];
         foreach ($perFileByUri as $uri => $perFile) {
-            $workspaceDiagnostics = array_map(
-                static fn ($d): LspDiagnostic => DiagnosticTranslator::toLsp($d),
+            $meta = $docMeta[$uri] ?? ['version' => 0, 'source' => ''];
+            $workspaceDiagnostics = $this->toLspClamped(
                 $workspaceByUri[$uri] ?? [],
+                $uri,
+                $meta['version'],
+                $meta['source'],
             );
             $result[$uri] = array_merge($perFile, $workspaceDiagnostics);
         }
 
         return $result;
     }
 
+    /**
+     * Translate internal diagnostics to LSP-wire diagnostics, clamping every
+     * range into the document's bounds first. The server must never emit a range
+     * outside the buffer it analyzed: a strict LSP annotator (PhpStorm) throws
+     * `Range must be inside element being annotated` when an EOF-anchored
+     * diagnostic lands one character past end-of-document mid-edit. Uses the
+     * cached PositionMap so we don't re-scan the source.
+     *
+     * @param  list<Diagnostic> $diags
+     * @return list<LspDiagnostic>
+     */
+    private function toLspClamped(array $diags, string $uri, int $version, string $source): array
+    {
+        if ($diags === []) {
+            return [];
+        }
+        $positionMap = $this->cache->positionMap($uri, $version, $source);
+        $out = [];
+        foreach ($diags as $d) {
+            $lsp = DiagnosticTranslator::toLsp($d);
+            [$sl, $sc, $el, $ec] = $positionMap->clampRange(
+                $lsp->range->start->line,
+                $lsp->range->start->character,
+                $lsp->range->end->line,
+                $lsp->range->end->character,
+            );
+            $lsp->range = new Range(new Position($sl, $sc), new Position($el, $ec));
+            $out[] = $lsp;
+        }
+        return $out;
+    }
+
     /**
      * Push `textDocument/publishDiagnostics` for every open document OTHER than
      * the one being linted whose diagnostics changed since we last published

src/LspDispatcherFactory.php

@@ -376,7 +376,7 @@ public function create(MessageTransmitter $transmitter, InitializeParams $initia
             new ErrorHandlingMiddleware($this->logger),
             new InitializeMiddleware($handlers, $eventDispatcher, [
                 'name' => 'xphp-lsp',
-                'version' => '0.2.3',
+                'version' => '0.2.4',
             ]),
             new ShutdownMiddleware($eventDispatcher),
             new ResponseHandlingMiddleware($responseWatcher),

src/PositionMap.php

@@ -163,10 +163,64 @@ public function fullLineRangeFromNikic(int $nikicLine): array
         if ($line >= count($this->lineOffsets)) {
             return [$line, 0, $line, 0];
         }
+        return [$line, 0, $line, $this->lineUtf16Length($line)];
+    }
+
+    /**
+     * UTF-16 code-unit length of the visible content of a 0-based line (the
+     * terminating `\n` is excluded). `$line` MUST be a valid line index
+     * (`0 .. count($lineOffsets) - 1`); callers that may exceed the range
+     * should go through {@see clampPosition} first.
+     */
+    private function lineUtf16Length(int $line): int
+    {
         $lineStart = $this->lineOffsets[$line];
+        // For the last line there is no following entry; +1 here is consumed
+        // by the `- 1` below so the slice runs to end-of-source. For any other
+        // line the next entry sits just past that line's `\n`, so `- 1` drops
+        // the terminator and we measure visible content only.
         $nextLineStart = $this->lineOffsets[$line + 1] ?? strlen($this->source) + 1;
         $lineText = substr($this->source, $lineStart, $nextLineStart - $lineStart - 1);
-        return [$line, 0, $line, self::toLspCharacter($lineText)];
+        return self::toLspCharacter($lineText);
+    }
+
+    /**
+     * Clamp an LSP {line, character} into this document's bounds: line into
+     * `[0, lastLine]` and character into `[0, lineLength]` (lengths in UTF-16
+     * code units, the LSP unit). An already-in-bounds position is returned
+     * unchanged.
+     *
+     * Diagnostic ranges built from parser column info can land one past EOL at
+     * EOF (nikic reports `endColumn == lineLength + 1` for an EOF-anchored
+     * error); some strict clients (PhpStorm's LSP annotator) throw when asked
+     * to render a range outside the document. Clamping here guarantees the
+     * server never emits an out-of-buffer range.
+     *
+     * @return array{0: int, 1: int} [line, character]
+     */
+    public function clampPosition(int $line, int $character): array
+    {
+        $lastLine = count($this->lineOffsets) - 1;
+        $line = max(0, min($line, $lastLine));
+        $character = max(0, min($character, $this->lineUtf16Length($line)));
+        return [$line, $character];
+    }
+
+    /**
+     * Clamp both endpoints of an LSP range into this document's bounds via
+     * {@see clampPosition}, then normalise so the end never precedes the start
+     * (an inverted range collapses to a zero-width range at the start).
+     *
+     * @return array{0: int, 1: int, 2: int, 3: int} [startLine, startChar, endLine, endChar]
+     */
+    public function clampRange(int $startLine, int $startCharacter, int $endLine, int $endCharacter): array
+    {
+        [$sl, $sc] = $this->clampPosition($startLine, $startCharacter);
+        [$el, $ec] = $this->clampPosition($endLine, $endCharacter);
+        if ($el < $sl || ($el === $sl && $ec < $sc)) {
+            [$el, $ec] = [$sl, $sc];
+        }
+        return [$sl, $sc, $el, $ec];
     }
 
     /**

src/Resolver/GenericResolver.php

@@ -593,6 +593,50 @@ public function resolvePropertyReceiverClassAt(string $uri, int $byteOffset): ?s
         return $fqn;
     }
 
+    /**
+     * Resolve the substituted concrete type of the property access at
+     * `$byteOffset`, rendered (e.g. `App\Containers\Pair<App\Models\Plastic,
+     * App\Models\User>`).  This is the member-hover parallel of the
+     * assignment-tracking path: where `resolvePropertyReceiverClassAt` answers
+     * "which class hosts this property", this answers "what concrete type does
+     * the property have once the receiver's type-args are substituted in" --
+     * so hovering `$pair->first` shows the substituted type instead of the
+     * declared type-param `A`.
+     *
+     * Returns null when the cursor isn't on a property-name token, the
+     * receiver isn't a tracked generic instantiation, or the property type
+     * isn't a shape we can model (union / intersection -- caller falls back
+     * to the unsubstituted render).
+     */
+    public function resolvePropertyTypeAt(string $uri, int $byteOffset): ?string
+    {
+        if (!$this->workspace->has($uri)) {
+            return null;
+        }
+        $item = $this->workspace->get($uri);
+        $scopes = $this->scopesFor($uri, $item->version, $item->text);
+        $bindings = self::bindingsAt($scopes, $byteOffset);
+
+        $result = $this->documents->getOrParse($uri, $item->version, $item->text);
+        if ($result->ast === null) {
+            return null;
+        }
+        $fetch = self::findEnclosingPropertyFetchNameAt($result->ast, $byteOffset);
+        if ($fetch === null) {
+            return null;
+        }
+        [$useMap, $namespace] = self::useMapAndNamespaceFor($result->ast);
+        $resolved = self::resolvePropertyFetch(
+            $fetch,
+            $bindings,
+            $this->classes,
+            $this->fqnIndex,
+            $useMap,
+            $namespace,
+        );
+        return $resolved?->render();
+    }
+
     /**
      * Walk the AST looking for a `PropertyFetch` or `NullsafePropertyFetch`
      * whose property-name identifier covers `$byteOffset`.  The cursor

src/Resolver/PhpHoverResolver.php

@@ -171,6 +171,12 @@ private function resolveInner(string $uri, int $line, int $character, ?Cancellat
         $propertyReceiver = $this->genericResolver->resolvePropertyReceiverClassAt($uri, $offset)
             ?? self::containerOrNull($context)
             ?? '';
+        // Substituted concrete type of the property at the cursor, e.g.
+        // hovering `$pair->first` on a `Pair<Plastic, User>` receiver yields
+        // `Pair<Plastic, User>` instead of the declared type-param `A`. Null
+        // when the receiver isn't a tracked generic instantiation -- the
+        // renderer falls back to the declared (prettified) type.
+        $propertyType = $this->genericResolver->resolvePropertyTypeAt($uri, $offset);
         $markdown = match ($symbol->symbolType()) {
             Symbol::CLASS_    => $this->fanOutRender(
                                     self::preferType($context, $symbol->name()),
@@ -195,6 +201,7 @@ private function resolveInner(string $uri, int $line, int $character, ?Cancellat
                                     fn (string $fqn): ?string => $this->renderProperty(
                                         $fqn,
                                         $symbol->name(),
+                                        $propertyType,
                                     ),
                                 ),
             Symbol::CONSTANT,
@@ -347,7 +354,7 @@ private function renderMethod(string $classFqn, string $methodName, ?MethodCallS
         return self::format($signature, $docblock);
     }
 
-    private function renderProperty(?string $classFqn, string $propertyName): ?string
+    private function renderProperty(?string $classFqn, string $propertyName, ?string $substitutedType = null): ?string
     {
         if ($classFqn === null) {
             return null;
@@ -364,7 +371,13 @@ private function renderProperty(?string $classFqn, string $propertyName): ?strin
         }
         $visibility = (string) $property->visibility();
         $static = $property->isStatic() ? 'static ' : '';
-        $type = $this->genericParams->prettify((string) $property->inferredType());
+        // When the cursor is on a property access through a tracked generic
+        // receiver (`$pair->first` with `$pair: Pair<Plastic, User>`),
+        // GenericResolver has already substituted the type-params, so the
+        // declared `A` renders as the concrete `Pair<Plastic, User>`.  Fall
+        // back to prettify(inferredType) when there's no substitution (plain
+        // property, untracked receiver, union/intersection type).
+        $type = $substitutedType ?? $this->genericParams->prettify((string) $property->inferredType());
         $signature = sprintf(
             "// %s\n%s %s%s\$%s",
             $classFqn,