Merge pull request #63 from 74nu5/fix/lsp-review5

Enter for-loops when collecting references, and pick the segment under the cursor

Author: 74nu5

src/Settex.LanguageServer/SettexHoverHandler.cs

@@ -105,7 +105,7 @@ public SettexHoverHandler(SettexWorkspace workspace, ILogger<SettexHoverHandler>
             {
                 // Le chemin complet inclut les blocs imbriqués traversés, donc
                 // survoler "Port" dans `Server { Port = … }` cible bien "Server.Port".
-                var (pathSegments, envName, isObjectHeader) = assignmentInfo.Value;
+                var (pathSegments, envName, isObjectHeader, segmentIndex) = assignmentInfo.Value;
 
                 // Vérifier que le mot sous le curseur est bien le path (ou une partie du path)
                 var isOnPath = pathSegments.Any(segment => segment == word);
@@ -114,8 +114,14 @@ public SettexHoverHandler(SettexWorkspace workspace, ILogger<SettexHoverHandler>
                 {
                     var path = string.Join(".", pathSegments);
 
-                    // Détecter si le mot survolé est un segment d'objet (pas le dernier segment)
-                    var wordIndex = pathSegments.IndexOf(word);
+                    // Quel segment est sous le curseur — déduit de la colonne, pas d'un
+                    // IndexOf : sur `A { B { A = 1 } }` le chemin est A.B.A, et chercher
+                    // la première occurrence de "A" renvoyait toujours 0, si bien que
+                    // survoler l'affectation la plus interne affichait l'objet A.
+                    var wordIndex = segmentIndex >= 0 && segmentIndex < pathSegments.Count
+                        ? segmentIndex
+                        : pathSegments.IndexOf(word);
+
                     var isObjectSegment = isObjectHeader || (wordIndex >= 0 && wordIndex < pathSegments.Count - 1);
 
                     if (isObjectSegment)
@@ -548,7 +554,7 @@ private static string EscapeString(string str)
     /// <summary>
     /// Trouve une assignation à la position donnée et retourne l'assignation + l'environnement (null si dans base).
     /// </summary>
-    private static (List<string> Path, string? EnvName, bool IsObjectHeader)? FindAssignmentAtPosition(
+    private static (List<string> Path, string? EnvName, bool IsObjectHeader, int SegmentIndex)? FindAssignmentAtPosition(
         Core.Parser.Ast.FileNode ast,
         Position position,
         string? documentFilePath = null)
@@ -574,15 +580,15 @@ private static (List<string> Path, string? EnvName, bool IsObjectHeader)? FindAs
                 var found = FindAssignmentInStatements(settings.Block.Statements, new List<string>(), line, column);
                 if (found != null)
                 {
-                    return (found.Value.Path, null, found.Value.IsObjectHeader); // Base, pas d'environnement
+                    return (found.Value.Path, null, found.Value.IsObjectHeader, found.Value.SegmentIndex); // Base, pas d'environnement
                 }
             }
             else if (stmt is Core.Parser.Ast.EnvBlockNode env)
             {
                 var found = FindAssignmentInStatements(env.SettingsBlock.Block.Statements, new List<string>(), line, column);
                 if (found != null)
                 {
-                    return (found.Value.Path, env.EnvironmentName, found.Value.IsObjectHeader); // Dans un environnement
+                    return (found.Value.Path, env.EnvironmentName, found.Value.IsObjectHeader, found.Value.SegmentIndex); // Dans un environnement
                 }
             }
         }
@@ -597,7 +603,7 @@ private static (List<string> Path, string? EnvName, bool IsObjectHeader)? FindAs
     /// (<c>Server.Port</c> et non <c>Port</c>) — c'est ce chemin que l'overlay
     /// recherche dans la configuration évaluée.
     /// </summary>
-    private static (List<string> Path, bool IsObjectHeader)? FindAssignmentInStatements(
+    private static (List<string> Path, bool IsObjectHeader, int SegmentIndex)? FindAssignmentInStatements(
         IReadOnlyList<Core.Parser.Ast.IStatement> statements,
         List<string> prefix,
         int line,
@@ -611,7 +617,8 @@ private static (List<string> Path, bool IsObjectHeader)? FindAssignmentInStateme
                 {
                     var fullPath = new List<string>(prefix);
                     fullPath.AddRange(assignment.Path.Segments);
-                    return (fullPath, false);
+
+                    return (fullPath, false, HoveredSegmentIndex(assignment, prefix.Count, line, column));
                 }
             }
             else if (stmt is Core.Parser.Ast.NestedBlockNode nested)
@@ -630,14 +637,50 @@ private static (List<string> Path, bool IsObjectHeader)? FindAssignmentInStateme
                 // ways. Checked after the body so an inner match still wins.
                 if (IsPositionOnBlockName(nested, line, column))
                 {
-                    return (nestedPrefix, true);
+                    // A header names the block itself: the last segment of its path.
+                    return (nestedPrefix, true, nestedPrefix.Count - 1);
                 }
             }
         }
 
         return null;
     }
 
+    /// <summary>
+    /// Index, dans le chemin complet, du segment réellement sous le curseur.
+    /// L'affectation démarre sur son chemin, donc les segments occupent des colonnes
+    /// consécutives séparées par un point ; on les parcourt pour trouver celui qui
+    /// contient la colonne. Renvoie -1 si le curseur est au-delà du chemin (sur la
+    /// valeur, par exemple), auquel cas l'appelant retombe sur son ancien calcul.
+    /// </summary>
+    private static int HoveredSegmentIndex(
+        Core.Parser.Ast.AssignmentNode assignment,
+        int prefixLength,
+        int line,
+        int column)
+    {
+        if (line != assignment.Location.Line)
+        {
+            return -1;
+        }
+
+        var segmentStart = assignment.Location.Column;
+
+        for (var i = 0; i < assignment.Path.Segments.Count; i++)
+        {
+            var segmentEnd = segmentStart + assignment.Path.Segments[i].Length;
+
+            if (column >= segmentStart && column < segmentEnd)
+            {
+                return prefixLength + i;
+            }
+
+            segmentStart = segmentEnd + 1; // le point
+        }
+
+        return -1;
+    }
+
     /// <summary>
     /// Vérifie si une position est sur le <strong>nom</strong> d'un bloc imbriqué,
     /// c'est-à-dire sur son en-tête et non dans son corps. Le nœud démarre sur son

src/Settex.LanguageServer/SettexReferencesHandler.cs

@@ -354,9 +354,25 @@ private static void FindReferencesInExpression(
             case Core.Parser.Ast.ArrayNode array:
                 foreach (var element in array.Elements)
                 {
-                    if (element is Core.Parser.Ast.IExpression expr)
+                    switch (element)
                     {
-                        FindReferencesInExpression(expr, name, references);
+                        // A ForNode is an IArrayElement but not an IExpression, so
+                        // filtering on IExpression dropped it silently — neither its
+                        // body nor the collection it walks was ever visited, and a
+                        // variable used only inside a loop reported zero references.
+                        case Core.Parser.Ast.ForNode forNode:
+                            FindReferencesInExpression(forNode.Collection, name, references);
+
+                            foreach (var stmt in forNode.Body.Statements)
+                            {
+                                FindReferencesInBlockStatement(stmt, name, references);
+                            }
+
+                            break;
+
+                        case Core.Parser.Ast.IExpression expr:
+                            FindReferencesInExpression(expr, name, references);
+                            break;
                     }
                 }
                 break;

src/Settex.LanguageServer/SettexTextDocumentSyncHandler.cs

@@ -82,18 +82,22 @@ public override Task<Unit> Handle(DidCloseTextDocumentParams request, Cancellati
 
         this.logger.LogTrace("Closed: {Uri}", uri);
 
-        // Documents that included this one fall back to the on-disk copy.
-        foreach (var affected in this.workspace.DidClose(uri))
-        {
-            this.PublishDiagnostics(affected.Uri, affected);
-        }
+        var affectedDocuments = this.workspace.DidClose(uri);
 
-        // Efface les diagnostics
+        // Clear this document's diagnostics first. Republishing the dependents came
+        // before, inside the same guard, so a failure there was swallowed and left the
+        // closed file underlined in the Problems panel with no way to clear it.
         this.languageServer.TextDocument.PublishDiagnostics(new PublishDiagnosticsParams
         {
             Uri = request.TextDocument.Uri,
             Diagnostics = new Container<Diagnostic>()
         });
+
+        // Documents that included this one fall back to the on-disk copy.
+        foreach (var affected in affectedDocuments)
+        {
+            this.PublishDiagnostics(affected.Uri, affected);
+        }
     });
 
     public override Task<Unit> Handle(DidSaveTextDocumentParams request, CancellationToken cancellationToken)

tests/Settex.LanguageServer.Tests/LoopReferenceAndSegmentTests.cs

@@ -0,0 +1,120 @@
+using Microsoft.Extensions.Logging.Abstractions;
+using OmniSharp.Extensions.LanguageServer.Protocol.Models;
+
+namespace Settex.LanguageServer.Tests;
+
+/// <summary>
+/// Two lookups that answered from the wrong place: references never entered a
+/// <c>for</c> loop, and hover picked a path segment by name rather than by position.
+/// </summary>
+public sealed class LoopReferenceAndSegmentTests
+{
+    [Test]
+    public async Task References_ToAVariableUsedOnlyInsideALoop_AreFoundAsync()
+    {
+        // ForNode is an IArrayElement but not an IExpression, and the collector filtered
+        // on IExpression — so neither the loop body nor the collection it walks was ever
+        // visited, and this returned nothing at all.
+        const string source = """
+            let svcs = [1, 2]
+            settings {
+                Items = [ for s in svcs { item { Value = s } } ]
+            }
+            """;
+
+        var (handler, uri) = CreateReferences(source);
+
+        // Cursor on the declaration of `svcs` (0-based line 0, column 4).
+        var locations = await handler.Handle(Request(uri, 0, 4), CancellationToken.None);
+
+        await Assert.That(locations).IsNotNull();
+
+        // The declaration plus its use inside the loop header.
+        await Assert.That(locations!.Count()).IsGreaterThanOrEqualTo(2);
+    }
+
+    [Test]
+    public async Task References_ToAVariableUsedInALoopBody_AreFoundAsync()
+    {
+        const string source = """
+            let prefix = "p"
+            settings {
+                Items = [ for s in [1] { item { Name = prefix } } ]
+                Other = prefix
+            }
+            """;
+
+        var (handler, uri) = CreateReferences(source);
+
+        var locations = await handler.Handle(Request(uri, 0, 4), CancellationToken.None);
+
+        await Assert.That(locations).IsNotNull();
+
+        // Declaration, the use in the loop body, and the one outside it.
+        await Assert.That(locations!.Count()).IsGreaterThanOrEqualTo(3);
+    }
+
+    [Test]
+    public async Task Hover_OnARepeatedPathSegment_UsesTheOneUnderTheCursorAsync()
+    {
+        // The path here is A.B.A. Picking the segment by IndexOf always returned 0, so
+        // hovering the innermost assignment showed the overlay for the outer object A
+        // instead of the value of A.B.A.
+        const string source = "settings {\n    A {\n        B {\n            A = 1\n        }\n    }\n}";
+        var handler = CreateHover(source, out var uri);
+
+        // Cursor on the inner "A" (0-based line 3, column 12).
+        var hover = await handler.Handle(HoverRequest(uri, 3, 12), CancellationToken.None);
+
+        await Assert.That(hover).IsNotNull();
+
+        var content = hover!.Contents.MarkupContent!.Value;
+
+        await Assert.That(content).Contains("A.B.A");
+    }
+
+    [Test]
+    public async Task Hover_OnAnObjectSegmentOfADottedPath_StillShowsTheObjectAsync()
+    {
+        // The guard: deriving the index from the column must not break the ordinary
+        // case the old IndexOf handled correctly.
+        const string source = "settings {\n    Server.Port = 8080\n}";
+        var handler = CreateHover(source, out var uri);
+
+        // Cursor on "Server" (0-based line 1, column 4).
+        var hover = await handler.Handle(HoverRequest(uri, 1, 4), CancellationToken.None);
+
+        await Assert.That(hover).IsNotNull();
+    }
+
+    private static (SettexReferencesHandler Handler, string Uri) CreateReferences(string source)
+    {
+        var workspace = new SettexWorkspace();
+        var uri = "untitled:loop-refs";
+        workspace.DidOpen(uri, source);
+
+        return (new SettexReferencesHandler(workspace, NullLogger<SettexReferencesHandler>.Instance), uri);
+    }
+
+    private static SettexHoverHandler CreateHover(string source, out string uri)
+    {
+        var workspace = new SettexWorkspace();
+        uri = "untitled:segment-hover";
+        workspace.DidOpen(uri, source);
+
+        return new SettexHoverHandler(workspace, NullLogger<SettexHoverHandler>.Instance);
+    }
+
+    private static ReferenceParams Request(string uri, int line, int character) => new()
+    {
+        TextDocument = new TextDocumentIdentifier { Uri = uri },
+        Position = new Position(line, character),
+        Context = new ReferenceContext { IncludeDeclaration = true },
+    };
+
+    private static HoverParams HoverRequest(string uri, int line, int character) => new()
+    {
+        TextDocument = new TextDocumentIdentifier { Uri = uri },
+        Position = new Position(line, character),
+    };
+}