Skip to content

Commit 784b404

Browse files
committed
fix(lsp): suppress hover on comments and function body whitespace
- Collect comment spans via a lightweight second lex pass in CompilerApi.Analyze and expose them on SemanticResult.CommentSpans. - HoverService.GetHoverResult returns null when the cursor falls inside any comment span (both leading "# ..." lines and trailing "x # ..."). - Function/class/struct/interface/enum hover cases now only resolve the declaration symbol when the cursor is on the header identifier; hovers on indented blank lines in the body no longer bubble up to the FunctionDef/ClassDef returned by FindInnermostNode. - Adds HoverServiceTests covering single-line comments, trailing comments, comment blocks, hover on def/class name, and blank-body-line cases. Addresses #540 sub-fix A.
1 parent 879613c commit 784b404

4 files changed

Lines changed: 210 additions & 1 deletion

File tree

src/Sharpy.Compiler/CompilerApi.cs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,16 +190,65 @@ public SemanticResult Analyze(string source, CancellationToken cancellationToken
190190
var compiler = new Compiler(opts, _logger);
191191
var result = compiler.Analyze(source, "<source>", cancellationToken);
192192

193+
// Collect comment spans via a second lightweight lex pass with preserveTrivia.
194+
// TODO(#540): This doubles lexer work per Analyze call. Consider threading
195+
// preserveTrivia through the pipeline so the primary lex can emit trivia directly.
196+
var commentSpans = CollectCommentSpans(source, cancellationToken);
197+
193198
return new SemanticResult
194199
{
195200
Success = !result.Diagnostics.HasErrors,
196201
Diagnostics = result.Diagnostics.GetAll(),
197202
Ast = result.Module,
198203
SemanticInfo = result.SemanticInfo,
199-
SymbolTable = result.SymbolTable
204+
SymbolTable = result.SymbolTable,
205+
CommentSpans = commentSpans
200206
};
201207
}
202208

209+
private IReadOnlyList<CommentSpan> CollectCommentSpans(string source, CancellationToken cancellationToken)
210+
{
211+
// Analyze is expected to return a (failed) result rather than throw when the
212+
// caller cancels, so swallow cancellation here like the primary pipeline does.
213+
if (cancellationToken.IsCancellationRequested)
214+
return Array.Empty<CommentSpan>();
215+
try
216+
{
217+
var lexer = new Lexer.Lexer(source, _logger, cancellationToken: cancellationToken, preserveTrivia: true);
218+
var tokens = lexer.TokenizeAll();
219+
var spans = new List<CommentSpan>();
220+
var seen = new HashSet<(int, int)>();
221+
foreach (var tok in tokens)
222+
{
223+
AppendTriviaSpans(tok.LeadingTrivia, spans, seen);
224+
AppendTriviaSpans(tok.TrailingTrivia, spans, seen);
225+
}
226+
return spans;
227+
}
228+
catch
229+
{
230+
return Array.Empty<CommentSpan>();
231+
}
232+
}
233+
234+
private static void AppendTriviaSpans(
235+
IReadOnlyList<Lexer.Trivia>? trivia,
236+
List<CommentSpan> spans,
237+
HashSet<(int, int)> seen)
238+
{
239+
if (trivia == null)
240+
return;
241+
foreach (var t in trivia)
242+
{
243+
if (t.Kind != Lexer.TriviaKind.Comment)
244+
continue;
245+
var key = (t.Line, t.Column);
246+
if (!seen.Add(key))
247+
continue;
248+
spans.Add(new CommentSpan(t.Line, t.Column, t.Column + t.Text.Length));
249+
}
250+
}
251+
203252
/// <summary>
204253
/// Analyzes a multi-file Sharpy project (Lexer → Parser → Semantic). No codegen.
205254
/// Runs phases 1-5 of the project compilation pipeline.

src/Sharpy.Compiler/CompilerApiTypes.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,18 @@ public sealed record SemanticResult
7777

7878
/// <summary>The symbol table from name resolution, or null if name resolution failed.</summary>
7979
public SymbolTable? SymbolTable { get; init; }
80+
81+
/// <summary>
82+
/// Source spans of comments in the analyzed file (1-based line and column).
83+
/// Used by LSP hover to suppress hover inside comments. Empty when no comments
84+
/// were collected (e.g., when analysis failed before lexing completed).
85+
/// </summary>
86+
public IReadOnlyList<CommentSpan> CommentSpans { get; init; } = Array.Empty<CommentSpan>();
8087
}
88+
89+
/// <summary>
90+
/// A single-line comment span in 1-based line/column coordinates.
91+
/// <c>StartColumn</c> points at the <c>#</c> character; <c>EndColumn</c> is exclusive
92+
/// (one past the last character of the comment).
93+
/// </summary>
94+
public readonly record struct CommentSpan(int Line, int StartColumn, int EndColumn);

src/Sharpy.Lsp.Tests/HoverServiceTests.cs

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,110 @@ public void GetHoverMarkdown_OverFunctionScopedTypeAlias_ReturnsAliasInfo()
282282
hover.Should().Contain("LocalAlias");
283283
hover.Should().Contain("str");
284284
}
285+
286+
// --- #540: hover should be suppressed on comments and function body whitespace ---
287+
288+
[Fact]
289+
public void GetHoverMarkdown_InsideSingleLineComment_ReturnsNull()
290+
{
291+
var source = "# hello world comment\nx: int = 42\n";
292+
var result = _api.Analyze(source);
293+
294+
// Cursor inside the "# hello world comment" on line 1
295+
var hover = _hoverService.GetHoverMarkdown(result, 1, 5);
296+
297+
hover.Should().BeNull();
298+
}
299+
300+
[Fact]
301+
public void GetHoverMarkdown_InsideTrailingComment_ReturnsNull()
302+
{
303+
var source = "x: int = 42 # inline comment\n";
304+
var result = _api.Analyze(source);
305+
306+
// "x: int = 42 # inline comment" — '#' is at col 14
307+
var hover = _hoverService.GetHoverMarkdown(result, 1, 18);
308+
309+
hover.Should().BeNull();
310+
}
311+
312+
[Fact]
313+
public void GetHoverMarkdown_OverVariableBeforeTrailingComment_StillResolves()
314+
{
315+
var source = "x: int = 42 # inline comment\n";
316+
var result = _api.Analyze(source);
317+
318+
// Cursor on 'x' at col 1 — not in the comment
319+
var hover = _hoverService.GetHoverMarkdown(result, 1, 1);
320+
321+
hover.Should().NotBeNull();
322+
hover.Should().Contain("int");
323+
}
324+
325+
[Fact]
326+
public void GetHoverMarkdown_InsideCommentBlockLines_ReturnsNull()
327+
{
328+
var source = "# line one\n# line two\n# line three\nx: int = 1\n";
329+
var result = _api.Analyze(source);
330+
331+
var hoverL1 = _hoverService.GetHoverMarkdown(result, 1, 3);
332+
var hoverL2 = _hoverService.GetHoverMarkdown(result, 2, 3);
333+
var hoverL3 = _hoverService.GetHoverMarkdown(result, 3, 3);
334+
335+
hoverL1.Should().BeNull();
336+
hoverL2.Should().BeNull();
337+
hoverL3.Should().BeNull();
338+
}
339+
340+
[Fact]
341+
public void GetHoverMarkdown_OverDefKeyword_ReturnsFunctionSymbol()
342+
{
343+
var source = "def greet(name: str) -> str:\n return name\n";
344+
var result = _api.Analyze(source);
345+
346+
// 'greet' starts at col 5; hover on 'g'
347+
var hover = _hoverService.GetHoverMarkdown(result, 1, 5);
348+
349+
hover.Should().NotBeNull();
350+
hover.Should().Contain("greet");
351+
}
352+
353+
[Fact]
354+
public void GetHoverMarkdown_OnFunctionBodyBlankLine_ReturnsNull()
355+
{
356+
// Indented blank line inside a function body should not resolve to the function.
357+
var source = "def greet() -> int:\n \n return 1\n";
358+
var result = _api.Analyze(source);
359+
360+
// Line 2 is the blank-indented line. Hovering on col 3 (inside indentation)
361+
// used to surface the FunctionDef because FindInnermostNode returned it.
362+
var hover = _hoverService.GetHoverMarkdown(result, 2, 3);
363+
364+
hover.Should().BeNull();
365+
}
366+
367+
[Fact]
368+
public void GetHoverMarkdown_OverClassName_ReturnsClassSymbol()
369+
{
370+
var source = "class Foo:\n x: int = 0\n";
371+
var result = _api.Analyze(source);
372+
373+
// 'Foo' starts at col 7
374+
var hover = _hoverService.GetHoverMarkdown(result, 1, 7);
375+
376+
hover.Should().NotBeNull();
377+
hover.Should().Contain("Foo");
378+
}
379+
380+
[Fact]
381+
public void GetHoverMarkdown_OnClassBodyWhitespace_ReturnsNull()
382+
{
383+
var source = "class Foo:\n \n x: int = 0\n";
384+
var result = _api.Analyze(source);
385+
386+
// Blank indented line 2, col 3 — should not resolve to Foo.
387+
var hover = _hoverService.GetHoverMarkdown(result, 2, 3);
388+
389+
hover.Should().BeNull();
390+
}
285391
}

src/Sharpy.Lsp/HoverService.cs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ public sealed record HoverResult(string Markdown, Node Node);
4646
if (analysis.Ast == null || analysis.SemanticQuery == null)
4747
return null;
4848

49+
// Suppress hover when the cursor is inside a comment.
50+
if (IsInsideComment(analysis.CommentSpans, line, col))
51+
return null;
52+
4953
var node = _api.FindNodeAtPosition(analysis.Ast, line, col);
5054
if (node == null)
5155
return null;
@@ -57,6 +61,18 @@ public sealed record HoverResult(string Markdown, Node Node);
5761
return new HoverResult(markdown, node);
5862
}
5963

64+
private static bool IsInsideComment(IReadOnlyList<CommentSpan> spans, int line, int col)
65+
{
66+
if (spans == null || spans.Count == 0)
67+
return false;
68+
foreach (var span in spans)
69+
{
70+
if (span.Line == line && col >= span.StartColumn && col < span.EndColumn)
71+
return true;
72+
}
73+
return false;
74+
}
75+
6076
internal string? GetHoverMarkdownForNode(Node node, SemanticResult analysis, int line, int col)
6177
{
6278
var query = analysis.SemanticQuery!;
@@ -178,6 +194,13 @@ public sealed record HoverResult(string Markdown, Node Node);
178194
? query.GetTypeAnnotation(param.Type) : null, className);
179195
}
180196

197+
// Only resolve to the function symbol when the cursor is on the
198+
// header identifier — not in the body whitespace or elsewhere.
199+
if (line != funcDef.NameLineStart ||
200+
col < funcDef.NameColumnStart ||
201+
col >= funcDef.NameColumnStart + funcDef.Name.Length)
202+
break;
203+
181204
// Hover on function name — try global scope first, then class scope
182205
var funcSymbol = analysis.SymbolTable?.LookupFunction(funcDef.Name);
183206
if (funcSymbol != null)
@@ -199,6 +222,9 @@ public sealed record HoverResult(string Markdown, Node Node);
199222
return baseHover;
200223
}
201224

225+
if (!IsOnHeaderName(line, col, classDef.NameLineStart, classDef.NameColumnStart, classDef.Name))
226+
break;
227+
202228
var typeSymbol = analysis.SymbolTable?.LookupType(classDef.Name);
203229
if (typeSymbol != null)
204230
return SymbolFormatter.FormatSymbolWithDocs(typeSymbol);
@@ -207,6 +233,9 @@ public sealed record HoverResult(string Markdown, Node Node);
207233

208234
case StructDef structDef:
209235
{
236+
if (!IsOnHeaderName(line, col, structDef.NameLineStart, structDef.NameColumnStart, structDef.Name))
237+
break;
238+
210239
var typeSymbol = analysis.SymbolTable?.LookupType(structDef.Name);
211240
if (typeSymbol != null)
212241
return SymbolFormatter.FormatSymbolWithDocs(typeSymbol);
@@ -215,6 +244,9 @@ public sealed record HoverResult(string Markdown, Node Node);
215244

216245
case InterfaceDef interfaceDef:
217246
{
247+
if (!IsOnHeaderName(line, col, interfaceDef.NameLineStart, interfaceDef.NameColumnStart, interfaceDef.Name))
248+
break;
249+
218250
var typeSymbol = analysis.SymbolTable?.LookupType(interfaceDef.Name);
219251
if (typeSymbol != null)
220252
return SymbolFormatter.FormatSymbolWithDocs(typeSymbol);
@@ -223,6 +255,9 @@ public sealed record HoverResult(string Markdown, Node Node);
223255

224256
case EnumDef enumDef:
225257
{
258+
if (!IsOnHeaderName(line, col, enumDef.NameLineStart, enumDef.NameColumnStart, enumDef.Name))
259+
break;
260+
226261
var typeSymbol = analysis.SymbolTable?.LookupType(enumDef.Name);
227262
if (typeSymbol != null)
228263
return SymbolFormatter.FormatSymbolWithDocs(typeSymbol);
@@ -552,6 +587,11 @@ private static string FormatFunctionTypeName(Compiler.Parser.Ast.FunctionType fu
552587
return $"({paramTypes}) -> {returnType}";
553588
}
554589

590+
private static bool IsOnHeaderName(int line, int col, int nameLine, int nameCol, string name)
591+
{
592+
return line == nameLine && col >= nameCol && col < nameCol + name.Length;
593+
}
594+
555595
internal static bool IsPositionInRange(int line, int col, int startLine, int startCol, int endLine, int endCol)
556596
{
557597
if (line < startLine || line > endLine)

0 commit comments

Comments
 (0)