add tclint

server/src/tools/checks.py

@@ -1,240 +1,227 @@
-"""Helpers for checking command arguments."""
+import re
 
-from collections.abc import Callable
-from typing import List, Optional, Union
+from src.tools.commands import get_commands
+from src.tools.violations import Rule, Violation
 
-from syntax_tree import ArgExpansion, QuotedWord, BracedWord, BareWord, Node
+from src.tools.syntax_tree import (
+    Visitor,
+    BracedExpression,
+    Expression,
+    BracedWord,
+    QuotedWord,
+    CommandSub,
+)
 
 
-class CommandArgError(Exception):
-    pass
+class LineLengthChecker:
+    """Ensures lines aren't too long.
 
+    Reports 'line-length' violations.
+    """
 
-def arg_count(args, parser):
-    # TODO: graceful handling of argsub going into things with recursive parsing.
-    # if the argsub happens to be "concrete", we can technically do the right
-    # thing (although this should probably be flagged as a readability issue...)
-    # otherwise, we should flag that the non-concrete argsub is not okay for
-    # these cases. however, I think its not okay-ness doesn't need to be absolute, e.g.
-    # I think we could allow:
-    #
-    #  catch {puts "my script"} {*}$catchopts
-    #
+    # ref: https://github.com/eslint/eslint/blob/b29a16b22f234f6134475efb6c7be5ac946556ee/lib/rules/max-len.js#L101 # noqa: E501
+    # ^ ironic lint waiver...
+    URL_RE = re.compile(r"[^:/?#]:\/\/[^?#]")
 
-    arg_count = 0
-    has_arg_expansion = False
-    for arg in args:
-        if isinstance(arg, ArgExpansion):
-            if arg.contents is None:
-                has_arg_expansion = True
+    def check(self, input, _, config):
+        violations = []
+        for i, line in enumerate(input.split("\n")):
+            if self.URL_RE.search(line) is not None:
+                # ignore URLs
                 continue
-            arg_count += len(parser.parse_list(arg.contents))
-        else:
-            arg_count += 1
 
-    return arg_count, has_arg_expansion
-
-
-def check_count(command, min=None, max=None, args_name="args"):
-    def check(args, parser):
-        if min is None and max is None:
-            return None
-
-        count, has_arg_expansion = arg_count(args, parser)
-
-        if not has_arg_expansion and min == max and count != min:
-            raise CommandArgError(
-                f"wrong # of {args_name} for {command}: got {count}, expected {min}"
-            )
-
-        if not has_arg_expansion and min is not None and count < min:
-            raise CommandArgError(
-                f"not enough {args_name} for {command}: got {count}, expected at least"
-                f" {min}"
-            )
-
-        if max is not None and count > max:
-            raise CommandArgError(
-                f"too many {args_name} for {command}: got {count}, expected no more"
-                f" than {max}"
-            )
-
-        return None
-
-    return check
-
-
-def eval(args, parser, command):
-    if len(args) > 1 and any(isinstance(arg, (QuotedWord, BracedWord)) for arg in args):
-        # Slightly odd restriction, but our syntax tree doesn't have a great way
-        # to handle this case. We require each command argument to correspond to
-        # one child node, but multiple quoted or braced word arguments can be
-        # combined into a single subcommand when interpreted eval-style. This
-        # requirement exists to facilitate style checking, if we had a separate
-        # CST for style checks and AST for logical checks we may be able to
-        # handle it.
-
-        raise CommandArgError(
-            f"unable to parse multiple {command} arguments when one includes a braced"
-            " or quoted word"
-        )
-
-    # Construct the body of the eval taking whitespace into account to ensure we get
-    # style checking.
-
-    eval_script = ""
-    prev_arg_end_pos = None
-    for arg in args:
-        contents = arg.contents
-        if contents is None:
-            # TODO: flag sort of eval-specific violation? Common patterns will
-            # often trigger this, and it seems useful to be able to turn it off
-            raise CommandArgError(
-                f"{command} received an argument with a substitution, unable to parse"
-                " its arguments"
-            )
-
-        if prev_arg_end_pos is not None:
-            if prev_arg_end_pos[0] != arg.line:
-                # If we have multiple args on the same line, we know there must be a
-                # backslash newline. Add it so the parsing works.
-                eval_script += "\\\n" * (arg.line - prev_arg_end_pos[0])
-                eval_script += " " * (arg.col - 1)
-            else:
-                eval_script += " " * (arg.col - prev_arg_end_pos[1])
-        eval_script += contents
-
-        prev_arg_end_pos = arg.end_pos
-
-    script = parser.parse(eval_script, pos=(args[0].pos))
-    script.end_pos = args[-1].end_pos
-
-    return [script]
-
-
-def check_command(
-    command: str, args: List[Node], parser, command_spec: Union[Callable, dict, None]
-) -> Optional[List[Node]]:
-    if command_spec is None:
-        return None
-
-    if isinstance(command_spec, dict):
-        return check_arg_spec(command, args, parser, command_spec)
-
-    return command_spec(args, parser)
-
-
-def check_arg_spec(
-    command: str, args: List[Node], parser, arg_spec: dict
-) -> Optional[List[Node]]:
-    if "subcommands" in arg_spec:
-        subcommands = arg_spec["subcommands"]
-        try:
-            subcommand = args[0].contents
-        except IndexError:
-            subcommand = None
-
-        if subcommand in subcommands:
-            new_args = check_command(
-                f"{command} {subcommand}", args[1:], parser, subcommands[subcommand]
-            )
-            if new_args is None:
-                return new_args
-            return args[0:1] + new_args
-
-        if "" in subcommands:
-            return check_command(command, args, parser, subcommands[""])
-
-        if subcommand is not None:
-            msg = f"invalid subcommand for {command}: got {subcommand}"
-        else:
-            msg = f"no subcommand provided for {command}"
-
-        raise CommandArgError(f"{msg}, expected one of {', '.join(subcommands.keys())}")
-
-    switches = arg_spec["switches"]
-    args_allowed = set(switches)
-    args_required = {switch for switch in switches if switches[switch]["required"]}
-    positional_args = []
-
-    args = list(args)
-    while len(args) > 0:
-        arg = args.pop(0)
-
-        # To facilitate better error messages, we expect that switches are always
-        # specified as BareWords that start with "-" or ">". This lets us throw an
-        # error when a switch-like thing doesn't match any supported arguments,
-        # rather than counting it towards the positional arguments (which usually
-        # ends up in a vague "too many arguments" error). To make tclint interpret a
-        # switch-like word as a positional argument, users should wrap it in "", and
-        # any switches should be BareWords.
-        contents = arg.contents
-        if not (isinstance(arg, BareWord) and contents and contents[0] in {"-", ">"}):
-            positional_args.append(arg)
-            continue
-
-        # TODO check required arguments
-        if contents in args_allowed:
-            if switches[contents]["value"]:
-                try:
-                    args.pop(0)
-                except IndexError:
-                    raise CommandArgError(
-                        f"invalid arguments for {command}: expected value after"
-                        f" {contents}"
+            lineno = i + 1
+            if len(line) > config.style_line_length:
+                start = (lineno, 1)
+                end = (lineno, len(line) + 1)
+                violations.append(
+                    Violation(
+                        Rule.LINE_LENGTH,
+                        f"line length is {len(line)}, maximum allowed is"
+                        f" {config.style_line_length}",
+                        start,
+                        end,
                     )
-            if not switches[contents]["repeated"]:
-                args_allowed.remove(contents)
-            if contents in args_required:
-                args_required.remove(contents)
-        elif contents in arg_spec:
-            raise CommandArgError(f"duplicate argument for {command}: {contents}")
-        else:
-            prefix_matches = []
-            for switch in switches:
-                if switch.startswith(contents):
-                    prefix_matches.append(switch)
-
-            if len(prefix_matches) == 1:
-                raise CommandArgError(
-                    f"shortened argument for {command}: expand {contents} to"
-                    f" {prefix_matches[0]}"
                 )
 
-            if len(prefix_matches) > 1:
-                raise CommandArgError(
-                    f"ambiguous argument for {command}: {contents} could be any of"
-                    f" {', '.join(prefix_matches)}"
+        return violations
+
+
+class TrailingWhitespaceChecker:
+    """Ensures lines don't include trailing whitespace.
+
+    Reports 'trailing-whitespace' violations.
+    """
+
+    def check(self, input, _, config):
+        violations = []
+        for i, line in enumerate(input.split("\n")):
+            lineno = i + 1
+
+            WHITESPACE = (" ", "\t")
+            if line.endswith(WHITESPACE):
+                start_col = len(line.rstrip("".join(WHITESPACE)))
+                start = (lineno, start_col + 1)
+                end = (lineno, len(line) + 1)
+                violations.append(
+                    Violation(
+                        Rule.TRAILING_WHITESPACE,
+                        "line has trailing whitespace",
+                        start,
+                        end,
+                    )
                 )
 
-            raise CommandArgError(f"unrecognized argument for {command}: {contents}")
+        return violations
 
-    if len(args_required) > 1:
-        raise CommandArgError(
-            f"missing required arguments for {command}: {', '.join(args_required)}"
-        )
-    elif len(args_required) == 1:
-        raise CommandArgError(
-            f"missing required argument for {command}: {args_required.pop()}"
+
+class RedefinedBuiltinChecker(Visitor):
+    """Ensures names of built-in commands aren't reused by proc definitions.
+
+    Reports 'redefined-builtin' violations.
+    """
+
+    def check(self, _, tree, config):
+        self._violations = []
+
+        plugins = [config.commands] if config.commands is not None else []
+        commands = get_commands(plugins)
+        self._commands = commands.keys()
+
+        tree.accept(self, recurse=True)
+
+        return self._violations
+
+    def visit_command(self, command):
+        if command.routine.contents != "proc":
+            return
+
+        if len(command.args) == 0:
+            # This is a syntax error, but should already be caught as a command-args
+            # error by the parser's `proc` command handling.
+            return
+
+        name = command.args[0].contents
+
+        if name in self._commands:
+            self._violations.append(
+                Violation(
+                    Rule.REDEFINED_BUILTIN,
+                    f"redefinition of built-in command '{name}'",
+                    command.pos,
+                    command.args[1].end_pos,
+                )
+            )
+
+
+class UnbracedExprChecker(Visitor):
+    def check(self, _, tree, __):
+        self._violations = []
+        tree.accept(self, recurse=True)
+        return self._violations
+
+    def visit_command(self, command):
+        if command.routine.contents != "expr":
+            return
+
+        if len(command.args) == 0:
+            # This is a syntax error, but should already be caught as a command-args
+            # error by the parser's `expr` command handling.
+            return
+
+        if len(command.args) == 1 and isinstance(
+            command.args[0], (BracedExpression, Expression)
+        ):
+            return
+
+        # If we got here, tclint had trouble parsing the expression due to one of the
+        # two following cases.
+
+        for child in command.args:
+            if child.contents is None:
+                self._violations.append(
+                    Violation(
+                        Rule.UNBRACED_EXPR,
+                        "expression with substitutions should be enclosed by braces",
+                        command.args[0].pos,
+                        command.args[-1].end_pos,
+                    )
+                )
+                return
+
+        for child in command.args:
+            if isinstance(child, (BracedWord, QuotedWord)):
+                self._violations.append(
+                    Violation(
+                        Rule.UNBRACED_EXPR,
+                        "expression containing braced or quoted words should be"
+                        " enclosed by braces",
+                        command.args[0].pos,
+                        command.args[-1].end_pos,
+                    )
+                )
+                return
+
+        # If we reach here, there's probably a bug in expr parsing logic.
+        assert False, (
+            "Children of expr node were different than expected, please file a bug"
+            " report"
         )
 
-    min_positionals = 0
-    max_positionals: Optional[int] = 0
-    for positional in arg_spec["positionals"]:
-        if positional["value"]["type"] == "variadic":
-            max_positionals = None
 
-        if positional["required"]:
-            min_positionals += 1
-        if max_positionals is not None:
-            max_positionals += 1
+class RedundantExprChecker(Visitor):
+    def check(self, _, tree, __):
+        self._violations = []
+        tree.accept(self, recurse=True)
+        return self._violations
 
-    check = check_count(
-        command,
-        min=min_positionals,
-        max=max_positionals,
-        args_name="positional args",
+    def _check_operand(self, operand):
+        if not isinstance(operand, CommandSub) or len(operand.children) != 1:
+            return
+
+        command = operand.children[0]
+        if command.routine.contents == "expr":
+            self._violations.append(
+                Violation(
+                    Rule.REDUNDANT_EXPR,
+                    "unnecessary command substitution within expression",
+                    operand.pos,
+                    operand.end_pos,
+                )
+            )
+
+    def visit_braced_expression(self, expression):
+        if len(expression.children) == 1:
+            self._check_operand(expression.children[0])
+
+    def visit_expression(self, expression):
+        if len(expression.children) == 1:
+            self._check_operand(expression.children[0])
+
+    def visit_unary_op(self, expr):
+        self._check_operand(expr.children[1])
+
+    def visit_binary_op(self, expr):
+        self._check_operand(expr.children[0])
+        self._check_operand(expr.children[2])
+
+    def visit_ternary_op(self, expr):
+        self._check_operand(expr.children[0])
+        self._check_operand(expr.children[2])
+        self._check_operand(expr.children[4])
+
+    def visit_function(self, function):
+        for arg in function.children[1:]:
+            self._check_operand(arg)
+
+
+def get_checkers():
+    checkers = (
+        RedefinedBuiltinChecker(),
+        UnbracedExprChecker(),
+        RedundantExprChecker(),
+        LineLengthChecker(),
+        TrailingWhitespaceChecker(),
     )
-    check(positional_args, None)
 
-    return None
+    return checkers

server/src/tools/commands/__init__.py

@@ -1,11 +1,11 @@
 import pathlib
 from typing import List, Dict, Union
 
-from tools.commands import builtin as _builtin
-from tools.commands.plugins import PluginManager
+from src.tools.commands import builtin as _builtin
+from src.tools.commands.plugins import PluginManager
 
 # import to expose in package
-from tools.commands.checks import CommandArgError
+from src.tools.commands.checks import CommandArgError
 
 __all__ = ["CommandArgError", "validate_command_plugins", "get_commands"]
 

server/src/tools/commands/checks.py

@@ -0,0 +1,240 @@
+"""Helpers for checking command arguments."""
+
+from collections.abc import Callable
+from typing import List, Optional, Union
+
+from src.tools.syntax_tree import ArgExpansion, QuotedWord, BracedWord, BareWord, Node
+
+
+class CommandArgError(Exception):
+    pass
+
+
+def arg_count(args, parser):
+    # TODO: graceful handling of argsub going into things with recursive parsing.
+    # if the argsub happens to be "concrete", we can technically do the right
+    # thing (although this should probably be flagged as a readability issue...)
+    # otherwise, we should flag that the non-concrete argsub is not okay for
+    # these cases. however, I think its not okay-ness doesn't need to be absolute, e.g.
+    # I think we could allow:
+    #
+    #  catch {puts "my script"} {*}$catchopts
+    #
+
+    arg_count = 0
+    has_arg_expansion = False
+    for arg in args:
+        if isinstance(arg, ArgExpansion):
+            if arg.contents is None:
+                has_arg_expansion = True
+                continue
+            arg_count += len(parser.parse_list(arg.contents))
+        else:
+            arg_count += 1
+
+    return arg_count, has_arg_expansion
+
+
+def check_count(command, min=None, max=None, args_name="args"):
+    def check(args, parser):
+        if min is None and max is None:
+            return None
+
+        count, has_arg_expansion = arg_count(args, parser)
+
+        if not has_arg_expansion and min == max and count != min:
+            raise CommandArgError(
+                f"wrong # of {args_name} for {command}: got {count}, expected {min}"
+            )
+
+        if not has_arg_expansion and min is not None and count < min:
+            raise CommandArgError(
+                f"not enough {args_name} for {command}: got {count}, expected at least"
+                f" {min}"
+            )
+
+        if max is not None and count > max:
+            raise CommandArgError(
+                f"too many {args_name} for {command}: got {count}, expected no more"
+                f" than {max}"
+            )
+
+        return None
+
+    return check
+
+
+def eval(args, parser, command):
+    if len(args) > 1 and any(isinstance(arg, (QuotedWord, BracedWord)) for arg in args):
+        # Slightly odd restriction, but our syntax tree doesn't have a great way
+        # to handle this case. We require each command argument to correspond to
+        # one child node, but multiple quoted or braced word arguments can be
+        # combined into a single subcommand when interpreted eval-style. This
+        # requirement exists to facilitate style checking, if we had a separate
+        # CST for style checks and AST for logical checks we may be able to
+        # handle it.
+
+        raise CommandArgError(
+            f"unable to parse multiple {command} arguments when one includes a braced"
+            " or quoted word"
+        )
+
+    # Construct the body of the eval taking whitespace into account to ensure we get
+    # style checking.
+
+    eval_script = ""
+    prev_arg_end_pos = None
+    for arg in args:
+        contents = arg.contents
+        if contents is None:
+            # TODO: flag sort of eval-specific violation? Common patterns will
+            # often trigger this, and it seems useful to be able to turn it off
+            raise CommandArgError(
+                f"{command} received an argument with a substitution, unable to parse"
+                " its arguments"
+            )
+
+        if prev_arg_end_pos is not None:
+            if prev_arg_end_pos[0] != arg.line:
+                # If we have multiple args on the same line, we know there must be a
+                # backslash newline. Add it so the parsing works.
+                eval_script += "\\\n" * (arg.line - prev_arg_end_pos[0])
+                eval_script += " " * (arg.col - 1)
+            else:
+                eval_script += " " * (arg.col - prev_arg_end_pos[1])
+        eval_script += contents
+
+        prev_arg_end_pos = arg.end_pos
+
+    script = parser.parse(eval_script, pos=(args[0].pos))
+    script.end_pos = args[-1].end_pos
+
+    return [script]
+
+
+def check_command(
+    command: str, args: List[Node], parser, command_spec: Union[Callable, dict, None]
+) -> Optional[List[Node]]:
+    if command_spec is None:
+        return None
+
+    if isinstance(command_spec, dict):
+        return check_arg_spec(command, args, parser, command_spec)
+
+    return command_spec(args, parser)
+
+
+def check_arg_spec(
+    command: str, args: List[Node], parser, arg_spec: dict
+) -> Optional[List[Node]]:
+    if "subcommands" in arg_spec:
+        subcommands = arg_spec["subcommands"]
+        try:
+            subcommand = args[0].contents
+        except IndexError:
+            subcommand = None
+
+        if subcommand in subcommands:
+            new_args = check_command(
+                f"{command} {subcommand}", args[1:], parser, subcommands[subcommand]
+            )
+            if new_args is None:
+                return new_args
+            return args[0:1] + new_args
+
+        if "" in subcommands:
+            return check_command(command, args, parser, subcommands[""])
+
+        if subcommand is not None:
+            msg = f"invalid subcommand for {command}: got {subcommand}"
+        else:
+            msg = f"no subcommand provided for {command}"
+
+        raise CommandArgError(f"{msg}, expected one of {', '.join(subcommands.keys())}")
+
+    switches = arg_spec["switches"]
+    args_allowed = set(switches)
+    args_required = {switch for switch in switches if switches[switch]["required"]}
+    positional_args = []
+
+    args = list(args)
+    while len(args) > 0:
+        arg = args.pop(0)
+
+        # To facilitate better error messages, we expect that switches are always
+        # specified as BareWords that start with "-" or ">". This lets us throw an
+        # error when a switch-like thing doesn't match any supported arguments,
+        # rather than counting it towards the positional arguments (which usually
+        # ends up in a vague "too many arguments" error). To make tclint interpret a
+        # switch-like word as a positional argument, users should wrap it in "", and
+        # any switches should be BareWords.
+        contents = arg.contents
+        if not (isinstance(arg, BareWord) and contents and contents[0] in {"-", ">"}):
+            positional_args.append(arg)
+            continue
+
+        # TODO check required arguments
+        if contents in args_allowed:
+            if switches[contents]["value"]:
+                try:
+                    args.pop(0)
+                except IndexError:
+                    raise CommandArgError(
+                        f"invalid arguments for {command}: expected value after"
+                        f" {contents}"
+                    )
+            if not switches[contents]["repeated"]:
+                args_allowed.remove(contents)
+            if contents in args_required:
+                args_required.remove(contents)
+        elif contents in arg_spec:
+            raise CommandArgError(f"duplicate argument for {command}: {contents}")
+        else:
+            prefix_matches = []
+            for switch in switches:
+                if switch.startswith(contents):
+                    prefix_matches.append(switch)
+
+            if len(prefix_matches) == 1:
+                raise CommandArgError(
+                    f"shortened argument for {command}: expand {contents} to"
+                    f" {prefix_matches[0]}"
+                )
+
+            if len(prefix_matches) > 1:
+                raise CommandArgError(
+                    f"ambiguous argument for {command}: {contents} could be any of"
+                    f" {', '.join(prefix_matches)}"
+                )
+
+            raise CommandArgError(f"unrecognized argument for {command}: {contents}")
+
+    if len(args_required) > 1:
+        raise CommandArgError(
+            f"missing required arguments for {command}: {', '.join(args_required)}"
+        )
+    elif len(args_required) == 1:
+        raise CommandArgError(
+            f"missing required argument for {command}: {args_required.pop()}"
+        )
+
+    min_positionals = 0
+    max_positionals: Optional[int] = 0
+    for positional in arg_spec["positionals"]:
+        if positional["value"]["type"] == "variadic":
+            max_positionals = None
+
+        if positional["required"]:
+            min_positionals += 1
+        if max_positionals is not None:
+            max_positionals += 1
+
+    check = check_count(
+        command,
+        min=min_positionals,
+        max=max_positionals,
+        args_name="positional args",
+    )
+    check(positional_args, None)
+
+    return None

server/src/tools/commands/plugins.py

@@ -0,0 +1,86 @@
+from importlib.metadata import entry_points
+import json
+import pathlib
+from typing import Dict, Optional
+from types import ModuleType
+
+import voluptuous
+
+from src.tools.commands.schema import schema as command_schema
+
+
+class _PluginManager:
+    def __init__(self):
+        self._loaded = {}
+        self._installed = {}
+        self._loaded_specs = {}
+        for plugin in entry_points(group="tclint.plugins"):
+            if plugin.name in self._installed:
+                print(f"Warning: found duplicate definitions for plugin {plugin.name}")
+            self._installed[plugin.name] = plugin
+
+    def load(self, name: str) -> Optional[Dict]:
+        if name in self._loaded:
+            return self._loaded[name]
+
+        mod = self._load(name)
+        self._loaded[name] = mod
+        return mod
+
+    def load_from_spec(self, path: pathlib.Path) -> Optional[Dict]:
+        if path in self._loaded_specs:
+            return self._loaded_specs[path]
+
+        spec = self._load_from_spec(path)
+        self._loaded_specs[path] = spec
+        return spec
+
+    def _load_from_spec(self, path: pathlib.Path) -> Optional[Dict]:
+        try:
+            with open(path.expanduser(), "r") as f:
+                spec = json.load(f)
+        except (FileNotFoundError, RuntimeError):
+            print(f"Warning: command spec {path} not found, skipping...")
+            return None
+
+        try:
+            # Apply defaults and validate the spec.
+            spec = command_schema(spec)
+        except voluptuous.Invalid as e:
+            print(f"Warning: invalid command spec {path}: {e}")
+            return None
+
+        return spec["commands"]
+
+    def get_mod(self, name: str) -> Optional[ModuleType]:
+        if name not in self._installed:
+            print(f"Warning: plugin {name} is not installed")
+            return None
+
+        plugin = self._installed[name]
+
+        try:
+            module = plugin.load()
+        except Exception as e:
+            print(f"Warning: error loading plugin {name}: {e}")
+            return None
+
+        return module
+
+    def _load(self, name: str):
+        module = self.get_mod(name)
+        if module is None:
+            print(f"Skipping requested plugin {name}")
+            return None
+
+        if not hasattr(module, "commands"):
+            print(f"Warning: skipping plugin {name} since it does not define commands")
+            return None
+
+        return getattr(module, "commands")
+
+
+# TODO: we'll probably want to construct this in the tclint entry point and pass
+# it around rather than using a singleton instance, but this made for an easier
+# refactor.
+PluginManager = _PluginManager()

server/src/tools/commands/schema.py

@@ -0,0 +1,35 @@
+from collections.abc import Callable
+from voluptuous import Schema, Optional, Or, Self
+
+# Need to define this as a Schema with required=True to ensure that this requirement
+# persists through the Or in the main schema definition.
+_command_args = Schema(
+    {
+        Optional("positionals", default=[]): [
+            {
+                "name": str,
+                "required": bool,
+                "value": Or({"type": "any"}, {"type": "variadic"}),
+            }
+        ],
+        Optional("switches", default={}): {
+            Optional(str): {
+                "required": bool,
+                "repeated": bool,
+                "value": Or({"type": "any"}, None),
+                Optional("metavar"): str,
+            }
+        },
+    },
+    required=True,
+)
+
+commands_schema = Schema(
+    {Optional(str): Or(_command_args, None, {"subcommands": Self}, Callable)},
+    required=True,
+)
+
+schema = Schema(
+    {"name": str, "commands": commands_schema},
+    required=True,
+)

server/src/tools/comments.py

@@ -0,0 +1,91 @@
+from collections import defaultdict
+
+from src.tools.syntax_tree import Visitor
+from src.tools.violations import ALL_RULES, Rule
+
+
+class CommentVisitor(Visitor):
+    """Scans the tree for lint waiver comments."""
+
+    def __init__(self):
+        # line -> [rule]
+        self.ignore_lines = defaultdict(set)
+
+        self._disable_regions = {
+            # rule -> line
+        }
+
+    def run(self, tree, path):
+        self._path = path
+        tree.accept(self, recurse=True)
+
+        # resolve remaining disabled regions
+        last_line = tree.end_pos[0]
+        for rule, start_line in self._disable_regions.items():
+            for line in range(start_line, last_line + 1):
+                self.ignore_lines[line].add(rule)
+
+        return self.ignore_lines
+
+    def visit_comment(self, comment):
+        contents = comment.value.strip()
+
+        if not contents.startswith("tclint-"):
+            return
+
+        split = contents.split(" ", 1)
+
+        command = split[0]
+
+        rule_strs = []
+        if len(split) > 1:
+            rest = split[-1]
+            rule_strs = rest.split("--", 1)[0]
+            rule_strs = rule_strs.replace(" ", "")
+            rule_strs = rule_strs.split(",")
+
+        rules = []
+        if not rule_strs:
+            # default if no rules specified is all violation types
+            rules = ALL_RULES
+        else:
+            for rule in rule_strs:
+                try:
+                    rules.append(Rule(rule))
+                except ValueError:
+                    self._warning(
+                        f"unknown rule '{rule}' provided to '{command}'", comment.pos
+                    )
+
+        if command == "tclint-disable":
+            for rule in rules:
+                # if in dictionary, already disabled - this has no effect
+                if rule not in self._disable_regions:
+                    self._disable_regions[rule] = comment.line
+        elif command == "tclint-disable-line":
+            line = comment.line
+            self.ignore_lines[line].update(rules)
+        elif command == "tclint-disable-next-line":
+            line = comment.line + 1
+            self.ignore_lines[line].update(rules)
+        elif command == "tclint-enable":
+            for rule in rules:
+                if rule in self._disable_regions:
+                    disable_start_line = self._disable_regions[rule]
+                    disable_end_line = comment.line
+
+                    for line in range(disable_start_line, disable_end_line + 1):
+                        self.ignore_lines[line].add(rule)
+
+                    del self._disable_regions[rule]
+        else:
+            self._warning(
+                f"comment starts with '{command}', which looks like a tclint keyword."
+                " Is this a typo?",
+                comment.pos,
+            )
+
+    def _warning(self, message, pos):
+        # TODO: formal warning mechanism
+        prefix = self._path if self._path is not None else "(stdin)"
+        print(f"Warning: {prefix}:{pos[0]}:{pos[1]}: {message}")

server/src/tools/config.py

@@ -0,0 +1,429 @@
+import argparse
+import pathlib
+from typing import Union, List
+from typing import Optional as OptionalType
+import dataclasses
+import sys
+
+if sys.version_info >= (3, 11):
+    import tomllib
+else:
+    import tomli as tomllib
+
+from voluptuous import Schema, Optional, And, Coerce, Invalid, Range
+
+from src.tools.violations import Rule
+
+
+@dataclasses.dataclass
+class Config:
+    """This dataclass defines the supported Config fields and their default
+    values. It provides an external interface for accessing config values.
+
+    The type annotations defined here are fairly loose - more specific type
+    validation (and normalization) is defined by `validators` below.
+    """
+
+    exclude: List[str] = dataclasses.field(default_factory=list)
+    ignore: List[Rule] = dataclasses.field(default_factory=list)
+    commands: OptionalType[pathlib.Path] = dataclasses.field(default=None)
+    extensions: List[str] = dataclasses.field(
+        default_factory=lambda: ["tcl", "sdc", "xdc", "upf"]
+    )
+    style_indent: OptionalType[Union[str, int]] = dataclasses.field(default=None)
+    style_line_length: int = dataclasses.field(default=100)
+    style_max_blank_lines: int = dataclasses.field(default=2)
+    style_indent_namespace_eval: bool = dataclasses.field(default=True)
+    style_spaces_in_braces: bool = dataclasses.field(default=False)
+
+    def apply_cli_args(self, args):
+        args_dict = vars(args)
+        for field in dataclasses.fields(self):
+            if field.name in args_dict and args_dict[field.name] is not None:
+                setattr(self, field.name, args_dict[field.name])
+
+        # Special arguments that aren't handled automatically
+        if "extend_exclude" in args_dict and args_dict["extend_exclude"] is not None:
+            self.exclude.extend(args_dict["extend_exclude"])
+
+        if "extend_ignore" in args_dict and args_dict["extend_ignore"] is not None:
+            self.ignore.extend(args_dict["extend_ignore"])
+
+    def get_indent(self) -> str:
+        """Get indent setting as string.
+
+        This helper does two things. One, it's a helpful utility to factor out the logic
+        required for calculating the indent. Two, it lets us ergonomically store if the
+        indentation is not set in style_indent, which the LSP relies on.
+        """
+        if self.style_indent is None:
+            # Default indent
+            return " " * 4
+        elif self.style_indent == "tab":
+            return "\t"
+        elif isinstance(self.style_indent, int):
+            return " " * self.style_indent
+
+        # Should be unreachable, validated on ingestion of config
+        raise ValueError(
+            f"unexpected value for config.style_indent: {self.style_indent}"
+        )
+
+
+# Validators using `voluptuous` library that check and normalize config inputs.
+# Used for checking both config files as well as config-related CLI args.
+
+# Using these for CLI args adds a constraint that all non-boolean validators
+# need to be able to normalize a value from a string. This means one could put
+# e.g. a string representation of a list into a .toml config file, but we shouldn't
+# document this, since it won't be considered stable behavior.
+
+
+def _str2list(s):
+    """Handles string-to-list normalization."""
+    if isinstance(s, str):
+        if s == "":
+            return []
+        return [v.strip() for v in s.split(",")]
+    return s
+
+
+_VALIDATORS = {
+    # note: it's ok if paths don't exist - allows for generic
+    # configurations with directories like .git/ excluded
+    "exclude": _str2list,
+    "ignore": And(
+        _str2list,
+        [
+            Coerce(Rule, msg="invalid rule ID"),
+        ],
+    ),
+    "commands": Coerce(pathlib.Path),
+    "extensions": _str2list,
+    "style_indent": Coerce(
+        lambda v: v if v == "tab" else int(v), msg="expected integer or 'tab'"
+    ),
+    "style_line_length": Coerce(int),
+    "style_max_blank_lines": And(
+        Coerce(int),
+        # we could technically support i >= 0, but I think 0 would be a weird
+        # setting and this lets us ignore pluralizing the violation message :)
+        Range(min=1),
+    ),
+    "style_indent_namespace_eval": bool,
+    "style_spaces_in_braces": bool,
+}
+
+
+def _validate_config(config):
+    """Validates dictionary read from TOML config file. Individual value validators
+    are implemented in the global dict, this defines the actual structure of the
+    schema."""
+
+    base_config = {
+        Optional("ignore"): _VALIDATORS["ignore"],
+        Optional("commands"): _VALIDATORS["commands"],
+        Optional("style"): {
+            Optional("indent"): _VALIDATORS["style_indent"],
+            Optional("line-length"): _VALIDATORS["style_line_length"],
+            Optional("max-blank-lines"): _VALIDATORS["style_max_blank_lines"],
+            Optional("indent-namespace-eval"): _VALIDATORS[
+                "style_indent_namespace_eval"
+            ],
+            Optional("spaces-in-braces"): _VALIDATORS["style_spaces_in_braces"],
+        },
+    }
+
+    schema = Schema(
+        {
+            # exclude and extensions can only be used in global context
+            Optional("exclude"): _VALIDATORS["exclude"],
+            Optional("extensions"): _VALIDATORS["extensions"],
+            **base_config,
+            Optional("fileset"): Schema(
+                [{"paths": [Coerce(pathlib.Path)], **base_config}], required=True
+            ),
+        }
+    )
+
+    try:
+        return schema(config)
+    except Invalid as e:
+        if not e.path:
+            raise ConfigError(e.error_message)
+
+        # Stringify error path to my own taste.
+        path = []
+        for item in e.path:
+            if isinstance(item, int):
+                # Brackets around indices
+                if len(path) > 0:
+                    path[-1] += f"[{item}]"
+                else:
+                    path.append(f"[{item}]")
+            else:
+                path.append(str(item))
+
+        raise ConfigError(f"{e.error_message} ({'.'.join(path)})")
+
+
+def _validator(key):
+    def func(s):
+        try:
+            return Schema(_VALIDATORS[key])(s)
+        except Invalid as e:
+            raise argparse.ArgumentTypeError(str(e))
+
+    return func
+
+
+def _add_bool(group, parser, dest, yes_flag, no_flag):
+    mutex_group = group.add_mutually_exclusive_group(required=False)
+    mutex_group.add_argument(yes_flag, dest=dest, action="store_true")
+    mutex_group.add_argument(no_flag, dest=dest, action="store_false")
+    parser.set_defaults(**{dest: None})
+
+
+def setup_common_config_cli_args(config_group):
+    config_group.add_argument(
+        "--exclude", type=_validator("exclude"), metavar='"path1, path2, ..."'
+    )
+    config_group.add_argument(
+        "--extend-exclude", type=_validator("exclude"), metavar='"path1, path2, ..."'
+    )
+    config_group.add_argument(
+        "--extensions", type=_validator("extensions"), metavar='"tcl, xdc, ..."'
+    )
+    config_group.add_argument(
+        "--commands", type=_validator("commands"), metavar="<path>"
+    )
+
+
+def setup_config_cli_args(parser):
+    """This method defines config-related CLI arguments.
+
+    The destvars of these switches should match the fields of Config.
+    """
+    config_group = parser.add_argument_group("configuration arguments")
+
+    config_group.add_argument(
+        "--ignore", type=_validator("ignore"), metavar='"rule1, rule2, ..."'
+    )
+    config_group.add_argument(
+        "--extend-ignore", type=_validator("ignore"), metavar='"rule1, rule2, ..."'
+    )
+    setup_common_config_cli_args(config_group)
+    config_group.add_argument(
+        "--style-line-length",
+        type=_validator("style_line_length"),
+        metavar="<line_length>",
+    )
+
+
+def setup_tclfmt_config_cli_args(parser):
+    """This method defines the subset of config-related CLI arguments used by tclfmt.
+
+    The destvars of these switches should match the fields of Config.
+    """
+    config_group = parser.add_argument_group("configuration arguments")
+
+    setup_common_config_cli_args(config_group)
+
+    config_group.add_argument(
+        "--indent",
+        type=_validator("style_indent"),
+        metavar="<indent>",
+        dest="style_indent",
+    )
+    config_group.add_argument(
+        "--max-blank-lines",
+        type=_validator("style_max_blank_lines"),
+        metavar="<max_blank_lines>",
+        dest="style_max_blank_lines",
+    )
+    _add_bool(
+        config_group,
+        parser,
+        "style_indent_namespace_eval",
+        "--indent-namespace-eval",
+        "--no-indent-namespace-eval",
+    )
+    _add_bool(
+        config_group,
+        parser,
+        "style_spaces_in_braces",
+        "--spaces-in-braces",
+        "--no-spaces-in-braces",
+    )
+
+
+def _flatten(d, prefix=None):
+    """Flattens TOML config dictionary structure to match the flat set of fields
+    expected by Config dataclass."""
+    if prefix is None:
+        prefix = []
+
+    flat = {}
+    for k, v in d.items():
+        if isinstance(v, dict):
+            flat.update(_flatten(v, prefix=prefix + [k]))
+        else:
+            flat["_".join(prefix + [k]).replace("-", "_")] = v
+
+    return flat
+
+
+class RunConfig:
+    """Class that holds information about both global and fileset configs. User
+    code can get a Config object that applies to a particular file by calling
+    get_from_path() and supplying that file's path."""
+
+    def __init__(self, global_config=None, fileset_configs=None):
+        if global_config is not None:
+            self._global_config = global_config
+        else:
+            self._global_config = Config()
+
+        self._fileset_configs = [
+            # ([pathlib.Path...], Config])
+        ]
+        if fileset_configs is not None:
+            self._fileset_configs = fileset_configs
+
+    @property
+    def exclude(self):
+        return self._global_config.exclude
+
+    @property
+    def extensions(self):
+        return self._global_config.extensions
+
+    @classmethod
+    def from_dict(cls, config_dict: dict, root: pathlib.Path):
+        config_dict = _validate_config(config_dict)
+        try:
+            fileset_config_dicts = config_dict.pop("fileset")
+        except KeyError:
+            fileset_config_dicts = []
+
+        config_dict = _flatten(config_dict)
+        global_config = Config(**config_dict)
+
+        fileset_configs = []
+        for fileset_config in fileset_config_dicts:
+            paths = []
+            for path in fileset_config.pop("paths"):
+                if not path.is_absolute():
+                    path = root / path
+                paths.append(path.resolve())
+
+            fileset_config = _flatten(fileset_config)
+
+            # pull in default values from global config
+            full_fileset_config = config_dict.copy()
+            full_fileset_config.update(fileset_config)
+
+            fileset_configs.append((paths, Config(**full_fileset_config)))
+
+        return cls(global_config, fileset_configs)
+
+    @classmethod
+    def from_path(cls, path: Union[str, pathlib.Path], root: pathlib.Path):
+        path = pathlib.Path(path)
+
+        if not path.exists():
+            raise FileNotFoundError
+
+        with open(path, "rb") as f:
+            try:
+                data = tomllib.load(f)
+            except tomllib.TOMLDecodeError as e:
+                raise ConfigError(f"{path}: {e}")
+
+        try:
+            return cls.from_dict(data, root)
+        except ConfigError as e:
+            raise ConfigError(f"{path}: {e}")
+
+    @classmethod
+    def from_pyproject(cls, directory=None):
+        if directory is None:
+            directory = pathlib.Path(".")
+        else:
+            directory = pathlib.Path(directory)
+
+        path = directory / "pyproject.toml"
+
+        if not path.exists():
+            raise FileNotFoundError
+
+        with open(path, "rb") as f:
+            data = tomllib.load(f)
+
+        tclint_config = data.get("tool", {})["tclint"]
+
+        try:
+            return cls.from_dict(tclint_config, directory)
+        except ConfigError as e:
+            raise ConfigError(f"pyproject.toml: {e}")
+
+    def get_for_path(self, path) -> Config:
+        if path is None:
+            return self._global_config
+
+        path = path.resolve()
+        for fileset_paths, config in self._fileset_configs:
+            for fileset_path in fileset_paths:
+                if path.is_relative_to(fileset_path):
+                    return config
+
+        return self._global_config
+
+    def apply_cli_args(self, args):
+        self._global_config.apply_cli_args(args)
+        for _, fileset_config in self._fileset_configs:
+            fileset_config.apply_cli_args(args)
+
+
+class ConfigError(Exception):
+    pass
+
+
+DEFAULT_CONFIGS = ("tclint.toml", ".tclint")
+
+
+def get_config(
+    config_path: OptionalType[pathlib.Path], root: pathlib.Path
+) -> OptionalType[RunConfig]:
+    """Loads a config file.
+
+    If `config_path` is supplied, attempts to read config file from this path. If the
+    path can't be found, raises a ConfigError.
+
+    Otherwise, attempts to read config from `root`/{tclint.toml, .tclint,
+    pyproject.toml} (in that order). If none of these files can be found, returns None.
+
+    `root` is also used to resolve some relative paths in the config file.
+    """
+    # user-supplied
+    if config_path is not None:
+        try:
+            return RunConfig.from_path(config_path, root)
+        except FileNotFoundError:
+            raise ConfigError(f"path {config_path} doesn't exist")
+
+    for path in DEFAULT_CONFIGS:
+        try:
+            return RunConfig.from_path(root / path, root)
+        except FileNotFoundError:
+            pass
+
+    try:
+        return RunConfig.from_pyproject(directory=root)
+    except ConfigError as e:
+        raise e
+    except (FileNotFoundError, tomllib.TOMLDecodeError, KeyError):
+        # just skip if file doesn't exist, contains TOML errors, or tclint key not found
+        pass
+
+    return None

server/src/tools/format.py

@@ -0,0 +1,480 @@
+import dataclasses
+import itertools
+import textwrap
+from typing import List, Tuple, Union
+import sys
+
+from src.tools.syntax_tree import (
+    Node,
+    Script,
+    Command,
+    Comment,
+    CommandSub,
+    BareWord,
+    QuotedWord,
+    BracedWord,
+    CompoundBareWord,
+    VarSub,
+    ArgExpansion,
+    Expression,
+    BracedExpression,
+    ParenExpression,
+    UnaryOp,
+    BinaryOp,
+    TernaryOp,
+    Function,
+)
+from src.tools.parser import Parser
+from src.tools.syntax_tree import List as ListNode
+
+
+@dataclasses.dataclass
+class LiteralBlock:
+    block: List[str]
+    pos: Tuple[int, int]
+    end_pos: Tuple[int, int]
+
+
+@dataclasses.dataclass
+class FormatterOpts:
+    indent: str
+    spaces_in_braces: bool
+    max_blank_lines: int
+    indent_namespace_eval: bool
+
+
+class Formatter:
+    def __init__(self, opts: FormatterOpts):
+        self.opts = opts
+
+    def _indent(self, lines: List[str], indent: str) -> List[str]:
+        indented = []
+        for line in lines:
+            if line == "":
+                indented.append("")
+            else:
+                indented.append(indent + line)
+
+        return indented
+
+    def _brace(self, lines: List[str]) -> List[str]:
+        spaces_in_braces = " " if self.opts.spaces_in_braces else ""
+        if lines == [""]:
+            return ["{" + spaces_in_braces + "}"]
+
+        braced_lines = lines[:]
+        braced_lines[0] = "{" + spaces_in_braces + lines[0]
+        braced_lines[-1] += spaces_in_braces + "}"
+        return braced_lines
+
+    def format(self, *nodes: Union[Node, LiteralBlock]) -> List[str]:
+        formatted = []
+        for node in nodes:
+            if isinstance(node, Script):
+                formatted += self.format_script(node)
+            elif isinstance(node, Command):
+                formatted += self.format_command(node)
+            elif isinstance(node, Comment):
+                formatted += self.format_comment(node)
+            elif isinstance(node, CommandSub):
+                formatted += self.format_command_sub(node)
+            elif isinstance(node, BareWord):
+                formatted += self.format_bare_word(node)
+            elif isinstance(node, QuotedWord):
+                formatted += self.format_quoted_word(node)
+            elif isinstance(node, BracedWord):
+                formatted += self.format_braced_word(node)
+            elif isinstance(node, CompoundBareWord):
+                formatted += self.format_compound_bare_word(node)
+            elif isinstance(node, VarSub):
+                formatted += self.format_var_sub(node)
+            elif isinstance(node, ArgExpansion):
+                formatted += self.format_arg_expansion(node)
+            elif isinstance(node, ListNode):
+                formatted += self.format_list(node)
+            elif isinstance(node, Expression):
+                formatted += self.format_expression(node)
+            elif isinstance(node, BracedExpression):
+                formatted += self.format_braced_expression(node)
+            elif isinstance(node, ParenExpression):
+                formatted += self.format_paren_expression(node)
+            elif isinstance(node, UnaryOp):
+                formatted += self.format_unary_op(node)
+            elif isinstance(node, BinaryOp):
+                formatted += self.format_binary_op(node)
+            elif isinstance(node, TernaryOp):
+                formatted += self.format_ternary_op(node)
+            elif isinstance(node, Function):
+                formatted += self.format_function(node)
+            elif isinstance(node, LiteralBlock):
+                formatted += node.block
+            else:
+                assert False, f"unrecognized node: {type(node)}"
+
+        return formatted
+
+    def format_top(self, script: str, parser: Parser) -> str:
+        tree = parser.parse(script)
+        self.script = script.split("\n")
+        return "\n".join(self.format_script_contents(tree)) + "\n"
+
+    def format_partial(self, script: str, parser: Parser) -> str:
+        """Formats a partial Tcl script.
+
+        This function formats a partial script according to the gofmt partial formatting
+        rules, "[preserving] leading indentation as well as leading and trailing spaces"
+        (ref: https://pkg.go.dev/cmd/gofmt#pkg-overview). Unlike Go, we have no way of
+        detecting if a given script is a program fragment, hence the distinct method
+        from `format_top` .
+        """
+        leading = "".join(itertools.takewhile(str.isspace, script))
+        try:
+            leading, indent = leading.rsplit("\n", 1)
+            leading += "\n"
+        except ValueError:
+            leading, indent = "", leading
+        trailing = "".join(itertools.takewhile(str.isspace, reversed(script)))[::-1]
+
+        script = script.strip()
+        tree = parser.parse(script)
+        self.script = script.split("\n")
+
+        formatted = "\n".join(self.format_script_contents(tree))
+
+        return leading + textwrap.indent(formatted, indent) + trailing
+
+    def format_script_contents(self, script: Union[Script, CommandSub]) -> List[str]:
+        to_format = []
+        skip_formatting_start = None
+        for child in script.children:
+            if skip_formatting_start is None:
+                to_format.append(child)
+
+            if isinstance(child, Comment):
+                if child.value.strip() == "tclfmt-disable":
+                    if skip_formatting_start is not None:
+                        print(
+                            "Warning: encountered 'tclint-disable' while formatting is"
+                            " already disabled, ignoring...",
+                            file=sys.stderr,
+                        )
+                    else:
+                        skip_formatting_start = child.pos[0]
+                elif child.value.strip() == "tclfmt-enable":
+                    if skip_formatting_start is None:
+                        print(
+                            "Warning: encountered 'tclint-enable' while formatting is"
+                            " already disabled, ignoring...",
+                            file=sys.stderr,
+                        )
+                    else:
+                        skip_formatting_end = child.pos[0]
+                        block = self.script[skip_formatting_start:skip_formatting_end]
+                        to_format.append(
+                            LiteralBlock(
+                                block,
+                                pos=(skip_formatting_start + 1, 1),
+                                end_pos=(skip_formatting_end, 1),
+                            )
+                        )
+                        skip_formatting_start = None
+
+        if skip_formatting_start is not None:
+            print("Warning: missing 'tclint-enable'", file=sys.stderr)
+            to_format.append(
+                LiteralBlock(
+                    self.script[skip_formatting_start:],
+                    pos=(skip_formatting_start + 1, 1),
+                    end_pos=script.end_pos,
+                )
+            )
+
+        formatted = [""]
+        last_line = None
+        for child in to_format:
+            if last_line is not None:
+                if last_line == child.pos[0]:
+                    if isinstance(child, Comment):
+                        formatted[-1] += " ;"
+                    else:
+                        formatted[-1] += "; "
+                else:
+                    newlines = child.pos[0] - last_line
+                    newlines = min(newlines, self.opts.max_blank_lines + 1)
+                    formatted.extend([""] * newlines)
+            last_line = child.end_pos[0]
+
+            lines = self.format(child)
+            formatted[-1] += lines[0]
+            formatted.extend(lines[1:])
+
+        return formatted
+
+    def format_script(self, script: Script, should_indent=True) -> List[str]:
+        lines = self.format_script_contents(script)
+        if script.pos[0] == script.end_pos[0]:
+            return self._brace(lines)
+
+        # Usually, we enforce that multi-line scripts start on a new line after the open
+        # brace. However, if a comment was originally on the same line as the open brace
+        # we preserve it, since it's probably meant to be associated with this line
+        # (e.g. a tclint-disable-line).
+        open_brace = "{"
+        if (
+            len(script.children) > 0
+            and isinstance(script.children[0], Comment)
+            and script.pos[0] == script.children[0].pos[0]
+        ):
+            open_brace += " " + lines[0]
+            lines = lines[1:]
+
+        if should_indent:
+            return [open_brace] + self._indent(lines, self.opts.indent) + ["}"]
+        else:
+            return [open_brace] + lines + ["}"]
+
+    def format_command(self, command: Command) -> List[str]:
+        is_namespace_eval = (
+            command.routine.contents == "namespace"
+            and len(command.args) > 0
+            and command.args[0].contents == "eval"
+        )
+        should_indent = not is_namespace_eval or self.opts.indent_namespace_eval
+
+        hanging_indent = False
+        formatted = self.format(command.routine)
+        last_line = command.routine.end_pos[0]
+        for child in command.args:
+            if isinstance(child, Script):
+                child_lines = self.format_script(child, should_indent=should_indent)
+            else:
+                child_lines = self.format(child)
+
+            if last_line == child.pos[0]:
+                formatted[-1] += " "
+                formatted[-1] += child_lines[0]
+            else:
+                formatted[-1] += " \\"
+                formatted.append(self.opts.indent + child_lines[0])
+                hanging_indent = True
+
+            if hanging_indent:
+                formatted.extend(self._indent(child_lines[1:], self.opts.indent))
+            else:
+                formatted.extend(child_lines[1:])
+
+            last_line = child.end_pos[0]
+
+        return formatted
+
+    def format_comment(self, comment: Comment) -> List[str]:
+        return [f"#{comment.value}"]
+
+    def format_command_sub(self, command_sub):
+        if len(command_sub.children) == 0:
+            return ["[]"]
+
+        formatted = []
+        contents = self.format_script_contents(command_sub)
+        if len(command_sub.children) > 1 and len(contents) > 1:
+            formatted.append("[")
+            formatted.extend(self._indent(contents, self.opts.indent))
+            formatted.append("]")
+        else:
+            formatted.append("[" + contents[0])
+            formatted.extend(contents[1:])
+            formatted[-1] += "]"
+
+        return formatted
+
+    def format_bare_word(self, word) -> List[str]:
+        # Property enforced by parser
+        assert word.contents is not None
+        return [word.contents]
+
+    def format_quoted_word(self, word) -> List[str]:
+        if word.contents is not None:
+            return [f'"{word.contents}"']
+
+        formatted = ""
+        for child in word.children:
+            formatted += "\n".join(self.format(child))
+
+        return [f'"{formatted}"']
+
+    def format_braced_word(self, word) -> List[str]:
+        assert word.contents is not None
+        return [f"{{{word.contents}}}"]
+
+    def format_compound_bare_word(self, word) -> List[str]:
+        formatted = [""]
+        for child in word.children:
+            child_lines = self.format(child)
+            formatted[-1] += child_lines[0]
+            formatted.extend(child_lines[1:])
+
+        return formatted
+
+    def format_var_sub(self, varsub) -> List[str]:
+        # We might be able to make the formatter infer whether braces are required, and
+        # remove them from the syntax tree. For now it's easier to just mimic the
+        # original format.
+        if varsub.braced:
+            formatted = [f"${{{varsub.value}}}"]
+        else:
+            formatted = [f"${varsub.value}"]
+
+        if varsub.children:
+            # We just concatenate everything as is, since changes in whitespace are
+            # semantically meaningful in this context. Any newlines are captured by
+            # BareWords.
+            formatted[-1] += "("
+            for child in varsub.children:
+                child_lines = self.format(child)
+                formatted[-1] += child_lines[0]
+                formatted.extend(child_lines[1:])
+            formatted[-1] += ")"
+
+        return formatted
+
+    def format_arg_expansion(self, arg_expansion) -> List[str]:
+        lines = self.format(arg_expansion.list)
+        lines[0] = "{*}" + lines[0]
+
+        return lines
+
+    def format_list(self, list_node) -> List[str]:
+        # Similar to Script, but the contents are a bit more straightforward.
+        contents = [""]
+        last_line = None
+        for child in list_node.children:
+            if last_line is not None:
+                if last_line == child.pos[0]:
+                    contents[-1] += " "
+                else:
+                    newlines = child.pos[0] - last_line
+                    newlines = min(newlines, 3)
+                    contents.extend([""] * newlines)
+
+            lines = self.format(child)
+            contents[-1] += lines[0]
+            contents.extend(lines[1:])
+
+            last_line = child.end_pos[0]
+
+        if list_node.pos[0] == list_node.end_pos[0]:
+            return self._brace(contents)
+
+        return ["{"] + self._indent(contents, self.opts.indent) + ["}"]
+
+    def format_expression(self, expr) -> List[str]:
+        formatted = [""]
+        for child in expr.children:
+            lines = self.format(child)
+            formatted[-1] += lines[0]
+            for line in lines[1:]:
+                formatted[-1] += " \\"
+                formatted += self._indent([line], self.opts.indent)
+
+        # Trick: we know there are quotes around the expression if the start of the
+        # expression is a different column than its first child.
+        quoted = expr.pos[1] != expr.children[0].pos[1]
+        if quoted:
+            formatted[0] = '"' + formatted[0]
+            formatted[-1] += '"'
+
+        return formatted
+
+    def format_braced_expression(self, expr) -> List[str]:
+        formatted = [""]
+        for child in expr.children:
+            lines = self.format(child)
+            formatted[-1] += lines[0]
+            formatted.extend(lines[1:])
+
+        if expr.pos[0] == expr.end_pos[0]:
+            return self._brace(formatted)
+
+        return ["{"] + self._indent(formatted, self.opts.indent) + ["}"]
+
+    def format_paren_expression(self, expr) -> List[str]:
+        body = expr.body
+
+        formatted = ["("]
+        lines = self.format(body)
+        if expr.pos[0] != body.pos[0]:
+            formatted.extend(lines)
+        else:
+            formatted[-1] += lines[0]
+            formatted.extend(lines[1:])
+
+        formatted = formatted[0:1] + self._indent(formatted[1:], self.opts.indent)
+
+        if expr.end_pos[0] != body.end_pos[0]:
+            formatted.append(")")
+        else:
+            formatted[-1] += ")"
+
+        return formatted
+
+    def format_unary_op(self, expr):
+        op = self.format(expr.operator)
+        assert len(op) == 1
+
+        lines = self.format(expr.operand)
+        lines[0] = op[0] + lines[0]
+        return lines
+
+    def _format_op(self, expr) -> List[str]:
+        nodes = expr.children
+        formatted = self.format(nodes[0])
+
+        last = nodes[0]
+        for next in nodes[1:]:
+            lines = self.format(next)
+            if last.end_pos[0] != next.pos[0]:
+                formatted.extend(lines)
+            else:
+                formatted[-1] += " "
+                formatted[-1] += lines[0]
+                formatted.extend(lines[1:])
+            last = next
+
+        return formatted
+
+    def format_binary_op(self, expr) -> List[str]:
+        return self._format_op(expr)
+
+    def format_ternary_op(self, expr) -> List[str]:
+        return self._format_op(expr)
+
+    def format_function(self, function):
+        name = self.format(function.name)
+        assert len(name) == 1
+        name = name[0]
+
+        formatted = [f"{name}("]
+
+        last = function.name
+        for i, child in enumerate(function.args):
+            if i > 0:
+                formatted[-1] += ","
+            lines = self.format(child)
+            if last.end_pos[0] != child.pos[0]:
+                formatted.extend(lines)
+            else:
+                if i > 0:
+                    formatted[-1] += " "
+                formatted[-1] += lines[0]
+                formatted.extend(lines[1:])
+            last = child
+
+        # indent any continuation lines, but we leave the closing paren dedented
+        formatted = formatted[0:1] + self._indent(formatted[1:], self.opts.indent)
+
+        if last.end_pos[0] != function.end_pos[0]:
+            formatted.append(")")
+        else:
+            formatted[-1] += ")"
+
+        return formatted

server/src/tools/lexer.py

@@ -1,34 +1,30 @@
-from enum import Enum
 import ply.lex as lex
 from typing import Tuple
 
-
-class Tok(str, Enum):
-    TOK_BACKSLASH_NEWLINE = "BACKSLASH_NEWLINE"
-    TOK_BACKSLASH_SUB = "BACKSLASH_SUB"
-    TOK_NEWLINE = "NEWLINE"
-    TOK_SEMI = "SEMI"
-    TOK_WS = "WS"
-    TOK_QUOTE = "QUOTE"
-    TOK_ARG_EXPANSION = "ARG_EXPANSION"
-    TOK_LBRACE = "LBRACE"
-    TOK_RBRACE = "RBRACE"
-    TOK_STAR = "STAR"
-    TOK_LBRACKET = "LBRACKET"
-    TOK_RBRACKET = "RBRACKET"
-    TOK_DOLLAR = "DOLLAR"
-    TOK_LPAREN = "LPAREN"
-    TOK_RPAREN = "RPAREN"
-    TOK_HASH = "HASH"
-    TOK_ALPHA_CHARS = "ALPHA_CHARS"
-    TOK_NUM_CHARS = "NUM_CHARS"
-    TOK_NAMESPACE_SEP = "NAMESPACE_SEP"
-    TOK_CHAR = "CHAR"
-    TOK_CONTENTS = "CONTENTS"
-
+TOK_BACKSLASH_NEWLINE = "BACKSLASH_NEWLINE"
+TOK_BACKSLASH_SUB = "BACKSLASH_SUB"
+TOK_NEWLINE = "NEWLINE"
+TOK_SEMI = "SEMI"
+TOK_WS = "WS"
+TOK_QUOTE = "QUOTE"
+TOK_ARG_EXPANSION = "ARG_EXPANSION"
+TOK_LBRACE = "LBRACE"
+TOK_RBRACE = "RBRACE"
+TOK_STAR = "STAR"
+TOK_LBRACKET = "LBRACKET"
+TOK_RBRACKET = "RBRACKET"
+TOK_DOLLAR = "DOLLAR"
+TOK_LPAREN = "LPAREN"
+TOK_RPAREN = "RPAREN"
+TOK_HASH = "HASH"
+TOK_ALPHA_CHARS = "ALPHA_CHARS"
+TOK_NUM_CHARS = "NUM_CHARS"
+TOK_NAMESPACE_SEP = "NAMESPACE_SEP"
+TOK_CHAR = "CHAR"
+TOK_CONTENTS = "CONTENTS"
+TOK_EOF = None
 
 STATE_BRACEDWORD = "bracedword"
-TOK_EOF = None
 
 
 class TclSyntaxError(Exception):
@@ -39,27 +35,38 @@ class TclSyntaxError(Exception):
 
 
 class _LexTable:
-    tokens = tuple(t.value for t in Tok)
+    tokens = (
+        TOK_BACKSLASH_NEWLINE,
+        TOK_BACKSLASH_SUB,
+        TOK_NEWLINE,
+        TOK_SEMI,
+        TOK_WS,
+        TOK_QUOTE,
+        TOK_ARG_EXPANSION,
+        TOK_LBRACE,
+        TOK_RBRACE,
+        TOK_STAR,
+        TOK_LBRACKET,
+        TOK_RBRACKET,
+        TOK_DOLLAR,
+        TOK_LPAREN,
+        TOK_RPAREN,
+        TOK_HASH,
+        TOK_ALPHA_CHARS,
+        TOK_NUM_CHARS,
+        TOK_NAMESPACE_SEP,
+        TOK_CHAR,
+        TOK_CONTENTS,
+    )
 
+    # This defines a conditional lexing state for parsing braced words. This is a
+    # performance optimization; since there are few special characters in this context,
+    # we can use a smaller set of tokens to parse them faster. This has a large impact
+    # since most Tcl programs have a large number of braced words. Any token with
+    # `bracedword` in its name is included in this state. Tokens that are included in
+    # this state and the default state also include `INITIAL` in their name.
     states = ((STATE_BRACEDWORD, "exclusive"),)
 
-    def __init__(self):
-        self.lexer = lex.lex(object=self)
-        self.lexer.lineno = 1
-        self.lexer.colno = 1
-
-    def new_lexer(self, pos=None):
-        lexer = self.lexer.clone()
-        lexer.lineno = 1
-        lexer.colno = 1
-
-        if pos is not None:
-            line, col = pos
-            lexer.lineno = line
-            lexer.colno = col
-
-        return lexer
-
     def _tok(self, t):
         pos = (t.lexer.lineno, t.lexer.colno)
         t.lexer.lineno += t.value.count("\n")
@@ -146,6 +153,9 @@ class _LexTable:
         r"[A-Za-z_]+"
         return self._tok(t)
 
+    # Valid numeric chars in variable names
+    # This is split up from the above to facilitate expression parsing, since
+    # e.g. 1eq1 can't be a single token.
     def t_NUM_CHARS(self, t):
         r"[0-9]+"
         return self._tok(t)
@@ -170,6 +180,23 @@ class _LexTable:
         print("Illegal character '%s'" % t.value[0])
         t.lexer.skip(1)
 
+    def __init__(self):
+        self.lexer = lex.lex(object=self)
+        self.lexer.lineno = 1
+        self.lexer.colno = 1
+
+    def new_lexer(self, pos=None):
+        lexer = self.lexer.clone()
+        lexer.lineno = 1
+        lexer.colno = 1
+
+        if pos is not None:
+            line, col = pos
+            lexer.lineno = line
+            lexer.colno = col
+
+        return lexer
+
 
 # Calling `lex.lex()` performs an expensive reflection process to generate the lexer.
 # This singleton class holds a preinitialized lexer that can then be cloned to create
@@ -214,21 +241,3 @@ class Lexer:
     def assert_(self, *tokens):
         assert self.current.type in tokens
         self.next()
-
-
-def dump_tokens(code):
-    lx = Lexer()
-    lx.input(code)
-    out = []
-    while lx.type() is not TOK_EOF:
-        out.append((lx.type(), lx.value(), lx.pos()))
-        lx.next()
-    return out
-
-
-if __name__ == "__main__":
-    code = (
-        "set a 1\nputs $a\nnamespace eval test {}\n proc myProc {arg1 {optArg 10}} {}"
-    )
-    for ttype, val, (ln, col) in dump_tokens(code):
-        print(f"{ttype:<18} {val!r:<10} @ ({ln},{col})")

server/src/tools/parser.py

@@ -1,19 +1,64 @@
-from tools.lexer import Lexer, TclSyntaxError, Tok, TOK_EOF
-from tools import syntax_tree as st
-from tools.commands import CommandArgError, get_commands
-from tools.checks import check_command
+import string
+import re
+
+from src.tools.lexer import (
+    Lexer,
+    TclSyntaxError,
+    STATE_BRACEDWORD,
+    TOK_BACKSLASH_NEWLINE,
+    TOK_NEWLINE,
+    TOK_SEMI,
+    TOK_WS,
+    TOK_QUOTE,
+    TOK_ARG_EXPANSION,
+    TOK_LBRACE,
+    TOK_RBRACE,
+    TOK_LBRACKET,
+    TOK_RBRACKET,
+    TOK_DOLLAR,
+    TOK_LPAREN,
+    TOK_RPAREN,
+    TOK_HASH,
+    TOK_ALPHA_CHARS,
+    TOK_NUM_CHARS,
+    TOK_NAMESPACE_SEP,
+    TOK_EOF,
+)
+from src.tools.syntax_tree import (
+    Script,
+    Comment,
+    Command,
+    CommandSub,
+    ArgExpansion,
+    VarSub,
+    BareWord,
+    BracedWord,
+    QuotedWord,
+    CompoundBareWord,
+    List,
+    Expression,
+    BracedExpression,
+    ParenExpression,
+    UnaryOp,
+    BinaryOp,
+    TernaryOp,
+    Function,
+)
+from src.tools.commands import CommandArgError, get_commands
+from src.tools.commands.checks import check_command
+from src.tools.violations import Rule, Violation
 
 
 def _strip_ws(parse_func):
     """Decorator used by expression parser for stripping whitespace around a node."""
 
     def func(parser, ts):
-        while ts.type() in {Tok.TOK_WS, Tok.TOK_BACKSLASH_NEWLINE, Tok.TOK_NEWLINE}:
+        while ts.type() in {TOK_WS, TOK_BACKSLASH_NEWLINE, TOK_NEWLINE}:
             ts.next()
 
         node = parse_func(parser, ts)
 
-        while ts.type() in {Tok.TOK_WS, Tok.TOK_BACKSLASH_NEWLINE, Tok.TOK_NEWLINE}:
+        while ts.type() in {TOK_WS, TOK_BACKSLASH_NEWLINE, TOK_NEWLINE}:
             ts.next()
 
         return node
@@ -22,6 +67,8 @@ def _strip_ws(parse_func):
 
 
 class _Word:
+    """Helper class for constructing Word nodes out of multiple segments."""
+
     def __init__(self):
         self.segments = []
         self.current_segment = ""
@@ -30,13 +77,12 @@ class _Word:
     def add_tok(self, tok):
         if self.current_start is None:
             self.current_start = tok.value[1]
+        self.current_segment += tok.value[0]
 
     def add_node(self, node):
         if self.current_segment != "":
             self.segments.append(
-                st.BareWord(
-                    self.current_segment, pos=self.current_start, end_pos=node.pos
-                )
+                BareWord(self.current_segment, pos=self.current_start, end_pos=node.pos)
             )
             self.current_segment = ""
             self.current_start = None
@@ -45,9 +91,7 @@ class _Word:
     def resolve(self, end_pos):
         if self.current_segment:
             self.segments.append(
-                st.BareWord(
-                    self.current_segment, pos=self.current_start, end_pos=end_pos
-                )
+                BareWord(self.current_segment, pos=self.current_start, end_pos=end_pos)
             )
 
         return self.segments
@@ -57,7 +101,9 @@ class Parser:
     def __init__(self, debug=False, command_plugins=None):
         self._debug = debug
         self._debug_indent = 0
+        # TODO: better way to handle this?
         self.violations = []
+
         if command_plugins is None:
             command_plugins = []
         self._commands = get_commands(command_plugins)
@@ -726,7 +772,7 @@ class Parser:
             pos=name.pos,
         )
 
-        delims = {Tok.TOK_RPAREN, TOK_EOF}
+        delims = {TOK_RPAREN, TOK_EOF}
 
         arguments = []
         if ts.type() not in delims:
@@ -745,11 +791,11 @@ class Parser:
             arguments.append(self._parse_expression(ts))
 
         ts.expect(
-            Tok.TOK_RPAREN,
+            TOK_RPAREN,
             message="expected close paren after function arguments",
             pos=name.pos,
         )
-        return st.Function(name, *arguments, pos=name.pos, end_pos=ts.pos())
+        return Function(name, *arguments, pos=name.pos, end_pos=ts.pos())
 
 
 def _all(_list, non_empty=False):

server/src/tools/syntax_tree.py

@@ -1,4 +1,4 @@
-"""Classes for representing and interacting with Tcl syntax trees."""
+"""Classes for representing and interacting with Tcl syntax trees. """
 
 
 class Visitor:
@@ -188,12 +188,12 @@ class Node:
             return lines
 
         if len(self.children) != len(other.children):
-            my_children = ",".join(
-                [child.__class__.__name__ for child in self.children]
-            )
-            other_children = ",".join(
-                [child.__class__.__name__ for child in other.children]
-            )
+            my_children = ",".join([
+                child.__class__.__name__ for child in self.children
+            ])
+            other_children = ",".join([
+                child.__class__.__name__ for child in other.children
+            ])
 
             lines += [f"{indent}-{my_cls}({my_children})"]
             lines += [f"{indent}+{other_cls}({other_children})"]

server/src/tools/violations.py

@@ -0,0 +1,50 @@
+from enum import Enum
+from typing import Tuple
+
+
+class Rule(Enum):
+    """This enum serves a few purposes:
+
+    1) define symbols for rule IDs to be used in code
+    2) map these symbols to names in the UI
+    3) collect all rule IDs/provide validation for IDs
+    """
+
+    LINE_LENGTH = "line-length"
+    TRAILING_WHITESPACE = "trailing-whitespace"
+    COMMAND_ARGS = "command-args"
+    REDEFINED_BUILTIN = "redefined-builtin"
+    UNBRACED_EXPR = "unbraced-expr"
+    REDUNDANT_EXPR = "redundant-expr"
+
+    def __str__(self):
+        return self.value
+
+
+ALL_RULES = [rule for rule in Rule]
+
+
+class Violation:
+    def __init__(
+        self, id: Rule, message: str, start: Tuple[int, int], end: Tuple[int, int]
+    ):
+        self.id = id
+        self.message = message
+        self.start = start
+        self.end = end
+
+    def __lt__(self, other):
+        return self.start < other.start
+
+    def __str__(self):
+        line, col = self.start
+        rule = str(self.id)
+
+        return f"{line}:{col}: {self.message} [{rule}]"
+
+    @classmethod
+    def create(cls, id):
+        def func(message: str, start: Tuple[int, int], end: Tuple[int, int]):
+            return cls(id, message, start, end)
+
+        return func