Skip to content

Commit 5bd930f

Browse files
committed
fix(lsp): position inlay hints from the document being edited
Inlay hint positions came from the model held by the LSP context, whose query carries the token positions of the file as it was last loaded. The context is only reloaded on didSave, so while the user edits a column name the hint is placed at the offset the name used to end at, which renders the type cast inside the name being typed (`zip::TEXT_code`). It corrects itself on save, render or reopen because those reload the context. Parse the text the editor currently holds and take the hint positions from that, looking column types up by name on the loaded model. A column that isn't on the model yet gets no hint until the next reload, and text that doesn't parse yet gets none at all, rather than hints at offsets that no longer match the document. Fixes #5630 Signed-off-by: Adegbite Ayoade <tripleaceme@gmail.com>
1 parent a6ba67d commit 5bd930f

3 files changed

Lines changed: 100 additions & 3 deletions

File tree

‎sqlmesh/lsp/hints.py‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66

77
from sqlglot import exp
88
from sqlglot.optimizer.normalize_identifiers import normalize_identifiers
9-
from sqlmesh.core.model.definition import SqlModel
9+
from sqlmesh.core.dialect import parse
10+
from sqlmesh.core.model.definition import SqlModel, _split_sql_model_statements
1011
from sqlmesh.lsp.context import LSPContext, ModelTarget
1112
from sqlmesh.lsp.uri import URI
1213

@@ -16,6 +17,7 @@ def get_hints(
1617
document_uri: URI,
1718
start_line: int,
1819
end_line: int,
20+
document_text: t.Optional[str] = None,
1921
) -> t.List[types.InlayHint]:
2022
"""
2123
Get type hints for certain lines in a document
@@ -25,6 +27,8 @@ def get_hints(
2527
document_uri: The URI of the document
2628
start_line: the starting line to get hints for
2729
end_line: the ending line to get hints for
30+
document_text: the text currently held by the editor, if it may differ from the
31+
text the context was loaded from
2832
2933
Returns:
3034
A list of hints to apply to the document
@@ -49,15 +53,45 @@ def get_hints(
4953
if not isinstance(model, SqlModel):
5054
return []
5155

52-
query = model.query
5356
dialect = model.dialect
5457
columns_to_types = model.columns_to_types or {}
5558

59+
query: exp.Expr
60+
if document_text is None:
61+
query = model.query
62+
else:
63+
# The context is only reloaded when the document is saved, so the model's query
64+
# carries the positions of the text as it was last saved. Placing hints at those
65+
# positions puts them inside tokens the user is still editing, so take the
66+
# positions from the text the editor currently holds instead. Column types are
67+
# still looked up by name on the loaded model, and a column that isn't on it yet
68+
# simply gets no hint until the next reload.
69+
parsed_query = _query_from_document_text(document_text, dialect)
70+
if parsed_query is None:
71+
return []
72+
query = parsed_query
73+
5674
return _get_type_hints_for_model_from_query(
5775
query, dialect, columns_to_types, start_line, end_line
5876
)
5977

6078

79+
def _query_from_document_text(document_text: str, dialect: str) -> t.Optional[exp.Expr]:
80+
"""Extract the model's query from the raw text of a model file.
81+
82+
Returns None if the text cannot be parsed, which is expected while the user is
83+
part-way through an edit.
84+
"""
85+
try:
86+
expressions = parse(document_text, default_dialect=dialect)
87+
if not expressions:
88+
return None
89+
query, *_ = _split_sql_model_statements(expressions[1:], None, dialect=dialect)
90+
return query
91+
except Exception:
92+
return None
93+
94+
6195
def _get_type_hints_for_select(
6296
expression: exp.Expr,
6397
dialect: str,

‎sqlmesh/lsp/main.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,10 +705,11 @@ def inlay_hint(
705705
try:
706706
uri = URI(params.text_document.uri)
707707
context = self._context_get_or_load(uri)
708+
document = ls.workspace.get_text_document(params.text_document.uri)
708709

709710
start_line = params.range.start.line
710711
end_line = params.range.end.line
711-
hints = get_hints(context, uri, start_line, end_line)
712+
hints = get_hints(context, uri, start_line, end_line, document.source)
712713
return hints
713714

714715
except Exception as e:

‎tests/lsp/test_hints.py‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
"""Tests for type hinting SQLMesh models"""
22

3+
import typing as t
4+
35
import pytest
46

7+
from lsprotocol import types
8+
59
from sqlglot import exp, parse_one
610

711
from sqlmesh.core.context import Context
@@ -10,6 +14,16 @@
1014
from sqlmesh.lsp.uri import URI
1115

1216

17+
def _render(text: str, hints: t.List[types.InlayHint]) -> str:
18+
"""Insert the hints into the text the way an editor displays them."""
19+
lines = text.split("\n")
20+
for hint in sorted(hints, key=lambda h: (h.position.line, h.position.character), reverse=True):
21+
line = lines[hint.position.line]
22+
character = hint.position.character
23+
lines[hint.position.line] = f"{line[:character]}{hint.label}{line[character:]}"
24+
return "\n".join(lines)
25+
26+
1327
@pytest.mark.fast
1428
def test_hints() -> None:
1529
context = Context(paths=["examples/sushi"])
@@ -201,3 +215,51 @@ def test_cte_with_union_hints() -> None:
201215
assert result[0].label == "::INT"
202216
assert result[1].label == "::TEXT"
203217
assert result[2].label == "::DATE"
218+
219+
220+
@pytest.mark.fast
221+
def test_hints_are_positioned_from_the_edited_document() -> None:
222+
"""The context is only reloaded on save, so hints must be positioned against the text
223+
the editor currently holds. Positioning them from the loaded model puts the type cast
224+
inside a column name the user is part-way through typing."""
225+
context = Context(paths=["examples/sushi"])
226+
lsp_context = LSPContext(context)
227+
228+
path = next(
229+
path
230+
for path, info in lsp_context.map.items()
231+
if isinstance(info, ModelTarget) and "sushi.active_customers" in info.names
232+
)
233+
uri = URI.from_path(path)
234+
saved = path.read_text()
235+
236+
# While the document matches what was loaded, nothing changes.
237+
unchanged = get_hints(lsp_context, uri, start_line=0, end_line=9999, document_text=saved)
238+
assert "SELECT customer_id::INT, zip::TEXT" in _render(saved, unchanged)
239+
240+
# The user is renaming `zip` to `zip_code` and has not saved yet.
241+
edited = saved.replace("SELECT customer_id, zip\n", "SELECT customer_id, zip_code\n")
242+
assert edited != saved
243+
hints = get_hints(lsp_context, uri, start_line=0, end_line=9999, document_text=edited)
244+
245+
# `customer_id` is untouched so it keeps its hint; the column being renamed gets none
246+
# until the context is reloaded, rather than one rendered inside its name.
247+
assert "SELECT customer_id::INT, zip_code" in _render(edited, hints)
248+
assert [hint.label for hint in hints] == ["::INT"]
249+
250+
251+
@pytest.mark.fast
252+
def test_hints_for_unparseable_document() -> None:
253+
"""No hints are better than hints positioned from a stale parse."""
254+
context = Context(paths=["examples/sushi"])
255+
lsp_context = LSPContext(context)
256+
257+
path = next(
258+
path
259+
for path, info in lsp_context.map.items()
260+
if isinstance(info, ModelTarget) and "sushi.active_customers" in info.names
261+
)
262+
uri = URI.from_path(path)
263+
edited = path.read_text().replace("SELECT customer_id, zip\n", "SELECT customer_id, zip(\n")
264+
265+
assert get_hints(lsp_context, uri, start_line=0, end_line=9999, document_text=edited) == []

0 commit comments

Comments
 (0)