feat(lsp): warn on undeclared block templates and addresses
Add diagnostics that warn when a literal NX command argument names a block template or address that no loaded .def file declares (diagnostic codes unknown-block-template and unknown-address). Names computed at runtime (e.g. $var or "CYCLE_$x") are not checked, and kinds whose .def file is not loaded are skipped. The server now recomputes and refreshes diagnostics when .def documents change (clears cached diagnostics and requests a workspace diagnostic refresh if the client supports it). Tests cover positive, negative and invalidated-cache cases.
This commit is contained in:
@@ -12,6 +12,7 @@ Versions correspond to the Git tags of this repository.
|
|||||||
- Find References and Rename for block templates and addresses across Tcl and `.def` files, including addresses used inside block templates
|
- Find References and Rename for block templates and addresses across Tcl and `.def` files, including addresses used inside block templates
|
||||||
- Go to Definition, hover, references, and rename also work inside `.def` files
|
- Go to Definition, hover, references, and rename also work inside `.def` files
|
||||||
- Hover and Go to Definition also recognize block template and address names that reach an NX command through a local variable (`set`, `lappend`, `list`, `foreach`) or through the parameter of a custom proc such as `LIB_SPF_call_cycle "absolute_mode"`, including nested wrapper procs; same-named strings without such a path are not recognized, and these derived names are not renamed
|
- Hover and Go to Definition also recognize block template and address names that reach an NX command through a local variable (`set`, `lappend`, `list`, `foreach`) or through the parameter of a custom proc such as `LIB_SPF_call_cycle "absolute_mode"`, including nested wrapper procs; same-named strings without such a path are not recognized, and these derived names are not renamed
|
||||||
|
- Warning for block template and address names in NX commands (`MOM_do_template stedy_rest`) that no loaded `.def` file declares; names built at runtime (`$var`, `"CYCLE_$x"`) are not checked, and the warnings update when a `.def` file changes
|
||||||
- Completion items for block templates and addresses (`BLOCK_LIST`, `ADDR_LIST`, `MOM_do_template`, ...) show the same preview as the hover
|
- Completion items for block templates and addresses (`BLOCK_LIST`, `ADDR_LIST`, `MOM_do_template`, ...) show the same preview as the hover
|
||||||
|
|
||||||
### Documentation
|
### Documentation
|
||||||
|
|||||||
@@ -18,7 +18,7 @@ from tools import checks, incremental_parse, parser
|
|||||||
from tools.completion_items import CompletionCollector
|
from tools.completion_items import CompletionCollector
|
||||||
from tools.tcloo_symbols import class_completion_items
|
from tools.tcloo_symbols import class_completion_items
|
||||||
from tools.tcloo_completion import indexed_classes
|
from tools.tcloo_completion import indexed_classes
|
||||||
from tools.def_flow import WrapperTable, build_wrapper_table
|
from tools.def_flow import WrapperTable, build_wrapper_table, unknown_def_names
|
||||||
from tools.def_symbols import ADDRESS, BLOCK_TEMPLATE, DefDocument, parse_def_document, read_def_source
|
from tools.def_symbols import ADDRESS, BLOCK_TEMPLATE, DefDocument, parse_def_document, read_def_source
|
||||||
from tools.file_sourcing import get_all_psc_files, psc_defined_event_files, psc_script_files
|
from tools.file_sourcing import get_all_psc_files, psc_defined_event_files, psc_script_files
|
||||||
from tools.formatter import NxFormatter as Formatter
|
from tools.formatter import NxFormatter as Formatter
|
||||||
@@ -276,7 +276,20 @@ class TclLanguageServer(LanguageServer):
|
|||||||
except OSError as error:
|
except OSError as error:
|
||||||
report(f"Could not read DEF file {def_file}: {error}")
|
report(f"Could not read DEF file {def_file}: {error}")
|
||||||
with self._index_lock:
|
with self._index_lock:
|
||||||
|
changed = documents != self.def_documents
|
||||||
self.def_documents = documents
|
self.def_documents = documents
|
||||||
|
if changed:
|
||||||
|
# Unknown template/address warnings depend on the .def files.
|
||||||
|
self.diagnostics.clear()
|
||||||
|
if changed:
|
||||||
|
self._request_diagnostic_refresh()
|
||||||
|
|
||||||
|
def _request_diagnostic_refresh(self) -> None:
|
||||||
|
# Set by the initialize request; absent before it and in tests.
|
||||||
|
capabilities = getattr(self.protocol, "client_capabilities", None)
|
||||||
|
diagnostics = getattr(getattr(capabilities, "workspace", None), "diagnostics", None)
|
||||||
|
if getattr(diagnostics, "refresh_support", False):
|
||||||
|
self.workspace_diagnostic_refresh(None)
|
||||||
|
|
||||||
def def_documents_snapshot(self, current_path=None, current_source: str | None = None) -> dict[str, DefDocument]:
|
def def_documents_snapshot(self, current_path=None, current_source: str | None = None) -> dict[str, DefDocument]:
|
||||||
"""Return the PSC .def documents; ``current_source`` replaces the file being edited."""
|
"""Return the PSC .def documents; ``current_source`` replaces the file being edited."""
|
||||||
@@ -898,8 +911,27 @@ class TclLanguageServer(LanguageServer):
|
|||||||
)
|
)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
diagnostics.extend(self._def_diagnostics(document))
|
||||||
return diagnostics
|
return diagnostics
|
||||||
|
|
||||||
|
def _def_diagnostics(self, document: TextDocument) -> List[lsp.Diagnostic]:
|
||||||
|
documents = self.def_documents_snapshot()
|
||||||
|
declared = {
|
||||||
|
kind: frozenset(name for def_document in documents.values() for name in def_document.names(kind))
|
||||||
|
for kind in (BLOCK_TEMPLATE, ADDRESS)
|
||||||
|
}
|
||||||
|
labels = {BLOCK_TEMPLATE: "Block template", ADDRESS: "Address"}
|
||||||
|
return [
|
||||||
|
lsp.Diagnostic(
|
||||||
|
message=f"{labels[kind]} '{name}' is not declared in any loaded .def file",
|
||||||
|
severity=lsp.DiagnosticSeverity.Warning,
|
||||||
|
range=range_,
|
||||||
|
code=f"unknown-{kind.replace('_', '-')}",
|
||||||
|
source=DIAGNOSTIC_SOURCE,
|
||||||
|
)
|
||||||
|
for kind, name, range_ in unknown_def_names(self.get_tree(document), declared)
|
||||||
|
]
|
||||||
|
|
||||||
def _compute_diagnostics(self, document: TextDocument) -> List[lsp.Diagnostic]:
|
def _compute_diagnostics(self, document: TextDocument) -> List[lsp.Diagnostic]:
|
||||||
return self.lint(document)
|
return self.lint(document)
|
||||||
|
|
||||||
|
|||||||
@@ -338,3 +338,27 @@ def derived_def_symbol(
|
|||||||
targets.extend(target for variable in variables for target in scope_targets.get(variable, ()))
|
targets.extend(target for variable in variables for target in scope_targets.get(variable, ()))
|
||||||
kinds = frozenset(kind for target in targets for kind in _target_kinds(target, table, namespace))
|
kinds = frozenset(kind for target in targets for kind in _target_kinds(target, table, namespace))
|
||||||
return (kinds, *literal) if kinds else None
|
return (kinds, *literal) if kinds else None
|
||||||
|
|
||||||
|
|
||||||
|
def _all_commands(node: Node) -> Iterator[Command]:
|
||||||
|
for child in getattr(node, "children", []):
|
||||||
|
if isinstance(child, Command):
|
||||||
|
yield child
|
||||||
|
yield from _all_commands(child)
|
||||||
|
|
||||||
|
|
||||||
|
def unknown_def_names(tree: Node, declared: dict[str, frozenset[str]]) -> list[tuple[str, str, lsp.Range]]:
|
||||||
|
"""Literal NX command arguments naming a block template or address no .def file declares.
|
||||||
|
|
||||||
|
``declared`` maps each kind to its declared names; kinds without any
|
||||||
|
declaration are not checked, since their .def file is not loaded.
|
||||||
|
"""
|
||||||
|
unknown = []
|
||||||
|
for command in _all_commands(tree):
|
||||||
|
for node, kind in def_argument_kinds(command):
|
||||||
|
names = declared.get(kind)
|
||||||
|
name = def_name(node) if names else None
|
||||||
|
if name is not None and name not in names:
|
||||||
|
line, column = node.contents_pos
|
||||||
|
unknown.append((kind, name, _range(line, column, name)))
|
||||||
|
return unknown
|
||||||
|
|||||||
@@ -180,3 +180,44 @@ def test_derived_names_are_not_renamed(tmp_path, monkeypatch):
|
|||||||
text_document=lsp.TextDocumentIdentifier(uri=caller.as_uri()), position=_position("absolute_mode")
|
text_document=lsp.TextDocumentIdentifier(uri=caller.as_uri()), position=_position("absolute_mode")
|
||||||
)
|
)
|
||||||
assert lsp_server.prepare_rename(params) is None
|
assert lsp_server.prepare_rename(params) is None
|
||||||
|
|
||||||
|
|
||||||
|
def _warnings(server, tmp_path: Path, source: str):
|
||||||
|
uri = (tmp_path / "check.tcl").as_uri()
|
||||||
|
server.workspace.put_text_document(lsp.TextDocumentItem(uri=uri, language_id="tcl", version=1, text=source))
|
||||||
|
diagnostics = server.lint(server.workspace.get_text_document(uri))
|
||||||
|
return [
|
||||||
|
(diagnostic.code, diagnostic.message, diagnostic.range.start.line, diagnostic.range.start.character, diagnostic.range.end.character)
|
||||||
|
for diagnostic in diagnostics
|
||||||
|
if diagnostic.code in {"unknown-block-template", "unknown-address"}
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_unknown_template_and_address_are_warned(tmp_path, monkeypatch):
|
||||||
|
server, _ = _project(tmp_path, monkeypatch)
|
||||||
|
source = 'proc p {} {\n MOM_do_template stedy_rest\n MOM_force Once SPOS "SPSO"\n}\n'
|
||||||
|
assert _warnings(server, tmp_path, source) == [
|
||||||
|
("unknown-block-template", "Block template 'stedy_rest' is not declared in any loaded .def file", 1, 20, 30),
|
||||||
|
("unknown-address", "Address 'SPSO' is not declared in any loaded .def file", 2, 25, 29),
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def test_known_and_dynamic_names_are_not_warned(tmp_path, monkeypatch):
|
||||||
|
server, _ = _project(tmp_path, monkeypatch)
|
||||||
|
source = 'MOM_do_template steady_rest CREATE\nMOM_do_template $name\nMOM_do_template "CYCLE_$x"\nMOM_force Once SPOS\n'
|
||||||
|
assert _warnings(server, tmp_path, source) == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_warnings_without_loaded_def_files(tmp_path, monkeypatch):
|
||||||
|
server, _ = _project(tmp_path, monkeypatch)
|
||||||
|
server.def_documents = {}
|
||||||
|
assert _warnings(server, tmp_path, "MOM_do_template stedy_rest\n") == []
|
||||||
|
|
||||||
|
|
||||||
|
def test_def_change_invalidates_cached_diagnostics(tmp_path, monkeypatch):
|
||||||
|
server, caller = _project(tmp_path, monkeypatch)
|
||||||
|
server.compute_diagnostics(server.workspace.get_text_document(caller.as_uri()))
|
||||||
|
assert server.diagnostic_snapshot(caller.as_uri()) is not None
|
||||||
|
(tmp_path / "service" / "service.def").write_text(DEF.replace("absolute_mode", "incremental_mode"), encoding="utf-8")
|
||||||
|
server.refresh_def_symbols([tmp_path])
|
||||||
|
assert server.diagnostic_snapshot(caller.as_uri()) is None
|
||||||
|
|||||||
Reference in New Issue
Block a user