From 26939c447f268cb287afa67e07016ccadf775bc5 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Wed, 23 Sep 2026 13:40:37 +0200 Subject: [PATCH] Refactoring: Improve class naming and consolidate text coordinate representations * Rename TextCoordinates -> TextCoordinateProvider * Consolidate ls_util.LineCol, text_utils.TextCoords -> ls_util.TextCoordinates --- scripts/profile_search_text.py | 8 +-- src/serena/util/text_utils.py | 24 ++----- src/solidlsp/ls_utils.py | 21 +++--- test/solidlsp/test_text_coordinates.py | 90 +++++++++++++------------- 4 files changed, 63 insertions(+), 80 deletions(-) diff --git a/scripts/profile_search_text.py b/scripts/profile_search_text.py index 47bdce5f..00f9f254 100644 --- a/scripts/profile_search_text.py +++ b/scripts/profile_search_text.py @@ -22,7 +22,7 @@ import sys import time import solidlsp # noqa: F401 # imported first: solidlsp must resolve before serena.util.text_utils (known circular-import window) -from solidlsp.ls_utils import TextCoordinates, TextUtils +from solidlsp.ls_utils import TextCoordinateProvider, TextUtils def generate_synthetic_content(num_lines: int, match_identifier: str, match_frequency: float = 0.15) -> str: @@ -67,12 +67,12 @@ def resolve_before(content: str, compiled_pattern: re.Pattern) -> list[tuple[int def resolve_after(content: str, compiled_pattern: re.Pattern) -> list[tuple[int, int]]: """Resolves line coordinates the way this PR proposes (cached line starts + binary search).""" - coordinates = TextCoordinates(content) + coordinates = TextCoordinateProvider(content) results = [] for match in compiled_pattern.finditer(content): start_pos, end_pos = match.start(), match.end() - s = coordinates.line_col_at_index(start_pos).line - end_loc = coordinates.line_col_at_index(end_pos) + s = coordinates.compute_coordinates(start_pos).line + end_loc = coordinates.compute_coordinates(end_pos) e = end_loc.line if e > s and end_loc.col == 0: e -= 1 diff --git a/src/serena/util/text_utils.py b/src/serena/util/text_utils.py index 79000bf3..9c030cf7 100644 --- a/src/serena/util/text_utils.py +++ b/src/serena/util/text_utils.py @@ -13,7 +13,7 @@ from joblib import Parallel, delayed from sensai.util.string import ToStringMixin from serena.util.file_proxy import FileCollection, FileProxy -from solidlsp.ls_utils import TextCoordinates, TextUtils +from solidlsp.ls_utils import TextCoordinateProvider, TextCoordinates, TextUtils log = logging.getLogger(__name__) @@ -152,7 +152,7 @@ def search_text( # precompute line start offsets once so that each match's coordinates can be resolved via binary search # instead of re-scanning the text from the beginning for every match - coordinates = TextCoordinates(content) + coordinates = TextCoordinateProvider(content) # For multiline matches, optionally use DOTALL so '.' matches newlines flags = (re.MULTILINE | re.DOTALL) if multiline else 0 @@ -163,8 +163,8 @@ def search_text( end_pos = match.end() # Find the line numbers for the start and end positions - start_loc = coordinates.line_col_at_index(start_pos) - end_loc = coordinates.line_col_at_index(end_pos) + start_loc = coordinates.compute_coordinates(start_pos) + end_loc = coordinates.compute_coordinates(end_pos) start_line_num, end_line_num = start_loc.line, end_loc.line if end_line_num > start_line_num and end_loc.col == 0: # `end_pos` is exclusive, so if it is at the start of a line, the match ends with the @@ -643,19 +643,7 @@ class MultiFileContentReplacer: return "\n".join(diff_lines) -@dataclass -class TextCoords: - line: int - """ - 0-based line number - """ - col: int - """ - 0-based column number - """ - - -def find_text_coordinates(content: str, regex: str, require_unique: bool = False) -> TextCoords | None: +def find_text_coordinates(content: str, regex: str, require_unique: bool = False) -> TextCoordinates | None: """ Finds the line and column number of the first match of a regex pattern in the given content. @@ -680,4 +668,4 @@ def find_text_coordinates(content: str, regex: str, require_unique: bool = False raise ValueError(f"Regex must contain exactly one group to capture the position, but found {len(match.groups())} groups.") index_in_content = match.start(1) line, col = TextUtils.get_line_col_from_index(content, index_in_content) - return TextCoords(line, col) + return TextCoordinates(line, col) diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index 99520c28..ae292e6c 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -181,8 +181,8 @@ class TextStepper: return lines -@dataclass(frozen=True, kw_only=True) -class LineCol: +@dataclass +class TextCoordinates: """ Represents a position in a text as a pair of 0-based line and column numbers. """ @@ -194,14 +194,9 @@ class LineCol: """the 0-based column number""" -class TextCoordinates: - r""" - Provides line/column coordinates for character indices in a text, backed by a privately cached table of line start - offsets. - - The table is built once upon construction and mirrors the line semantics of :class:`TextStepper`: "\n", "\r\n" - and a bare "\r" are all treated as line separators (as defined by the Language Server Protocol). Resolving a - coordinate then requires only a binary search, independent of the text length. +class TextCoordinateProvider: + """ + Accelerates multiple computations of line/column coordinates in a given text by precomputing the text's line start indices. """ def __init__(self, text: str): @@ -211,7 +206,7 @@ class TextCoordinates: self._text = text self._line_starts = self._compute_line_starts() - def line_col_at_index(self, index: int) -> LineCol: + def compute_coordinates(self, index: int) -> TextCoordinates: r""" Returns the line/column coordinates corresponding to the given character index. @@ -230,9 +225,9 @@ class TextCoordinates: # an index pointing at the "\n" of a "\r\n" pair maps to the beginning of the following line if index > 0 and self._text[index - 1] == "\r" and self._text[index : index + 1] == "\n": - return LineCol(line=line_num + 1, col=0) + return TextCoordinates(line=line_num + 1, col=0) - return LineCol(line=line_num, col=index - line_start) + return TextCoordinates(line=line_num, col=index - line_start) def _compute_line_starts(self) -> list[int]: """ diff --git a/test/solidlsp/test_text_coordinates.py b/test/solidlsp/test_text_coordinates.py index 04733da5..8510bba3 100644 --- a/test/solidlsp/test_text_coordinates.py +++ b/test/solidlsp/test_text_coordinates.py @@ -1,6 +1,6 @@ import pytest -from solidlsp.ls_utils import InvalidTextLocationError, LineCol, TextCoordinates +from solidlsp.ls_utils import InvalidTextLocationError, TextCoordinateProvider, TextCoordinates class TestTextCoordinates: @@ -22,9 +22,9 @@ class TestTextCoordinates: ) def test_line_col_at_index_end_to_end(self, content): """Every index within the text resolves to a position that is consistent with the line layout.""" - coordinates = TextCoordinates(content) + coordinates = TextCoordinateProvider(content) for index in range(len(content) + 1): - loc = coordinates.line_col_at_index(index) + loc = coordinates.compute_coordinates(index) assert loc.line >= 0 assert loc.col >= 0 # a col greater than 0 implies that none of the col characters before the index is a line separator @@ -34,68 +34,68 @@ class TestTextCoordinates: def test_line_col_at_index_basic_lf(self): """Coordinates for LF-only content match the manually derived line layout.""" - coordinates = TextCoordinates("alpha\nbeta\ngamma\n") - assert coordinates.line_col_at_index(0) == LineCol(line=0, col=0) - assert coordinates.line_col_at_index(3) == LineCol(line=0, col=3) - assert coordinates.line_col_at_index(5) == LineCol(line=0, col=5) - assert coordinates.line_col_at_index(6) == LineCol(line=1, col=0) - assert coordinates.line_col_at_index(8) == LineCol(line=1, col=2) - assert coordinates.line_col_at_index(11) == LineCol(line=2, col=0) - assert coordinates.line_col_at_index(12) == LineCol(line=2, col=1) - assert coordinates.line_col_at_index(16) == LineCol(line=2, col=5) + coordinates = TextCoordinateProvider("alpha\nbeta\ngamma\n") + assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0) + assert coordinates.compute_coordinates(3) == TextCoordinates(line=0, col=3) + assert coordinates.compute_coordinates(5) == TextCoordinates(line=0, col=5) + assert coordinates.compute_coordinates(6) == TextCoordinates(line=1, col=0) + assert coordinates.compute_coordinates(8) == TextCoordinates(line=1, col=2) + assert coordinates.compute_coordinates(11) == TextCoordinates(line=2, col=0) + assert coordinates.compute_coordinates(12) == TextCoordinates(line=2, col=1) + assert coordinates.compute_coordinates(16) == TextCoordinates(line=2, col=5) # an index at the end of the text (after the trailing newline) denotes the start of a new line - assert coordinates.line_col_at_index(17) == LineCol(line=3, col=0) + assert coordinates.compute_coordinates(17) == TextCoordinates(line=3, col=0) def test_line_col_at_index_crlf(self): r"""CRLF sequences count as a single line ending. An index pointing at the "\n" of a pair denotes the beginning of the following line (column 0), whereas the "\r" itself still belongs to the preceding line. """ - coordinates = TextCoordinates("alpha\r\nbeta\r\n") - assert coordinates.line_col_at_index(0) == LineCol(line=0, col=0) - assert coordinates.line_col_at_index(5) == LineCol(line=0, col=5) - assert coordinates.line_col_at_index(6) == LineCol(line=1, col=0) - assert coordinates.line_col_at_index(7) == LineCol(line=1, col=0) - assert coordinates.line_col_at_index(8) == LineCol(line=1, col=1) - assert coordinates.line_col_at_index(11) == LineCol(line=1, col=4) - assert coordinates.line_col_at_index(12) == LineCol(line=2, col=0) - assert coordinates.line_col_at_index(13) == LineCol(line=2, col=0) + coordinates = TextCoordinateProvider("alpha\r\nbeta\r\n") + assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0) + assert coordinates.compute_coordinates(5) == TextCoordinates(line=0, col=5) + assert coordinates.compute_coordinates(6) == TextCoordinates(line=1, col=0) + assert coordinates.compute_coordinates(7) == TextCoordinates(line=1, col=0) + assert coordinates.compute_coordinates(8) == TextCoordinates(line=1, col=1) + assert coordinates.compute_coordinates(11) == TextCoordinates(line=1, col=4) + assert coordinates.compute_coordinates(12) == TextCoordinates(line=2, col=0) + assert coordinates.compute_coordinates(13) == TextCoordinates(line=2, col=0) def test_line_col_at_index_bare_cr(self): r"""Bare "\r" characters act as line separators.""" - coordinates = TextCoordinates("alpha\rbeta\r") - assert coordinates.line_col_at_index(5) == LineCol(line=0, col=5) - assert coordinates.line_col_at_index(6) == LineCol(line=1, col=0) - assert coordinates.line_col_at_index(10) == LineCol(line=1, col=4) - assert coordinates.line_col_at_index(11) == LineCol(line=2, col=0) + coordinates = TextCoordinateProvider("alpha\rbeta\r") + assert coordinates.compute_coordinates(5) == TextCoordinates(line=0, col=5) + assert coordinates.compute_coordinates(6) == TextCoordinates(line=1, col=0) + assert coordinates.compute_coordinates(10) == TextCoordinates(line=1, col=4) + assert coordinates.compute_coordinates(11) == TextCoordinates(line=2, col=0) def test_line_col_at_index_mixed(self): """Mixed line endings resolve consistently in a single text.""" - coordinates = TextCoordinates("a\r\nb\rc\nd\r\n") + coordinates = TextCoordinateProvider("a\r\nb\rc\nd\r\n") # "\r\n" ends line 0; "\r" ends line 1; "\n" ends line 2; "\r\n" ends line 3 - assert coordinates.line_col_at_index(0) == LineCol(line=0, col=0) - assert coordinates.line_col_at_index(1) == LineCol(line=0, col=1) - assert coordinates.line_col_at_index(2) == LineCol(line=1, col=0) - assert coordinates.line_col_at_index(3) == LineCol(line=1, col=0) - assert coordinates.line_col_at_index(4) == LineCol(line=1, col=1) - assert coordinates.line_col_at_index(5) == LineCol(line=2, col=0) - assert coordinates.line_col_at_index(6) == LineCol(line=2, col=1) - assert coordinates.line_col_at_index(7) == LineCol(line=3, col=0) - assert coordinates.line_col_at_index(8) == LineCol(line=3, col=1) - assert coordinates.line_col_at_index(9) == LineCol(line=4, col=0) - assert coordinates.line_col_at_index(10) == LineCol(line=4, col=0) + assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0) + assert coordinates.compute_coordinates(1) == TextCoordinates(line=0, col=1) + assert coordinates.compute_coordinates(2) == TextCoordinates(line=1, col=0) + assert coordinates.compute_coordinates(3) == TextCoordinates(line=1, col=0) + assert coordinates.compute_coordinates(4) == TextCoordinates(line=1, col=1) + assert coordinates.compute_coordinates(5) == TextCoordinates(line=2, col=0) + assert coordinates.compute_coordinates(6) == TextCoordinates(line=2, col=1) + assert coordinates.compute_coordinates(7) == TextCoordinates(line=3, col=0) + assert coordinates.compute_coordinates(8) == TextCoordinates(line=3, col=1) + assert coordinates.compute_coordinates(9) == TextCoordinates(line=4, col=0) + assert coordinates.compute_coordinates(10) == TextCoordinates(line=4, col=0) def test_line_col_at_index_empty_content(self): """The only valid index in empty content resolves to the origin.""" - coordinates = TextCoordinates("") - assert coordinates.line_col_at_index(0) == LineCol(line=0, col=0) + coordinates = TextCoordinateProvider("") + assert coordinates.compute_coordinates(0) == TextCoordinates(line=0, col=0) with pytest.raises(InvalidTextLocationError): - coordinates.line_col_at_index(1) + coordinates.compute_coordinates(1) def test_line_col_at_index_out_of_range(self): """Indices beyond the text length are rejected.""" - coordinates = TextCoordinates("abc") + coordinates = TextCoordinateProvider("abc") with pytest.raises(InvalidTextLocationError): - coordinates.line_col_at_index(4) + coordinates.compute_coordinates(4) with pytest.raises(InvalidTextLocationError): - coordinates.line_col_at_index(-1) + coordinates.compute_coordinates(-1)