mirror of
https://github.com/tiennm99/serena.git
synced 2026-10-05 02:13:55 +00:00
fix(ls): correct SymbolBody.get_text for the one-line-past-EOF range
An LSP server can end a symbol range one line past EOF: the convention for a range covering whole lines ends it at the start of the following, non-existent line. SymbolBody.get_text indexes self._lines[end_line] directly, so that well-defined case raised IndexError. get_text now corrects exactly that case (end_line == len(lines), end_col=0) to end at the actual last line, in get_text itself so it also applies to a cached SymbolBody, not just a freshly constructed one. Any other out-of-range end_line now raises InvalidTextLocationError. Fixes #1498
This commit is contained in:
1 parent
ad2fdf1fa6
commit
3a67ea2086
4 files changed
+129
-5
No files matched your search
@@ -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
|
||||
|
||||
+26
-5
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
Reference in new issue
Block a user