Skip to content

Commit a5fec6a

Browse files
committed
fix(lsp): narrow hover highlight to keyword tokens for compound expressions
For AwaitExpression, YieldStatement, and UnaryOp(Not), delegate hover markdown to the inner operand while narrowing the highlight range to just the keyword token span. This means hovering 'await' in 'await foo()' shows the hover for foo() but highlights only 'await' — matching user intuition about which token the tooltip is describing. HoverResult gains optional Highlight* init overrides; HoverHandler uses them when set, falling back to the full node span otherwise. TryNarrowToKeyword runs before GetHoverMarkdownForNode so that 'not p' still produces a tooltip even though UnaryOp hover is suppressed for builtin operands.
1 parent 784b404 commit a5fec6a

File tree

3 files changed

+117
-4
lines changed

3 files changed

+117
-4
lines changed

src/Sharpy.Lsp.Tests/HoverServiceTests.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,4 +388,53 @@ public void GetHoverMarkdown_OnClassBodyWhitespace_ReturnsNull()
388388

389389
hover.Should().BeNull();
390390
}
391+
392+
// --- #540 sub-fix B: keyword hover narrows highlight to the keyword span ---
393+
394+
[Fact]
395+
public void GetHoverResult_OverAwaitKeyword_NarrowsHighlightToKeyword()
396+
{
397+
var source = "async def foo() -> int:\n return 1\nasync def main():\n await foo()\n";
398+
var result = _api.Analyze(source);
399+
400+
// Line 4: " await foo()" — 'await' starts at col 5
401+
var hover = _hoverService.GetHoverResult(result, 4, 5);
402+
403+
hover.Should().NotBeNull();
404+
// Highlight narrowed to just "await" (5 chars)
405+
hover!.HighlightLineStart.Should().Be(4);
406+
hover.HighlightColumnStart.Should().Be(5);
407+
hover.HighlightLineEnd.Should().Be(4);
408+
hover.HighlightColumnEnd.Should().Be(10);
409+
}
410+
411+
[Fact]
412+
public void GetHoverResult_OverAwaitOperand_NoNarrowing()
413+
{
414+
var source = "async def foo() -> int:\n return 1\nasync def main():\n await foo()\n";
415+
var result = _api.Analyze(source);
416+
417+
// Line 4: " await foo()" — 'foo' starts at col 11
418+
var hover = _hoverService.GetHoverResult(result, 4, 11);
419+
420+
hover.Should().NotBeNull();
421+
// Cursor on operand — no narrowing; highlight overrides should be null
422+
hover!.HighlightLineStart.Should().BeNull();
423+
hover.HighlightColumnStart.Should().BeNull();
424+
}
425+
426+
[Fact]
427+
public void GetHoverResult_OverNotKeyword_NarrowsHighlightToKeyword()
428+
{
429+
var source = "def main():\n p: bool = True\n q = not p\n";
430+
var result = _api.Analyze(source);
431+
432+
// Line 3: " q = not p" — 'not' starts at col 9
433+
var hover = _hoverService.GetHoverResult(result, 3, 9);
434+
435+
hover.Should().NotBeNull();
436+
hover!.HighlightLineStart.Should().Be(3);
437+
hover.HighlightColumnStart.Should().Be(9);
438+
hover.HighlightColumnEnd.Should().Be(12);
439+
}
391440
}

src/Sharpy.Lsp/Handlers/HoverHandler.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ public SharpyHoverHandler(LanguageService languageService, HoverService hoverSer
3333
return null;
3434

3535
var node = result.Node;
36+
var startLine = result.HighlightLineStart ?? node.LineStart;
37+
var startCol = result.HighlightColumnStart ?? node.ColumnStart;
38+
var endLine = result.HighlightLineEnd ?? node.LineEnd;
39+
var endCol = result.HighlightColumnEnd ?? node.ColumnEnd;
3640
return new Hover
3741
{
3842
Contents = new MarkedStringsOrMarkupContent(
@@ -43,8 +47,8 @@ public SharpyHoverHandler(LanguageService languageService, HoverService hoverSer
4347
}
4448
),
4549
Range = new OmniSharp.Extensions.LanguageServer.Protocol.Models.Range(
46-
PositionConverter.ToLsp(node.LineStart, node.ColumnStart),
47-
PositionConverter.ToLsp(node.LineEnd, node.ColumnEnd)
50+
PositionConverter.ToLsp(startLine, startCol),
51+
PositionConverter.ToLsp(endLine, endCol)
4852
)
4953
};
5054
}

src/Sharpy.Lsp/HoverService.cs

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,17 @@ public HoverService(CompilerApi api)
2121

2222
/// <summary>
2323
/// Result of a hover resolution, containing the markdown content and the AST node
24-
/// whose range should be highlighted.
24+
/// whose range should be highlighted. Optional Highlight* overrides narrow the
25+
/// highlight to a subrange of the node (e.g. the <c>await</c> keyword inside an
26+
/// <see cref="AwaitExpression"/>).
2527
/// </summary>
26-
public sealed record HoverResult(string Markdown, Node Node);
28+
public sealed record HoverResult(string Markdown, Node Node)
29+
{
30+
public int? HighlightLineStart { get; init; }
31+
public int? HighlightColumnStart { get; init; }
32+
public int? HighlightLineEnd { get; init; }
33+
public int? HighlightColumnEnd { get; init; }
34+
}
2735

2836
/// <summary>
2937
/// Returns hover markdown for the given position in the analysis result,
@@ -54,13 +62,65 @@ public sealed record HoverResult(string Markdown, Node Node);
5462
if (node == null)
5563
return null;
5664

65+
// Keyword/operator nodes delegate to their operand's hover and narrow the
66+
// highlight to the keyword token. Attempt this first so that hovering `not p`
67+
// still produces hover text even though builtin UnaryOp hover is suppressed.
68+
var narrowed = TryNarrowToKeyword(node, analysis);
69+
if (narrowed != null)
70+
return narrowed;
71+
5772
var markdown = GetHoverMarkdownForNode(node, analysis, line, col);
5873
if (markdown == null)
5974
return null;
6075

6176
return new HoverResult(markdown, node);
6277
}
6378

79+
/// <summary>
80+
/// For compound expressions whose hover target is a leading keyword/operator token
81+
/// (e.g. <c>await foo()</c>, <c>yield x</c>, <c>not p</c>), delegate the hover text
82+
/// to the inner operand while narrowing the highlight range to just the keyword
83+
/// token span. Returns null when the node is not one of the supported kinds or when
84+
/// the inner delegation yields no markdown (in which case callers should keep the
85+
/// original result).
86+
/// </summary>
87+
private HoverResult? TryNarrowToKeyword(Node node, SemanticResult analysis)
88+
{
89+
Expression? operand;
90+
int keywordLength;
91+
switch (node)
92+
{
93+
case AwaitExpression awaitExpr:
94+
operand = awaitExpr.Operand;
95+
keywordLength = 5; // "await"
96+
break;
97+
case YieldStatement yieldStmt:
98+
if (yieldStmt.Value == null)
99+
return null;
100+
operand = yieldStmt.Value;
101+
keywordLength = yieldStmt.IsFrom ? 10 : 5; // "yield from" or "yield"
102+
break;
103+
case UnaryOp { Operator: UnaryOperator.Not } unaryOp:
104+
operand = unaryOp.Operand;
105+
keywordLength = 3; // "not"
106+
break;
107+
default:
108+
return null;
109+
}
110+
111+
var innerMarkdown = GetHoverMarkdownForNode(operand, analysis, operand.LineStart, operand.ColumnStart);
112+
if (innerMarkdown == null)
113+
return null;
114+
115+
return new HoverResult(innerMarkdown, node)
116+
{
117+
HighlightLineStart = node.LineStart,
118+
HighlightColumnStart = node.ColumnStart,
119+
HighlightLineEnd = node.LineStart,
120+
HighlightColumnEnd = node.ColumnStart + keywordLength,
121+
};
122+
}
123+
64124
private static bool IsInsideComment(IReadOnlyList<CommentSpan> spans, int line, int col)
65125
{
66126
if (spans == null || spans.Count == 0)

0 commit comments

Comments
 (0)