Skip to content

Commit cd8b778

Browse files
antonsyndclaude
andcommitted
fix(lsp): show hover info for string literals and include range in all hovers
String literals were explicitly suppressed from hover (returning null). Now they fall through to the generic Expression handler like all other literals, showing their type (str). Also added HoverResult to return the resolved AST node alongside markdown, enabling the hover handler to set a Range on all responses so the LSP client highlights the correct token span. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 61ba240 commit cd8b778

3 files changed

Lines changed: 43 additions & 15 deletions

File tree

src/Sharpy.Lsp.Tests/HoverTests.cs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -423,9 +423,9 @@ public async Task Hover_VariableTypeAnnotation_ShowsTypeInfo()
423423
}
424424

425425
[Fact]
426-
public async Task Hover_StringLiteral_ReturnsNull()
426+
public async Task Hover_StringLiteral_ReturnsStringType()
427427
{
428-
// Hovering on a string literal should return null (self-evident type)
428+
// Hovering on a string literal should show its type (string)
429429
var source = "def main():\n x = \"hello\"";
430430
_workspace.OpenDocument("file:///test_string_hover.spy", source, 1);
431431

@@ -436,12 +436,17 @@ public async Task Hover_StringLiteral_ReturnsNull()
436436
var node = _api.FindNodeAtPosition(analysis!.Ast!, 2, 9);
437437
node.Should().NotBeNull();
438438
node.Should().BeOfType<StringLiteral>();
439+
440+
var service = new HoverService(_api);
441+
var hover = service.GetHoverMarkdown(analysis!, 2, 9);
442+
hover.Should().NotBeNull();
443+
hover.Should().Contain("str");
439444
}
440445

441446
[Fact]
442-
public async Task Hover_FStringLiteral_ReturnsNull()
447+
public async Task Hover_FStringLiteral_ReturnsStringType()
443448
{
444-
// Hovering on an f-string should return null (self-evident type)
449+
// Hovering on an f-string should show its type (string)
445450
var source = "def main():\n name = \"world\"\n x = f\"hello {name}\"";
446451
_workspace.OpenDocument("file:///test_fstring_hover.spy", source, 1);
447452

@@ -451,10 +456,12 @@ public async Task Hover_FStringLiteral_ReturnsNull()
451456
// Line 3: " x = f\"hello {name}\"" — f-string starts at col 9
452457
var node = _api.FindNodeAtPosition(analysis!.Ast!, 3, 9);
453458
node.Should().NotBeNull();
454-
// The node should be an FStringLiteral (or contained expression)
455459
if (node is FStringLiteral)
456460
{
457-
// Hover should suppress for f-strings
461+
var service = new HoverService(_api);
462+
var hover = service.GetHoverMarkdown(analysis!, 3, 9);
463+
hover.Should().NotBeNull();
464+
hover.Should().Contain("str");
458465
}
459466
}
460467

src/Sharpy.Lsp/Handlers/HoverHandler.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,23 @@ public SharpyHoverHandler(LanguageService languageService, HoverService hoverSer
2828
return null;
2929

3030
var (line, col) = PositionConverter.ToCompiler(request.Position);
31-
var hoverMarkdown = _hoverService.GetHoverMarkdown(analysis, line, col);
32-
if (hoverMarkdown == null)
31+
var result = _hoverService.GetHoverResult(analysis, line, col);
32+
if (result == null)
3333
return null;
3434

35+
var node = result.Node;
3536
return new Hover
3637
{
3738
Contents = new MarkedStringsOrMarkupContent(
3839
new MarkupContent
3940
{
4041
Kind = MarkupKind.Markdown,
41-
Value = hoverMarkdown
42+
Value = result.Markdown
4243
}
44+
),
45+
Range = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Range(
46+
PositionConverter.ToLsp(node.LineStart, node.ColumnStart),
47+
PositionConverter.ToLsp(node.LineEnd, node.ColumnEnd)
4348
)
4449
};
4550
}

src/Sharpy.Lsp/HoverService.cs

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,29 @@ public HoverService(CompilerApi api)
1919
_api = api;
2020
}
2121

22+
/// <summary>
23+
/// Result of a hover resolution, containing the markdown content and the AST node
24+
/// whose range should be highlighted.
25+
/// </summary>
26+
public sealed record HoverResult(string Markdown, Node Node);
27+
2228
/// <summary>
2329
/// Returns hover markdown for the given position in the analysis result,
2430
/// or null if no hover information is available.
2531
/// Line and column are 1-based (compiler convention).
2632
/// </summary>
2733
public string? GetHoverMarkdown(SemanticResult analysis, int line, int col)
34+
{
35+
return GetHoverResult(analysis, line, col)?.Markdown;
36+
}
37+
38+
/// <summary>
39+
/// Returns hover markdown and the resolved AST node for the given position,
40+
/// or null if no hover information is available.
41+
/// The node's LineStart/ColumnStart/LineEnd/ColumnEnd can be used to set the hover range.
42+
/// Line and column are 1-based (compiler convention).
43+
/// </summary>
44+
public HoverResult? GetHoverResult(SemanticResult analysis, int line, int col)
2845
{
2946
if (analysis.Ast == null || analysis.SemanticQuery == null)
3047
return null;
@@ -33,7 +50,11 @@ public HoverService(CompilerApi api)
3350
if (node == null)
3451
return null;
3552

36-
return GetHoverMarkdownForNode(node, analysis, line, col);
53+
var markdown = GetHoverMarkdownForNode(node, analysis, line, col);
54+
if (markdown == null)
55+
return null;
56+
57+
return new HoverResult(markdown, node);
3758
}
3859

3960
internal string? GetHoverMarkdownForNode(Node node, SemanticResult analysis, int line, int col)
@@ -388,11 +409,6 @@ public HoverService(CompilerApi api)
388409
return $"```sharpy\n(module) {fromImport.Module}\n```";
389410
}
390411

391-
// String literals: suppress hover (self-evident type)
392-
case StringLiteral:
393-
case FStringLiteral:
394-
return null;
395-
396412
case Expression expr:
397413
{
398414
var type = query.GetEffectiveType(expr);

0 commit comments

Comments
 (0)