diff --git a/CHANGELOG.md b/CHANGELOG.md index ef8e53e4..48e059c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ Status of the `main` branch. Changes prior to the next official version change w reached Serena's in-memory file contents, where the rest of the code assumes LF-normalized text and the `line_ending` setting is meant to be the single point of line-ending translation on write. The fallback now normalizes line endings to LF, consistently with the primary path. + - Fix: a symbol whose LSP range ended exactly one line past EOF, at column 0 (the convention for + a range covering whole lines through the end of the file), raised `IndexError` in + `SymbolBody.get_text`. That one well-defined case is now corrected to end at the actual last + line; any other out-of-range end position now raises `InvalidTextLocationError` instead, + rather than guessing at a body that could be wrong #1498 * Tools: - Fix: `search_for_pattern` marked one line too many as matched whenever a match ended with a line diff --git a/src/solidlsp/ls.py b/src/solidlsp/ls.py index 82ac0c30..8b05a32f 100644 --- a/src/solidlsp/ls.py +++ b/src/solidlsp/ls.py @@ -32,7 +32,7 @@ from solidlsp.dependency_provider import ( ) from solidlsp.initialize_params import DefaultInitializeParamsBuilder, InitializeParamsBuilder from solidlsp.ls_config import FilenameMatcher, Language, LanguageServerConfig -from solidlsp.ls_exceptions import SolidLSPException +from solidlsp.ls_exceptions import InvalidTextLocationError, SolidLSPException from solidlsp.ls_process import LanguageServerInterface, StdioLanguageServer from solidlsp.ls_types import UnifiedSymbolInformation from solidlsp.ls_utils import FileUtils, PathUtils, TextUtils @@ -209,17 +209,38 @@ class SymbolBody(ToStringMixin): return ["_lines"] def get_text(self) -> str: + end_line = self._end_line + end_col = self._end_col + if end_line >= len(self._lines): + if end_line == len(self._lines) and end_col == 0: + # LSP convention: a range covering whole lines through EOF sometimes ends + # at the start of the following, non-existent line (exactly one line past + # the last valid index, at column 0). That is well-defined: it means + # "through EOF", so treat it as ending at the end of the actual last line. + end_line = len(self._lines) - 1 + end_col = len(self._lines[end_line]) + else: + # Any other out-of-range end position (further past EOF, or exactly one + # line past EOF but not at column 0) is not the well-defined convention + # above; applying the same correction there would silently assume that + # a column meant for a nonexistent line still applies to the corrected + # one, which can produce a garbage body. Reject it instead of guessing. + raise InvalidTextLocationError( + f"Symbol range end (line {self._end_line}, col {self._end_col}) is out of bounds " + f"for a file with {len(self._lines)} lines" + ) + # extract relevant lines - symbol_body = "\n".join(self._lines[self._start_line : self._end_line + 1]) + symbol_body = "\n".join(self._lines[self._start_line : end_line + 1]) # remove leading content from the first line symbol_body = symbol_body[self._start_col :] # remove trailing content from the last line - last_line = self._lines[self._end_line] - trailing_length = len(last_line) - self._end_col + last_line = self._lines[end_line] + trailing_length = len(last_line) - end_col if trailing_length > 0: - symbol_body = symbol_body[: -(len(last_line) - self._end_col)] + symbol_body = symbol_body[: -(len(last_line) - end_col)] return symbol_body diff --git a/src/solidlsp/ls_exceptions.py b/src/solidlsp/ls_exceptions.py index 1a82f6b6..214b890b 100644 --- a/src/solidlsp/ls_exceptions.py +++ b/src/solidlsp/ls_exceptions.py @@ -52,6 +52,15 @@ class SolidLSPException(Exception): return s +class InvalidTextLocationError(SolidLSPException): + """ + Raised when a symbol's LSP range refers to a text location that does not exist + in the file's current line buffer, other than the well-defined whole-line-through-EOF + convention (end line exactly one past EOF at column 0), which is corrected rather + than rejected. + """ + + class MetalsStaleLockError(SolidLSPException): """ Raised when a stale Metals H2 database lock is detected and the user diff --git a/test/solidlsp/test_symbol_body.py b/test/solidlsp/test_symbol_body.py new file mode 100644 index 00000000..87660bf6 --- /dev/null +++ b/test/solidlsp/test_symbol_body.py @@ -0,0 +1,89 @@ +"""Unit tests for SymbolBody / SymbolBodyFactory that need no running language server.""" + +import pytest + +from solidlsp.ls import SymbolBodyFactory +from solidlsp.ls_exceptions import InvalidTextLocationError + + +class _StubBuffer: + """Minimal stand-in for LSPFileBuffer: the factory only reads split_lines().""" + + def __init__(self, lines: list[str]) -> None: + self._lines = lines + + def split_lines(self) -> list[str]: + return self._lines + + +def _symbol(start_line: int, start_col: int, end_line: int, end_col: int) -> dict: + return { + "location": { + "range": { + "start": {"line": start_line, "character": start_col}, + "end": {"line": end_line, "character": end_col}, + } + } + } + + +# 3 lines, valid indices 0..2 +LINES = ["class Foo:", " var x = 1", " var y = 2"] +FULL = "\n".join(LINES) + + +def _factory() -> SymbolBodyFactory: + return SymbolBodyFactory(_StubBuffer(list(LINES))) + + +def test_get_text_in_bounds_range() -> None: + """A range ending at the last real position returns the whole symbol (control).""" + body = _factory().create_symbol_body(_symbol(0, 0, 2, len(LINES[2]))) + assert body.get_text() == FULL + + +def test_get_text_end_line_past_eof_does_not_raise() -> None: + """A range whose end.line is past EOF used to raise IndexError in get_text. + + The LSP convention for a range covering whole lines ends it at the start of the + following line, which for the last line is one line past EOF. That end position + must be clamped to the end of the file, so the text runs through the last line. + """ + body = _factory().create_symbol_body(_symbol(0, 0, len(LINES), 0)) + assert body.get_text() == FULL + + +def test_get_text_end_col_past_line_end() -> None: + """An end.character past the end of a valid last line is clamped, no over-trim.""" + body = _factory().create_symbol_body(_symbol(0, 0, 2, 999)) + assert body.get_text() == FULL + + +def test_get_text_start_line_past_eof_returns_empty() -> None: + """A start.line past EOF is degenerate; it must not raise and yields no text.""" + body = _factory().create_symbol_body(_symbol(len(LINES), 0, len(LINES), 0)) + assert body.get_text() == "" + + +def test_get_text_end_line_far_past_eof_still_raises() -> None: + """end.line more than one line past EOF is a different, unconfirmed problem. + + Only the single-line-past-EOF case (the documented whole-line-range convention) is + well-defined enough to correct. Anything further out is rejected explicitly, rather + than guessing at a body that could be silently wrong. + """ + body = _factory().create_symbol_body(_symbol(0, 0, len(LINES) + 1, 0)) + with pytest.raises(InvalidTextLocationError): + body.get_text() + + +def test_get_text_end_line_past_eof_with_nonzero_col_raises() -> None: + """end.line one past EOF with a nonzero end.character is not the documented convention. + + The well-defined case is specifically column 0 (the start of the nonexistent + following line). A nonzero column there has no defined meaning for a line that does + not exist, so it must raise rather than being clamped as if it were the same case. + """ + body = _factory().create_symbol_body(_symbol(0, 0, len(LINES), 5)) + with pytest.raises(InvalidTextLocationError): + body.get_text()