diff --git a/server/src/tools/checks.py b/server/src/tools/checks.py index 70d6886..6971779 100644 --- a/server/src/tools/checks.py +++ b/server/src/tools/checks.py @@ -1,11 +1,12 @@ from enum import Enum -from tclint.syntax_tree import Visitor, BareWord, Command +from tclint.syntax_tree import Visitor, BareWord, Command, VarSub, QuotedWord, BracedWord from tclint.violations import Violation, Rule class Rules(Enum): VALIDATION = "validation" OPTIONAL_ARG_POSITION = "optinal_args" + UNDECLARED_VARIABLE = "undeclared-variable" def __str__(self): return self.value @@ -21,10 +22,7 @@ class CommandArgsCheck(Visitor): return self._violations def visit_command(self, command: Command): - if ( - not hasattr(command.routine, "contents") - or command.routine.contents != "proc" - ): + if not hasattr(command.routine, "contents") or command.routine.contents != "proc": return if len(command.args) < 2: return @@ -51,7 +49,105 @@ class CommandArgsCheck(Visitor): found_optional = True +class UndeclaredVariableCheck(Visitor): + """ + Warn when inside a proc a variable is used ($var) that is neither: + - declared global in the proc via `global var`, nor + - referenced as namespaced global (e.g. $::var), nor + - assigned locally in the proc via `set var ...`, nor + - a proc argument. + """ + + def __init__(self): + self._violations = [] + + def check(self, source, tree): + self._violations.clear() + # Traverse to find procs; inner scanning is handled per-proc + tree.accept(self, recurse=True) + return self._violations + + def visit_command(self, command: Command): + # Only interested in procs + if not hasattr(command.routine, "contents"): + return + if command.routine.contents != "proc": + return + # Expect: proc + if len(command.args) < 3: + return + + proc_name_node = command.args[0] + args_node = command.args[1] + body_node = command.args[2] + + # Collect proc arg names + arg_names = set() + if hasattr(args_node, "children"): + for child in args_node.children: + if isinstance(child, BareWord): + if child.contents: + arg_names.add(child.contents) + elif hasattr(child, "children") and child.children: + # Defaulted arg: first element should be the arg name + first = child.children[0] + if hasattr(first, "contents") and first.contents: + arg_names.add(first.contents) + + # Two-pass scan over body: first collect locals (set) and declared globals + locals_set = set(arg_names) + declared_globals = set() + + def _word_text(node) -> str | None: + if isinstance(node, BareWord) or isinstance(node, QuotedWord) or isinstance(node, BracedWord): + return node.contents + return getattr(node, "contents", None) + + def _collect(node): + # Depth-first walk to collect set/global declarations + if isinstance(node, Command) and hasattr(node.routine, "contents"): + name = node.routine.contents + if name == "global": + for an in node.args: + text = _word_text(an) + if text: + declared_globals.add(text) + elif name == "set" and node.args: + # set var [value] - first arg is the variable name + first_arg = node.args[0] + text = _word_text(first_arg) + if text: + base = text.split("(", 1)[0] # Handle array syntax + if not base.startswith("::"): + locals_set.add(base) + # Recurse + for ch in getattr(node, "children", []): + _collect(ch) + + _collect(body_node) + + # Second pass: flag VarSub usages that are not accounted for + def _scan(node): + if isinstance(node, VarSub): + var_name = node.value or "" + # Strip leading namespace separator for comparison; treat only leading :: as global + if var_name.startswith("::"): + return + base = var_name + # VarSub.value does not include index; indices are separate children + if base not in locals_set and base not in declared_globals: + msg = f"Variable '{base}' used in proc is not set locally and not declared global; use 'global {base}' or '$::{base}'" + self._violations.append(Violation(Rules.UNDECLARED_VARIABLE, msg, node.pos, node.end_pos)) + for ch in getattr(node, "children", []): + _scan(ch) + + _scan(body_node) + + def get_checkers(): - checkers = (CommandArgsCheck(),) + checkers = ( + CommandArgsCheck(), + UndeclaredVariableCheck(), + ) return checkers