diff --git a/server/src/lsp_server.py b/server/src/lsp_server.py index 4745b92..8fe17e9 100644 --- a/server/src/lsp_server.py +++ b/server/src/lsp_server.py @@ -60,7 +60,6 @@ from tools.navigation import ( SymbolIdentity, call_hierarchy_identity, call_hierarchy_items, - definition_identities, document_highlights, incoming_call_hierarchy, matching_occurrences, @@ -77,7 +76,7 @@ from tools.semantic_tokens import ( from tools.signature_help import build_signature_help from tools.tcloo_arguments import method_signature_help from tclint.lexer import TclSyntaxError -from tools.tcloo_completion import parse_completion_source, tcloo_completions +from tools.tcloo_completion import may_contain_classes, parse_completion_source, tcloo_completions from tools.tcloo_symbols import class_completion_items from tools.tcloo_navigation import tcloo_definition from tools.tcl_command_completion import ( @@ -270,7 +269,14 @@ def on_completion(params: lsp.CompletionParams) -> lsp.CompletionList: doc = LSP_SERVER.workspace.get_text_document(params.text_document.uri) position = params.position source_lines = LSP_SERVER.get_lines(doc) - oo_items = tcloo_completions(source_lines, position, LSP_SERVER.class_snapshot(doc.path)) + + def current_tree(): + try: + return LSP_SERVER.get_tree(doc) + except TclSyntaxError: + return None + + oo_items = tcloo_completions(source_lines, position, LSP_SERVER.class_snapshot(doc.path), current_tree) if oo_items is not None: return lsp.CompletionList(is_incomplete=False, items=oo_items) array_items = array_element_completions( @@ -434,15 +440,15 @@ def on_completion(params: lsp.CompletionParams) -> lsp.CompletionList: ) def signature_help(params: lsp.SignatureHelpParams) -> lsp.SignatureHelp | None: document = LSP_SERVER.workspace.get_text_document(params.text_document.uri) - method_help = method_signature_help(document.source, params.position, LSP_SERVER.class_snapshot(document.path)) - if method_help is not None: - return method_help try: tree = LSP_SERVER.get_tree(document) except TclSyntaxError: tree = parse_completion_source(document.source) if tree is None: return None + method_help = method_signature_help(document.source, params.position, LSP_SERVER.class_snapshot(document.path), tree) + if method_help is not None: + return method_help custom_signatures, custom_docs = LSP_SERVER.proc_metadata_snapshot(document.path) @@ -522,8 +528,9 @@ def semantic_tokens(params: lsp.SemanticTokensParams): # Reuse cached AST tree = LSP_SERVER.get_tree(document) classes = LSP_SERVER.class_snapshot(document.path) - hl.highlight_classes(tree, classes) - hl.highlight_methods(tree, document.source, document.uri, classes) + if may_contain_classes(document.source, classes): + hl.highlight_classes(tree, classes) + hl.highlight_methods(tree, document.source, document.uri, classes) tree.accept(hl, recurse=True) tokens = hl.tokens() @@ -619,8 +626,12 @@ def goto_definition(params: lsp.DefinitionParams): workspace = None if workspace is not None: document = workspace.get_text_document(params.text_document.uri) + try: + tree = LSP_SERVER.get_tree(document) + except TclSyntaxError: + tree = None # tcloo_definition repairs open delimiters itself. target = tcloo_definition(document.source, document.uri, params.position, - LSP_SERVER.class_snapshot(document.path)) + LSP_SERVER.class_snapshot(document.path), tree) if target is not None: return [target] context = _navigation_context(params.text_document.uri, params.position) @@ -633,18 +644,17 @@ def goto_definition(params: lsp.DefinitionParams): def _navigation_context(uri: str, position: lsp.Position): - indexes = LSP_SERVER.navigation_snapshot() + indexes, definitions = LSP_SERVER.navigation_state() filepath = str(pathlib.Path(uris.to_fs_path(uri))) index = indexes.get(filepath) if index is None or LSP_SERVER.index_update_pending(filepath): document = LSP_SERVER.workspace.get_text_document(uri) LSP_SERVER.update_poco_completion_for_file(document) - indexes = LSP_SERVER.navigation_snapshot() + indexes, definitions = LSP_SERVER.navigation_state() index = indexes.get(filepath) if index is None: return None - definitions = definition_identities(indexes) result = symbol_at_position(index, position, definitions) if result is None: return None @@ -786,12 +796,8 @@ def incoming_calls(params: lsp.CallHierarchyIncomingCallsParams): if identity is None: return [] - indexes = LSP_SERVER.navigation_snapshot() - return incoming_call_hierarchy( - identity, - indexes, - definition_identities(indexes), - ) + indexes, definitions = LSP_SERVER.navigation_state() + return incoming_call_hierarchy(identity, indexes, definitions) @LSP_SERVER.feature(lsp.CALL_HIERARCHY_OUTGOING_CALLS) @@ -800,12 +806,8 @@ def outgoing_calls(params: lsp.CallHierarchyOutgoingCallsParams): if identity is None: return [] - indexes = LSP_SERVER.navigation_snapshot() - return outgoing_call_hierarchy( - identity, - indexes, - definition_identities(indexes), - ) + indexes, definitions = LSP_SERVER.navigation_state() + return outgoing_call_hierarchy(identity, indexes, definitions) # ********************************************************** diff --git a/server/src/lsp_tclserver.py b/server/src/lsp_tclserver.py index 3b41f55..637f814 100644 --- a/server/src/lsp_tclserver.py +++ b/server/src/lsp_tclserver.py @@ -20,7 +20,12 @@ from tools.tcloo_completion import indexed_classes from tools.file_sourcing import get_all_psc_files, psc_script_files from tools.formatter import NxFormatter as Formatter from tools.inlay_hint import InlayHintSignature, build_custom_inlay_signatures -from tools.navigation import FileSymbolIndex, build_file_symbol_index +from tools.navigation import ( + FileSymbolIndex, + SymbolIdentity, + build_file_symbol_index, + definition_identities, +) from tools.proc_docs import build_proc_docs from tools.variable_index import ProcRange, build_variable_index @@ -54,6 +59,7 @@ class TclLanguageServer(LanguageServer): self._ast_cache = {} self._line_cache: dict[tuple[str, int | None], tuple[str, ...]] = {} self._parser_lock = threading.RLock() + self._thread_parsers = threading.local() self._index_lock = threading.RLock() self._index_tokens: dict[str, int] = {} self._index_versions: dict[str, int | None] = {} @@ -67,6 +73,9 @@ class TclLanguageServer(LanguageServer): -1, frozenset(), ) + self._definition_identities_cache: tuple[ + int, frozenset[SymbolIdentity] + ] = (-1, frozenset()) self._proc_metadata_cache: dict[ str, tuple[int, dict[str, list[str]], dict[str, str]] ] = {} @@ -84,10 +93,17 @@ class TclLanguageServer(LanguageServer): return tree, list(self.parser.violations) def parse_source(self, source: str): - """Parse without retaining an AST, serialized around the shared parser.""" - with self._parser_lock: - tree, _ = self._parse_source(source) - return tree + """Parse without retaining an AST, on a parser owned by this thread. + + Background indexing must not hold the shared parser lock for whole + files while request handlers wait for their document's tree. + """ + local_parser = getattr(self._thread_parsers, "parser", None) + if local_parser is None: + # Plugin commands live in tclint's shared registry, see __init__. + local_parser = self._thread_parsers.parser = parser.CustomParser() + local_parser.violations = [] + return local_parser.parse(source) def get_tree(self, document: TextDocument): key = (document.uri, document.version) @@ -154,6 +170,7 @@ class TclLanguageServer(LanguageServer): self._index_generation += 1 self._workspace_completion_cache = (-1, ()) self._custom_function_names_cache = (-1, frozenset()) + self._definition_identities_cache = (-1, frozenset()) self._proc_metadata_cache.clear() self._custom_inlay_cache.clear() @@ -505,6 +522,22 @@ class TclLanguageServer(LanguageServer): with self._index_lock: return dict(self.navigation_indexes) + def navigation_state( + self, + ) -> tuple[dict[str, FileSymbolIndex], frozenset[SymbolIdentity]]: + """Return indexes plus their definitions, cached by index generation.""" + with self._index_lock: + generation, definitions = self._definition_identities_cache + if generation != self._index_generation: + definitions = frozenset( + definition_identities(self.navigation_indexes) + ) + self._definition_identities_cache = ( + self._index_generation, + definitions, + ) + return dict(self.navigation_indexes), definitions + def _begin_index_update(self, filepath: str, version: int | None) -> int | None: with self._index_lock: indexed_version = self._index_versions.get(filepath) diff --git a/server/src/tools/inlay_hint.py b/server/src/tools/inlay_hint.py index a9063c2..e57bbd8 100644 --- a/server/src/tools/inlay_hint.py +++ b/server/src/tools/inlay_hint.py @@ -200,7 +200,7 @@ class InlayHintGenerator(Visitor): walk(tree) if self.parameter_names != "none": - for call in resolved_method_calls(self.source, self.external_classes): + for call in resolved_method_calls(self.source, self.external_classes, tree): if not self._node_intersects_requested_range(call.command): continue parameters = method_parameters(call.parameters) diff --git a/server/src/tools/navigation.py b/server/src/tools/navigation.py index 4410743..c90707c 100644 --- a/server/src/tools/navigation.py +++ b/server/src/tools/navigation.py @@ -506,6 +506,15 @@ def resolve_identity( return occurrence.identity +def _may_resolve_to(occurrence: SymbolOccurrence, identity: SymbolIdentity) -> bool: + """Cheap name pre-filter; resolve_identity only returns one of these two.""" + name = identity.name + fallback = occurrence.fallback_identity + return occurrence.identity.name == name or ( + fallback is not None and fallback.name == name + ) + + def symbol_at_position( index: FileSymbolIndex, position: lsp.Position, @@ -530,6 +539,8 @@ def matching_occurrences( matches = [] for index in indexes.values(): for occurrence in index.occurrences: + if not _may_resolve_to(occurrence, identity): + continue if resolve_identity(occurrence, definitions) == identity: matches.append((index, occurrence)) return matches @@ -543,6 +554,8 @@ def document_highlights( """Return all occurrences of one symbol in the active document.""" highlights = [] for occurrence in index.occurrences: + if not _may_resolve_to(occurrence, identity): + continue if resolve_identity(occurrence, definitions) != identity: continue diff --git a/server/src/tools/tcloo_arguments.py b/server/src/tools/tcloo_arguments.py index 232d8ee..51bcbc2 100644 --- a/server/src/tools/tcloo_arguments.py +++ b/server/src/tools/tcloo_arguments.py @@ -38,14 +38,14 @@ def method_parameters(parameters: str) -> list[MethodParameter]: return [] -def method_signature_help(source: str, position: lsp.Position, external_classes=None) -> lsp.SignatureHelp | None: +def method_signature_help(source: str, position: lsp.Position, external_classes=None, tree=None) -> lsp.SignatureHelp | None: lines = source.split("\n") if position.line >= len(lines): return None # AST columns are codepoints; LSP columns are UTF-16 code units. prefix = lines[position.line].encode("utf-16-le")[:position.character * 2].decode("utf-16-le", errors="ignore") cursor = (position.line, len(prefix)) - candidates = [call for call in resolved_method_calls(source, external_classes) + candidates = [call for call in resolved_method_calls(source, external_classes, tree) if _contains_cursor(call.command, lines, cursor)] if not candidates: return None diff --git a/server/src/tools/tcloo_completion.py b/server/src/tools/tcloo_completion.py index 7caa1b8..fe420a3 100644 --- a/server/src/tools/tcloo_completion.py +++ b/server/src/tools/tcloo_completion.py @@ -1,6 +1,6 @@ """Static TclOO inference using local and indexed classes, without executing Tcl.""" -from collections.abc import Sequence +from collections.abc import Callable, Sequence from dataclasses import dataclass, field import re @@ -9,8 +9,30 @@ from tclint.lexer import TclSyntaxError from tclint.syntax_tree import BracedWord, Command, CommandSub, Script, VarSub from tools.parser import CustomParser +from tools.signature_help import _active_argument, _contains_cursor from tools.tcl_command_completion import line_prefix_at_position +_LEADING_RECEIVER = re.compile(r"\s*([A-Za-z_]\w*)\s+[\w:]*$") + + +def may_contain_classes(source, external_classes=None) -> bool: + """Cheap pre-check: without any class, TclOO analysis yields nothing.""" + return bool(external_classes) or "oo::class" in source + + +def _may_be_receiver(name, source_lines, external_classes) -> bool: + """Whether a bare command word can name a class, an object or `my`. + + Objects and local classes only come from `... create `, so a word + never created anywhere cannot resolve and needs no full-document parse. + """ + if name in {"my", "self", "next"}: + return True + if any(key.rsplit("::", 1)[-1] == name for key in external_classes or ()): + return True + created = re.compile(rf"\bcreate\s+[{{\"]?(?:[\w:]*::)?{re.escape(name)}\b") + return any("create" in line and created.search(line) for line in source_lines) + @dataclass class ClassInfo: @@ -74,10 +96,42 @@ def _qualified(name, namespace): return name if name.startswith("::") else f"{namespace}::{name}" +def _cursor_may_be_method_word(tree, source_lines, position) -> bool: + """Whether the innermost command at the cursor is at its first argument. + + The marker parse below can only succeed there, and the current document's + tree has the same structure apart from the marker. + """ + line = source_lines[position.line] + # AST columns are codepoints; LSP columns are UTF-16 code units. + prefix = line.encode("utf-16-le")[:position.character * 2].decode("utf-16-le", errors="ignore") + cursor = (position.line, len(prefix)) + lines = list(source_lines) + innermost = None + + def walk(node): + nonlocal innermost + start, end = getattr(node, "pos", None), getattr(node, "end_pos", None) + if start is not None and end is not None and not start[0] - 1 <= cursor[0] <= end[0] - 1: + return + if isinstance(node, Command) and _contains_cursor(node, lines, cursor): + innermost = node + for child in node.children: + walk(child) + + walk(tree) + return innermost is None or _active_argument(innermost, cursor) == 0 + + def tcloo_completions( source_lines: Sequence[str], position: lsp.Position, external_classes=None, + current_tree: Callable[[], Script | None] | None = None, ) -> list[lsp.CompletionItem] | None: - """Return receiver-specific methods, or None outside a known OO context.""" + """Return receiver-specific methods, or None outside a known OO context. + + `current_tree` lazily returns the parsed, unmodified document (or None) so + cursors that cannot hold a method name skip the full marker reparse. + """ prefix = line_prefix_at_position(source_lines, position) if prefix is None: return None @@ -87,6 +141,19 @@ def tcloo_completions( word_start = len(prefix) - len(typed) if word_start == 0 or prefix[word_start - 1] not in " \t": return None + if not external_classes and not any("oo::class" in line for line in source_lines): + return None + continued = position.line > 0 and source_lines[position.line - 1].endswith("\\") + if not continued: + # The first word is the command itself, never a method name. + if not prefix[:word_start].strip(): + return None + receiver = _LEADING_RECEIVER.match(prefix) + if receiver is not None and not _may_be_receiver(receiver.group(1), source_lines, external_classes): + return None + tree = current_tree() if current_tree is not None else None + if tree is not None and not _cursor_may_be_method_word(tree, source_lines, position): + return None marker = "__nx_tcloo_completion_cursor__" lines = list(source_lines) suffix = lines[position.line][len(prefix):] @@ -258,6 +325,9 @@ def _analyze(tree, typed="", marker="", external_classes=None, uri=None, source= return classes, result, calls -def resolved_method_calls(source, external_classes=None): - tree = parse_completion_source(source) +def resolved_method_calls(source, external_classes=None, tree=None): + if not may_contain_classes(source, external_classes): + return [] + if tree is None: + tree = parse_completion_source(source) return _analyze(tree, external_classes=external_classes)[2] if tree is not None else [] diff --git a/server/src/tools/tcloo_navigation.py b/server/src/tools/tcloo_navigation.py index 69b8bcb..86c48a4 100644 --- a/server/src/tools/tcloo_navigation.py +++ b/server/src/tools/tcloo_navigation.py @@ -1,11 +1,14 @@ """Definition targets for literal TclOO classes and resolved method calls.""" -from tools.tcloo_completion import _analyze, name_location, parse_completion_source +from tools.tcloo_completion import _analyze, may_contain_classes, name_location, parse_completion_source from tools.tcloo_symbols import class_symbols -def tcloo_definition(source, uri, position, external_classes=None): - tree = parse_completion_source(source) +def tcloo_definition(source, uri, position, external_classes=None, tree=None): + if not may_contain_classes(source, external_classes): + return None + if tree is None: + tree = parse_completion_source(source) if tree is None: return None classes, _, calls = _analyze(tree, external_classes=external_classes, uri=uri, source=source) diff --git a/server/tests/python_tests/test_index_stability.py b/server/tests/python_tests/test_index_stability.py index 384b653..3d88aa8 100644 --- a/server/tests/python_tests/test_index_stability.py +++ b/server/tests/python_tests/test_index_stability.py @@ -267,3 +267,35 @@ def test_variable_and_workspace_request_caches_are_reused(tmp_path: Path): assert first_completions is second_completions assert first_names is second_names assert "cached_proc" in first_names + + +def test_navigation_definitions_are_cached_until_the_index_changes(tmp_path: Path): + server = _server() + first = _document(tmp_path / "first.tcl", "proc first_proc {} {}") + assert server.update_poco_completion_for_file(first) + + indexes, definitions = server.navigation_state() + assert server.navigation_state()[1] is definitions + assert {identity.name for identity in definitions} >= {"::first_proc"} + + second = _document(tmp_path / "second.tcl", "proc second_proc {} {}") + assert server.update_poco_completion_for_file(second) + indexes, definitions = server.navigation_state() + assert second.path in indexes + assert {identity.name for identity in definitions} >= {"::first_proc", "::second_proc"} + + +def test_background_parse_does_not_wait_for_the_request_parser(tmp_path: Path): + server = _server() + document = _document(tmp_path / "background.tcl", "proc background_proc {} {}") + finished = Event() + + def index(): + assert server.update_poco_completion_for_file(document, cache_tree=False) + finished.set() + + with server._parser_lock: + with ThreadPoolExecutor(max_workers=1) as executor: + executor.submit(index) + assert finished.wait(timeout=5) + assert "background_proc" in server.custom_function_names_snapshot() diff --git a/server/tests/python_tests/test_tcloo_arguments.py b/server/tests/python_tests/test_tcloo_arguments.py index f9b269b..04fce87 100644 --- a/server/tests/python_tests/test_tcloo_arguments.py +++ b/server/tests/python_tests/test_tcloo_arguments.py @@ -98,3 +98,21 @@ def test_lsp_signature_help_with_unfinished_bracket(tmp_path, monkeypatch): )) assert result.active_parameter == 1 assert result.signatures[0].label == "::MCS initValue i value" + + +def test_cached_tree_gives_same_signature_without_reparsing(monkeypatch): + import tools.tcloo_completion as tcloo + + source = CLASS + "set mcs [MCS new test]\n$mcs initValue 0 " + position = lsp.Position(line=source.count("\n"), character=len(source.rsplit("\n", 1)[-1])) + expected = method_signature_help(source, position) + tree = CustomParser().parse(source) + parse_body = tcloo.parse_completion_source + + def parse(text, pos=None): + # Braced class bodies are still parsed on demand, the document is not. + assert text != source, "unexpected full-document parse" + return parse_body(text, pos) + + monkeypatch.setattr(tcloo, "parse_completion_source", parse) + assert method_signature_help(source, position, tree=tree) == expected diff --git a/server/tests/python_tests/test_tcloo_completion.py b/server/tests/python_tests/test_tcloo_completion.py index 61e5eb0..1cd0f28 100644 --- a/server/tests/python_tests/test_tcloo_completion.py +++ b/server/tests/python_tests/test_tcloo_completion.py @@ -1,7 +1,7 @@ import lsprotocol.types as lsp import pytest -from tools.tcloo_completion import tcloo_completions +from tools.tcloo_completion import ClassInfo, tcloo_completions CLASS = """oo::class create MCS { @@ -164,3 +164,68 @@ def test_namespaced_class_symbols_and_method_body_references(): assert set(declarations) == {"::geometry::MCS"} assert [node.contents for node in references] == ["MCS", "MCS", "geometry::MCS", "::geometry::MCS"] assert class_completion_items(tree)[0].label == "geometry::MCS" + + +@pytest.mark.parametrize("code", [ + "set mcs [MCS new]\nputs |", + "set mcs [MCS new]\n se|", + "set mcs [MCS new]\nMOM_output_literal |", +]) +def test_non_receivers_skip_the_completion_reparse(code, monkeypatch): + import tools.tcloo_completion as tcloo + + def fail(*_args, **_kwargs): + raise AssertionError("unexpected full-document parse") + + monkeypatch.setattr(tcloo, "parse_completion_source", fail) + assert complete(CLASS + code) is None + + +def test_documents_without_classes_skip_the_completion_reparse(monkeypatch): + import tools.tcloo_completion as tcloo + + monkeypatch.setattr(tcloo, "parse_completion_source", lambda *_: pytest.fail("parsed")) + assert complete("set value [expr 1]\n$value |") is None + + +def test_external_class_receiver_still_completes(): + source = "Logger |" + position = lsp.Position(line=0, character=len(source) - 1) + items = tcloo_completions([source.replace("|", "")], position, {"::Logger": ClassInfo()}) + assert {item.label for item in items} == {"new", "create"} + + +def complete_with_tree(source): + from tclint.lexer import TclSyntaxError + from tools.parser import CustomParser + + offset = source.index("|") + before = source[:offset] + position = lsp.Position(line=before.count("\n"), character=len(before.rsplit("\n", 1)[-1].encode("utf-16-le")) // 2) + text = source.replace("|", "") + try: + tree = CustomParser().parse(text) + except TclSyntaxError: + tree = None + return tcloo_completions(text.splitlines(), position, current_tree=lambda: tree) + + +@pytest.mark.parametrize("code", [ + "set mcs [MCS new]\n$mcs |", + "MCS create instance\ninstance |", + "set mcs [[MCS new] initOrg 1 2 3]\n$mcs |", + "proc run {} {set mcs [MCS new]; $mcs |}", + "set mcs [MCS new]\nputs [$mcs |]", + "set mcs [MCS new]\nputs [$mcs |", + "[MCS new] |", + "set mcs [MCS new]\n$mcs initV|alue", +]) +def test_cached_tree_keeps_method_completions(code): + assert "initValue" in {item.label for item in complete_with_tree(CLASS + code)} + + +def test_cached_tree_skips_reparse_outside_first_argument(monkeypatch): + import tools.tcloo_completion as tcloo + + monkeypatch.setattr(tcloo, "parse_completion_source", lambda *_: pytest.fail("parsed")) + assert complete_with_tree(CLASS + "set mcs [MCS new]\nif {$mcs ne {}} |{ puts 1 }") is None