diff --git a/CHANGELOG.md b/CHANGELOG.md index be25874..5fec4c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ - Templates in a cycle of includes are no longer walked again on every path (#284): 20 levels of doubled includes closed into a cycle took 56 s, and 40 templates including one another more than a minute; both now take less than a second. +- The positions in a template are no longer counted from its start, for each filter, + include, extends and load (#288): a template of 2 MB applying 160,000 filters took + 50 s to analyse, and now takes 3 s. ## 0.24.0 (2026-10-23) diff --git a/src/coretrace_python/source/__init__.py b/src/coretrace_python/source/__init__.py index aa2756b..cb36428 100644 --- a/src/coretrace_python/source/__init__.py +++ b/src/coretrace_python/source/__init__.py @@ -1,6 +1,12 @@ """Source storage and location primitives.""" -from coretrace_python.source.manager import LINE_BREAK, SourceManager, decode_text, lines_of +from coretrace_python.source.manager import ( + LINE_BREAK, + LineIndex, + SourceManager, + decode_text, + lines_of, +) from coretrace_python.source.model import ( FileLocation, ImportName, @@ -14,6 +20,7 @@ "LINE_BREAK", "FileLocation", "ImportName", + "LineIndex", "Location", "SourceFile", "SourceId", diff --git a/src/coretrace_python/source/manager.py b/src/coretrace_python/source/manager.py index 248c2f9..ee48afe 100644 --- a/src/coretrace_python/source/manager.py +++ b/src/coretrace_python/source/manager.py @@ -3,6 +3,7 @@ from __future__ import annotations import re +from bisect import bisect_right from pathlib import Path from coretrace_python.source.model import ImportName, SourceFile, SourceId @@ -86,6 +87,26 @@ def lines_of(text: str) -> list[str]: return lines +class LineIndex: + """The line and column of each offset of a text, lines counted as ``lines_of`` + counts them. The start of each line is found once; the line of an offset is then + found by bisection, in time logarithmic in the lines.""" + + def __init__(self, text: str) -> None: + self._starts = [0, *(match.end() for match in LINE_BREAK.finditer(text))] + + def position(self, offset: int) -> tuple[int, int]: + """The one-based line and column of ``offset``.""" + + line = bisect_right(self._starts, offset) + return line, offset - self._starts[line - 1] + 1 + + def offset(self, line: int, column: int) -> int: + """The offset of the one-based ``line`` and ``column``.""" + + return self._starts[line - 1] + column - 1 + + class SourceManager: """Own source text and return one canonical object for each source ID.""" diff --git a/src/coretrace_python/source/positions.py b/src/coretrace_python/source/positions.py index f5f8dc9..ae4e173 100644 --- a/src/coretrace_python/source/positions.py +++ b/src/coretrace_python/source/positions.py @@ -16,9 +16,8 @@ import json import re import tomllib -from bisect import bisect_right -from coretrace_python.source.manager import LINE_BREAK, lines_of +from coretrace_python.source.manager import LineIndex, lines_of from coretrace_python.source.model import FileLocation, SourceId Pointer = tuple[str | int, ...] @@ -54,22 +53,6 @@ def pointer_text(path: Pointer) -> str: return "".join("/" + str(part).replace("~", "~0").replace("/", "~1") for part in path) -class _Lines: - def __init__(self, text: str) -> None: - self._starts = [0, *(match.end() for match in LINE_BREAK.finditer(text))] - - def position(self, offset: int) -> tuple[int, int]: - """The one-based line and column of ``offset``.""" - - line = bisect_right(self._starts, offset) - return line, offset - self._starts[line - 1] + 1 - - def offset(self, line: int, column: int) -> int: - """The offset of the one-based ``line`` and ``column``.""" - - return self._starts[line - 1] + column - 1 - - def _same(found: object, value: object) -> bool: return type(found) is type(value) and found == value @@ -87,7 +70,7 @@ def __init__(self, text: str) -> None: raise ValueError("trailing text") except (ValueError, IndexError, RecursionError): self._values = {} - self._lines = _Lines(text) + self._lines = LineIndex(text) def locate(self, path: Pointer, value: object) -> tuple[int, int] | None: """The line and column ``value`` starts at under ``path``, or None when the text @@ -183,7 +166,7 @@ def __init__(self, text: str) -> None: self._lines = lines_of(text) # The text with one line feed per line break, where arrays are read, once each. self._text = "\n".join(self._lines) - self._offsets = _Lines(self._text) + self._offsets = LineIndex(self._text) self._arrays: dict[int, list[tuple[int, int]]] = {} state = _LexState() for number, line in enumerate(self._lines, start=1): diff --git a/src/coretrace_python/taint/templates.py b/src/coretrace_python/taint/templates.py index 71b44c0..39f6dae 100644 --- a/src/coretrace_python/taint/templates.py +++ b/src/coretrace_python/taint/templates.py @@ -79,7 +79,7 @@ analyze_scopes, ) from coretrace_python.semantic.symbols import SymbolId, analyze_symbols -from coretrace_python.source import SourceId, SourceManager, SourceSpan +from coretrace_python.source import LineIndex, SourceId, SourceManager, SourceSpan from coretrace_python.taint.models import ModelTable TEMPLATES_DIRECTORY = "templates" @@ -935,13 +935,13 @@ class _Application: @dataclass(frozen=True) class _Reference: - """A template another one includes, or ``extends``, at ``offset`` in its ``blocks``: + """A template another one includes, or ``extends``, at ``span`` in its ``blocks``: by ``name``, None when an expression names it. The referenced template's free names are ``bindings``, then, unless ``only``, what the ``scope`` binds there, then the referencing template's.""" name: str | None - offset: int + span: SourceSpan bindings: Mapping[str, frozenset[str]] = field(default_factory=dict) only: bool = False scope: Mapping[str, frozenset[str]] = field(default_factory=dict) @@ -961,12 +961,12 @@ def maps(self, names: frozenset[str]) -> frozenset[str]: @dataclass(frozen=True) class _Load: - """A ``{% load %}`` of ``library`` at ``offset``, of the ``names`` it selects + """A ``{% load %}`` of ``library`` at ``span``, of the ``names`` it selects (``{% load a b from lib %}``), None for the whole library.""" library: str names: frozenset[str] | None - offset: int + span: SourceSpan @dataclass(frozen=True) @@ -1027,7 +1027,7 @@ def project_templates(root: Path) -> ProjectTemplates: continue read[path] = _read_scopes(text, SourceId(str(path)), libraries) for reference in read[path].references: - where = f"{relative}:{_position(text, reference.offset)[0]}" + where = f"{relative}:{reference.span.start_line}" if reference.name is None: unread.append(f"{where} names a template by an expression") elif reference.name not in files: @@ -1035,7 +1035,7 @@ def project_templates(root: Path) -> ProjectTemplates: for load in read[path].unresolved: library = libraries.get(load.library) reason = "not found" if library is None else "not read" if library.uncertain is None else "found in several files" - unread.append(f"{relative}:{_position(text, load.offset)[0]} loads {load.library!r}, {reason}") + unread.append(f"{relative}:{load.span.start_line} loads {load.library!r}, {reason}") calls = tuple(FilterCall(a.symbol, a.span) for template in read.values() for a in template.applications) # What each template reaches rendered on its own, for this analysis only. settled = _settled(files, read) @@ -1315,6 +1315,11 @@ def _read_scopes(text: str, source: SourceId, libraries: Mapping[str, _Library]) blocks: list[tuple[list[tuple[str, int, frozenset[str]]], set[str]]] = [] loaded: list[_Load] = [] unresolved: dict[_Load, None] = {} + lines = LineIndex(text) + + def span(offset: int) -> SourceSpan: + line, column = lines.position(offset) + return SourceSpan(source, line, column) def free(names: frozenset[str]) -> frozenset[str]: found: set[str] = set() @@ -1354,9 +1359,7 @@ def apply(head: frozenset[str], chain: list[tuple[str, int, frozenset[str]]]) -> if isinstance(symbol, _Load): unresolved[symbol] = None elif symbol is not None: - line, column = _position(text, offset) - span = SourceSpan(source, line, column) - applications.append(_Application(symbol, span, value, free(argument), tuple(opened))) + applications.append(_Application(symbol, span(offset), value, free(argument), tuple(opened))) value |= free(argument) return value @@ -1414,13 +1417,13 @@ def apply(head: frozenset[str], chain: list[tuple[str, int, frozenset[str]]]) -> elif name in ("include", "extends"): named = _constant(words[1]) if len(words) > 1 else None if name == "extends": - references.append(_Reference(named, offset, extends=True)) + references.append(_Reference(named, span(offset), extends=True)) else: context = {k: v for scope in scopes for k, v in scope.items()} - references.append(_Reference(named, offset, bindings, "only" in words, context, tuple(opened))) + references.append(_Reference(named, span(offset), bindings, "only" in words, context, tuple(opened))) elif name == "load": selected = frozenset(words[1 : words.index("from")]) if "from" in words else None - loaded.extend(_Load(library, selected, offset) for library in _loaded(words[1:])) + loaded.extend(_Load(library, selected, span(offset)) for library in _loaded(words[1:])) elif "as" in words[1:-1]: target = words[words.index("as", 1) + 1] scopes[-1][target] = values.get(words[1], frozenset()) if name == "regroup" else frozenset() @@ -1704,12 +1707,6 @@ def _expressions(text: str) -> Iterator[tuple[int, str, bool]]: yield match.start(2), tag, True -def _position(text: str, offset: int) -> tuple[int, int]: - """The one-based line and column of ``offset`` in ``text``.""" - - return text.count("\n", 0, offset) + 1, offset - text.rfind("\n", 0, offset) - - class EscapedTemplates(Analysis[frozenset[str]]): """The names of the project templates that escape everything they render, provided by the engine for a project; none on its own.""" diff --git a/tests/test_line_index.py b/tests/test_line_index.py new file mode 100644 index 0000000..980fb52 --- /dev/null +++ b/tests/test_line_index.py @@ -0,0 +1,23 @@ +"""The line index of a text: the line and column of each offset, lines counted as +``lines_of`` counts them, and back.""" + +from __future__ import annotations + +from coretrace_python.source import LineIndex + +# Lines broken by a line feed, a carriage return and line feed, and a lone carriage +# return; a column counts characters, not bytes. +TEXT = "a\nbé\r\nc\rd" +POSITIONS = [(1, 1), (1, 2), (2, 1), (2, 2), (2, 3), (2, 4), (3, 1), (3, 2), (4, 1), (4, 2)] + + +def test_each_offset_has_the_line_and_column_lines_of_counts() -> None: + index = LineIndex(TEXT) + + assert [index.position(offset) for offset in range(len(TEXT) + 1)] == POSITIONS + + +def test_each_position_has_its_offset_back() -> None: + index = LineIndex(TEXT) + + assert [index.offset(line, column) for line, column in POSITIONS] == list(range(len(TEXT) + 1)) diff --git a/tests/test_template_positions.py b/tests/test_template_positions.py new file mode 100644 index 0000000..07b8db5 --- /dev/null +++ b/tests/test_template_positions.py @@ -0,0 +1,68 @@ +"""Acceptance tests for issue #288: the positions of a template's filters, includes and +loads are found in time linear in the template. + +Each position was counted from the start of the file, so collecting the filters of a +template applying n of them took time quadratic in its size, where Django compiles and +renders it in linear time. The start of each line of a template is now found once, when +it is read, and each position by bisection. +""" + +from __future__ import annotations + +import json +import subprocess +import sys +from pathlib import Path + +import pytest + +TIMEOUT = 60 +COUNT = 200_000 +# The template's number of filters and the last one's line and column, then the number +# of unread entries and the last one. +DESCRIBE = ( + "import json, sys\n" + "from pathlib import Path\n" + "from coretrace_python.taint.templates import project_templates\n" + "found = project_templates(Path(sys.argv[1]))\n" + "filters = found.filters.get('page.html', ())\n" + "last = [filters[-1].location.start_line, filters[-1].location.start_column] if filters else None\n" + "print(json.dumps([len(filters), last, len(found.unread), found.unread[-1:]]))\n" +) + + +@pytest.mark.parametrize( + ("unit", "expected"), + [ + ("{{ v|upper }}", [COUNT, [1, 13 * (COUNT - 1) + 6], 0, []]), + ( + "{% include v %}\n", + [0, None, COUNT, [f"app/templates/page.html:{COUNT} names a template by an expression"]], + ), + ( + "{% load missing %}{{ v|upper }}\n", + [0, None, COUNT, [f"app/templates/page.html:{COUNT} loads 'missing', not found"]], + ), + ], + ids=["filters-on-one-line", "includes-named-by-an-expression", "loads-not-found"], +) +def test_the_positions_in_a_large_template_are_found_in_linear_time( + tmp_path: Path, unit: str, expected: list[object] +) -> None: + # Counted from the start of the file each time, these positions took minutes. + templates = tmp_path / "app" / "templates" + templates.mkdir(parents=True) + (templates / "page.html").write_text(unit * COUNT, encoding="utf-8") + try: + completed = subprocess.run( + [sys.executable, "-c", DESCRIBE, str(tmp_path)], + capture_output=True, + text=True, + check=False, + timeout=TIMEOUT, + ) + except subprocess.TimeoutExpired: + pytest.fail(f"finding the positions did not end within {TIMEOUT} s") + + assert completed.returncode == 0, completed.stderr + assert json.loads(completed.stdout) == expected