plugins: get rid of PluginManager singleton

This rids us of some tech debt and will facilitate configuration of the
plugin manager in a future commit.

Author: nmoroze

src/tclint/checks.py

@@ -1,6 +1,6 @@
 import re
 
-from tclint.commands import get_commands
+from tclint.commands.plugins import PluginManager
 from tclint.config import Config
 from tclint.syntax_tree import (
     BracedExpression,
@@ -82,11 +82,14 @@ class RedefinedBuiltinChecker(Visitor):
     Reports 'redefined-builtin' violations.
     """
 
+    def __init__(self, plugin_manager: PluginManager):
+        self._plugin_manager = plugin_manager
+
     def check(self, _, tree: Script, config: Config) -> list[Violation]:
         self._violations: list[Violation] = []
 
         plugins = [config.commands] if config.commands is not None else []
-        commands = get_commands(plugins)
+        commands = self._plugin_manager.get_commands(plugins)
         self._commands = commands.keys()
 
         tree.accept(self, recurse=True)
@@ -216,9 +219,9 @@ def visit_function(self, function):
             self._check_operand(arg)
 
 
-def get_checkers():
+def get_checkers(plugin_manager: PluginManager):
     checkers = (
-        RedefinedBuiltinChecker(),
+        RedefinedBuiltinChecker(plugin_manager),
         UnbracedExprChecker(),
         RedundantExprChecker(),
         LineLengthChecker(),

src/tclint/cli/tclfmt.py

@@ -6,6 +6,7 @@
 
 from tclint.cli.resolver import Resolver
 from tclint.cli.utils import register_codec_warning
+from tclint.commands.plugins import PluginManager
 from tclint.config import Config, ConfigError, setup_tclfmt_config_cli_args
 from tclint.format import Formatter, FormatterOpts
 from tclint.parser import Parser, TclSyntaxError
@@ -22,9 +23,11 @@
 EXIT_INPUT_ERROR = 4
 
 
-def format(script: str, config: Config, debug=False, partial=False) -> str:
-    plugins = [config.commands] if config.commands is not None else []
-    parser = Parser(debug=debug, command_plugins=plugins)
+def format(
+    script: str, config: Config, plugins: PluginManager, debug=False, partial=False
+) -> str:
+    _plugins = [config.commands] if config.commands is not None else []
+    parser = Parser(debug=debug, commands=plugins.get_commands(_plugins))
 
     formatter = Formatter(
         FormatterOpts(
@@ -125,6 +128,8 @@ def main():
         print(f"Invalid config file: {e}")
         return EXIT_INPUT_ERROR
 
+    plugin_manager = PluginManager()
+
     retcode = EXIT_OK
 
     register_codec_warning("replace_with_warning")
@@ -143,6 +148,7 @@ def main():
             formatted = format(
                 script,
                 config,
+                plugin_manager,
                 debug=(args.debug > 1),
                 partial=args.partial,
             )

src/tclint/cli/tclint.py

@@ -8,6 +8,7 @@
 from tclint.checks import get_checkers
 from tclint.cli.resolver import Resolver
 from tclint.cli.utils import register_codec_warning
+from tclint.commands.plugins import PluginManager
 from tclint.comments import CommentVisitor
 from tclint.config import Config, ConfigError, setup_config_cli_args
 from tclint.parser import Parser, TclSyntaxError
@@ -47,11 +48,12 @@ def filter_violations(
 def lint(
     script: str,
     config: Config,
+    plugins: PluginManager,
     path: Optional[pathlib.Path],
     debug=0,
 ) -> list[Violation]:
-    plugins = [config.commands] if config.commands is not None else []
-    parser = Parser(debug=(debug > 0), command_plugins=plugins)
+    _plugins = [config.commands] if config.commands is not None else []
+    parser = Parser(debug=(debug > 0), commands=plugins.get_commands(_plugins))
 
     violations = []
     tree = parser.parse(script)
@@ -60,7 +62,7 @@ def lint(
     if debug > 0:
         print(tree.pretty(positions=(debug > 1)))
 
-    for checker in get_checkers():
+    for checker in get_checkers(plugins):
         violations += checker.check(script, tree, config)
 
     v = CommentVisitor()
@@ -126,6 +128,8 @@ def main():
         print(f"Invalid config file: {e}")
         return EXIT_INPUT_ERROR
 
+    plugin_manager = PluginManager()
+
     retcode = EXIT_OK
 
     register_codec_warning("replace_with_warning")
@@ -143,6 +147,7 @@ def main():
             violations = lint(
                 script,
                 config,
+                plugin_manager,
                 path,
                 debug=args.debug,
             )

src/tclint/cli/tclsp.py

@@ -11,6 +11,7 @@
 from pygls.workspace import TextDocument
 
 from tclint.cli import tclint, utils
+from tclint.commands.plugins import PluginManager
 from tclint.config import DEFAULT_CONFIGS, Config, ConfigError, load_config_at
 from tclint.format import Formatter, FormatterOpts
 from tclint.lexer import TclSyntaxError
@@ -26,11 +27,11 @@
 _DEFAULT_CONFIG = Config()
 
 
-def lint(source, config, path):
+def lint(source, config, plugin_manager, path):
     diagnostics = []
 
     try:
-        violations = tclint.lint(source, config, path)
+        violations = tclint.lint(source, config, plugin_manager, path)
     except TclSyntaxError as e:
         return [
             lsp.Diagnostic(
@@ -99,6 +100,8 @@ def __init__(self, *args, **kwargs):
         self.global_settings = ExtensionSettings()
         self.workspace_settings: dict[Path, ExtensionSettings] = {}
 
+        self.plugin_manager = PluginManager()
+
     def get_roots(self) -> list[Path]:
         """Returns root folders currently open in the workspace."""
         roots = []
@@ -221,7 +224,7 @@ def _compute_diagnostics(self, document: TextDocument) -> list[lsp.Diagnostic]:
         if is_excluded(path):
             return []
 
-        return lint(document.source, config, path)
+        return lint(document.source, config, self.plugin_manager, path)
 
     def compute_diagnostics(self, document: TextDocument):
         # `None` sentinel ensures that `diagnostics` gets updated if the URI is not

src/tclint/commands/__init__.py

@@ -1,40 +1,4 @@
-import pathlib
-from collections.abc import Sequence
-
-from tclint.commands import builtin as _builtin
-
-# import to expose in package
+# Import to expose in package.
 from tclint.commands.checks import CommandArgError
-from tclint.commands.plugins import PluginManager
-
-__all__ = ["CommandArgError", "validate_command_plugins", "get_commands"]
-
-
-def validate_command_plugins(plugins: list[str]) -> list[str]:
-    valid_plugins = []
-    for plugin in set(plugins):
-        if PluginManager.load(plugin) is not None:
-            valid_plugins.append(plugin)
-
-    return valid_plugins
-
-
-def get_commands(plugins: Sequence[str | pathlib.Path]) -> dict:
-    commands = {}
-    commands.update(_builtin.commands)
-
-    for plugin in plugins:
-        if isinstance(plugin, str):
-            plugin_commands = PluginManager.load(plugin)
-        elif isinstance(plugin, pathlib.Path):
-            if plugin.suffix == ".py":
-                plugin_commands = PluginManager.load_from_py(plugin)
-            else:
-                plugin_commands = PluginManager.load_from_spec(plugin)
-        else:
-            raise TypeError(f"Plugins must be strings or paths, got {type(plugin)}")
-
-        if plugin_commands is not None:
-            commands.update(plugin_commands)
 
-    return commands
+__all__ = ["CommandArgError"]

src/tclint/commands/plugins.py

@@ -1,16 +1,18 @@
 import json
 import pathlib
+from collections.abc import Sequence
 from importlib.util import module_from_spec, spec_from_file_location
 from types import ModuleType
 from typing import Optional
 
 import voluptuous
 from importlib_metadata import entry_points
 
+from tclint.commands import builtin as _builtin
 from tclint.commands import schema
 
 
-class _PluginManager:
+class PluginManager:
     def __init__(self):
         self._loaded = {}
         self._installed = {}
@@ -123,8 +125,22 @@ def _load_from_py(self, path: pathlib.Path) -> Optional[dict]:
 
         return self._load_module(name, mod)
 
-
-# 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()
+    def get_commands(self, plugins: Sequence[str | pathlib.Path]) -> dict:
+        commands = {}
+        commands.update(_builtin.commands)
+
+        for plugin in plugins:
+            if isinstance(plugin, str):
+                plugin_commands = self.load(plugin)
+            elif isinstance(plugin, pathlib.Path):
+                if plugin.suffix == ".py":
+                    plugin_commands = self.load_from_py(plugin)
+                else:
+                    plugin_commands = self.load_from_spec(plugin)
+            else:
+                raise TypeError(f"Plugins must be strings or paths, got {type(plugin)}")
+
+            if plugin_commands is not None:
+                commands.update(plugin_commands)
+
+        return commands

src/tclint/parser.py

@@ -1,11 +1,10 @@
 import io
 import re
 import string
-from collections.abc import Sequence
-from pathlib import Path
 from typing import Optional, Tuple
 
-from tclint.commands import CommandArgError, get_commands
+from tclint.commands import CommandArgError
+from tclint.commands import builtin as _builtin
 from tclint.commands.checks import check_command
 from tclint.lexer import (
     STATE_BRACEDWORD,
@@ -103,17 +102,15 @@ def resolve(self, end_pos):
 
 
 class Parser:
-    def __init__(
-        self, debug=False, command_plugins: Optional[Sequence[str | Path]] = None
-    ):
+    def __init__(self, debug=False, commands: Optional[dict] = None):
         self._debug = debug
         self._debug_indent = 0
         # TODO: better way to handle this?
         self.violations: list[Violation] = []
 
-        if command_plugins is None:
-            command_plugins = []
-        self._commands = get_commands(command_plugins)
+        if commands is None:
+            commands = _builtin.commands
+        self._commands = commands
 
         # Used to normalize newlines consistently with open()'s universal newlines mode.
         self._decoder = io.IncrementalNewlineDecoder(None, True)

tests/commands/test_plugin.py

@@ -1,8 +1,6 @@
 from pathlib import Path
 
-# Not clean, but we import the _PluginManager class instead of using the singleton so we
-# have isolation between tests.
-from tclint.commands.plugins import _PluginManager as PluginManager
+from tclint.commands.plugins import PluginManager
 
 MY_DIR = Path(__file__).parent.resolve()
 TEST_DATA_DIR = MY_DIR / "data"

tests/plugins/test_expect.py

@@ -1,8 +1,6 @@
 import pytest
 
-# Not clean, but we import the _PluginManager class instead of using the singleton so we
-# have isolation between tests.
-from tclint.commands.plugins import _PluginManager as PluginManager
+from tclint.commands.plugins import PluginManager
 from tclint.parser import Parser
 
 
@@ -13,6 +11,7 @@ def test_load():
 
 @pytest.mark.parametrize("command", ["close", "close -i $spawn_id"])
 def test_parse_valid(command):
-    parser = Parser(command_plugins=["expect"])
+    plugins = PluginManager()
+    parser = Parser(commands=plugins.get_commands(["expect"]))
     parser.parse(command)
     assert len(parser.violations) == 0, f"unexpected violation: {parser.violations[0]}"

tests/test_lint.py

@@ -1,11 +1,17 @@
 import json
 from pathlib import Path
 
-from tclint.cli.tclint import lint
+from tclint.cli.tclint import lint as _lint
+from tclint.commands.plugins import PluginManager
 from tclint.config import Config
 from tclint.violations import Rule
 
 
+def lint(script, config, path):
+    plugin_manager = PluginManager()
+    return _lint(script, config, plugin_manager, path)
+
+
 def test_cmd_args_in_sub():
     """Ensure command args get checked in cmd sub."""
     script = "[puts]"