Option for absolute rule imports from project root

- New `enable-root-import` config option for root configs
- Add root config directory, or specified relative path, to `sys.path`
  before materializing configured lint rules.
- Tests
- Documentation

Fixes #316

ghstack-source-id: ca9ce72cebf5100b7aa9aff3ee5cdcdf50e2130c
Pull Request resolved: #338

docs/guide/configuration.rst

@@ -31,11 +31,12 @@ The main configuration table.
 
 .. attribute:: root
     :type: bool
-    :value: False
+    :value: false
 
     Marks this file as a root of the configuration hierarchy.
 
-    If set to ``True``, Fixit will not visit any files further up the hierarchy.
+    If set to ``true``, Fixit will not visit any configuration files further up
+    the filesystem hierarchy.
 
 .. attribute:: enable
     :type: list[str]
@@ -84,26 +85,62 @@ The main configuration table.
 
     See :attr:`enable` for details on referencing lint rules.
 
+.. attribute:: enable-root-import
+    :type: bool | str
+
+    Allow importing local rules using absolute imports, relative to the root
+    of the project. This provides an alternative to using dotted rule names for
+    enabling and importing local rules (see :attr:`enable`) from either the
+    directory containing the root config (when set to ``true``), or a single,
+    optional path relative to the root config.
+
+    For example, project ``orange`` using a ``src/orange/`` project hierarchy
+    could use the following config:
+
+    .. code-block:: toml
+
+        root = True
+        enable-root-import = "src"
+        enable = ["orange.rules"]
+
+    Assuming that the namespace ``orange`` is not already in site-packages,
+    then ``orange.rules`` would be imported from ``src/orange/rules/``, while
+    also allowing these local rules to import from other components in the
+    ``orange`` namespace.
+
+    This option may only be specified in the root config file. Specifying the
+    option in any other config file is treated as a configuration error.
+    Absolute paths, or paths containing ``..`` parent-relative components,
+    are not allowed.
+
+    This option is roughly equivalent to adding the configured path, relative
+    to the root configuration, to :attr:`sys.path` when attempting to import
+    and materialize any enabled lint rules.
+
 .. attribute:: python-version
     :type: str
-    :value: "3.10"
 
     Python version to target when selecting lint rules. Rules with
     :attr:`~fixit.LintRule.PYTHON_VERSION` specifiers that don't match this
     target version will be automatically disabled during linting.
+
+    To target a minimum Python version of 3.10:
+
+    .. code-block:: toml
+
+        python-version = "3.10"
     
     Defaults to the currently active version of Python.
     Set to empty string ``""`` to disable target version checking.
 
 .. attribute:: formatter
     :type: str
-    :value: None
 
     Code formatting style to apply after fixing source files.
 
     Supported code styles:
 
-    - ``None``: No style is applied (default).
+    - ``(unset)``: No style is applied (default).
 
     - ``"black"``: `Black <https://black.rtfd.io>`_ code formatter.
 

makefile

@@ -1,3 +1,4 @@
+all: format test lint html
 
 .PHONY: venv
 venv:

src/fixit/config.py

@@ -9,7 +9,7 @@
 import pkgutil
 import platform
 import sys
-from contextlib import contextmanager
+from contextlib import contextmanager, ExitStack
 from pathlib import Path
 from types import ModuleType
 from typing import (
@@ -24,6 +24,7 @@
     Sequence,
     Set,
     Type,
+    Union,
 )
 
 from packaging.specifiers import SpecifierSet
@@ -44,6 +45,7 @@
     T,
 )
 from .rule import LintRule
+from .util import append_sys_path
 
 if sys.version_info >= (3, 11):
     import tomllib
@@ -207,30 +209,41 @@ def collect_rules(
     else:
         disabled_rules = {}
 
-    for qualified_rule in config.enable:
-        all_rules |= set(find_rules(qualified_rule))
+    with ExitStack() as stack:
+        if config.enable_root_import:
+            path = (
+                config.root / config.enable_root_import
+                if isinstance(config.enable_root_import, Path)
+                else config.root
+            )
+            stack.enter_context(append_sys_path(path))
 
-    for qualified_rule in config.disable:
-        disabled_rules.update({r: "disabled" for r in find_rules(qualified_rule)})
-        all_rules -= set(disabled_rules)
+        for qualified_rule in config.enable:
+            all_rules |= set(find_rules(qualified_rule))
 
-    if config.tags:
-        disabled_rules.update(
-            {R: "tags" for R in all_rules if R.TAGS not in config.tags}
-        )
-        all_rules -= set(disabled_rules)
-
-    if config.python_version is not None:
-        disabled_rules.update(
-            {
-                R: "python_version"
-                for R in all_rules
-                if config.python_version not in SpecifierSet(R.PYTHON_VERSION)
-            }
-        )
-        all_rules -= set(disabled_rules)
+        for qualified_rule in config.disable:
+            disabled_rules.update({r: "disabled" for r in find_rules(qualified_rule)})
+            all_rules -= set(disabled_rules)
+
+        if config.tags:
+            disabled_rules.update(
+                {R: "tags" for R in all_rules if R.TAGS not in config.tags}
+            )
+            all_rules -= set(disabled_rules)
+
+        if config.python_version is not None:
+            disabled_rules.update(
+                {
+                    R: "python_version"
+                    for R in all_rules
+                    if config.python_version not in SpecifierSet(R.PYTHON_VERSION)
+                }
+            )
+            all_rules -= set(disabled_rules)
 
-    return [R() for R in all_rules]
+        materialized_rules = [R() for R in all_rules]
+
+    return materialized_rules
 
 
 def locate_configs(path: Path, root: Optional[Path] = None) -> List[Path]:
@@ -369,6 +382,7 @@ def merge_configs(
     """
 
     config: RawConfig
+    enable_root_import: Union[bool, Path] = Config.enable_root_import
     enable_rules: Set[QualifiedRule] = {QualifiedRule("fixit.rules")}
     disable_rules: Set[QualifiedRule] = set()
     rule_options: RuleOptionsTable = {}
@@ -436,6 +450,25 @@ def process_subpath(
         if data.pop("root", False):
             root = config.path.parent
 
+        if value := data.pop("enable-root-import", False):
+            if root != config.path.parent:
+                raise ConfigError(
+                    "enable-root-import not allowed in non-root configs", config=config
+                )
+            if isinstance(value, str):
+                value_path = Path(value)
+                if value_path.is_absolute():
+                    raise ConfigError(
+                        "enable-root-import: absolute paths not allowed", config=config
+                    )
+                if ".." in value_path.parts:
+                    raise ConfigError(
+                        "enable-root-import: '..' components not allowed", config=config
+                    )
+                enable_root_import = value_path
+            else:
+                enable_root_import = True
+
         process_subpath(
             config.path.parent,
             enable=get_sequence(config, "enable"),
@@ -471,6 +504,7 @@ def process_subpath(
     return Config(
         path=path,
         root=root or Path(path.anchor),
+        enable_root_import=enable_root_import,
         enable=sorted(enable_rules),
         disable=sorted(disable_rules),
         options=rule_options,

src/fixit/ftypes.py

@@ -190,6 +190,10 @@ class Config:
     path: Path = field(default_factory=Path)
     root: Path = field(default_factory=Path.cwd)
 
+    # feature flags
+    enable_root_import: Union[bool, Path] = False
+
+    # rule selection
     enable: List[QualifiedRule] = field(
         default_factory=lambda: [QualifiedRule("fixit.rules")]
     )

src/fixit/tests/config.py

@@ -29,6 +29,7 @@ def setUp(self):
                 """
                 [tool.fixit]
                 root = true
+                enable-root-import = true
                 enable = ["more.rules"]
                 disable = ["fixit.rules.SomethingSpecific"]
                 python_version = "3.8"
@@ -168,6 +169,7 @@ def test_read_configs(self):
                         top,
                         {
                             "root": True,
+                            "enable-root-import": True,
                             "enable": ["more.rules"],
                             "disable": ["fixit.rules.SomethingSpecific"],
                             "python_version": "3.8",
@@ -197,6 +199,7 @@ def test_read_configs(self):
                         top,
                         {
                             "root": True,
+                            "enable-root-import": True,
                             "enable": ["more.rules"],
                             "disable": ["fixit.rules.SomethingSpecific"],
                             "python_version": "3.8",
@@ -223,6 +226,7 @@ def test_read_configs(self):
                         top,
                         {
                             "root": True,
+                            "enable-root-import": True,
                             "enable": ["more.rules"],
                             "disable": ["fixit.rules.SomethingSpecific"],
                             "python_version": "3.8",
@@ -356,6 +360,7 @@ def test_generate_config(self):
                 Config(
                     path=self.outer / "foo.py",
                     root=self.tdp,
+                    enable_root_import=True,
                     enable=[
                         QualifiedRule(".localrules", local=".", root=self.outer),
                         QualifiedRule("more.rules"),
@@ -385,6 +390,7 @@ def test_generate_config(self):
                 Config(
                     path=self.tdp / "other" / "foo.py",
                     root=self.tdp,
+                    enable_root_import=True,
                     enable=[
                         QualifiedRule(".globalrules", local=".", root=self.tdp),
                         QualifiedRule("more.rules"),
@@ -405,6 +411,7 @@ def test_generate_config(self):
                 Config(
                     path=self.tdp / "foo.py",
                     root=self.tdp,
+                    enable_root_import=True,
                     enable=[QualifiedRule("fixit.rules"), QualifiedRule("more.rules")],
                     disable=[QualifiedRule("fixit.rules.SomethingSpecific")],
                     python_version=Version("3.8"),
@@ -415,6 +422,16 @@ def test_generate_config(self):
                 actual = config.generate_config(path, root)
                 self.assertDictEqual(asdict(expected), asdict(actual))
 
+    def test_invalid_config(self):
+        with self.subTest("inner enable-root-import"):
+            (self.tdp / "pyproject.toml").write_text("[tool.fixit]\nroot = true\n")
+            (self.tdp / "outer" / "pyproject.toml").write_text(
+                "[tool.fixit]\nenable-root-import = true\n"
+            )
+
+            with self.assertRaisesRegex(config.ConfigError, "enable-root-import"):
+                config.generate_config(self.tdp / "outer" / "foo.py")
+
     def test_collect_rules(self):
         from fixit.rules.avoid_or_in_except import AvoidOrInExceptRule
         from fixit.rules.cls_in_classmethod import UseClsInClassmethodRule

src/fixit/util.py

@@ -3,6 +3,9 @@
 # This source code is licensed under the MIT license found in the
 # LICENSE file in the root directory of this source tree.
 
+import sys
+from contextlib import contextmanager
+from pathlib import Path
 from typing import cast, Generator, Generic, Optional, TypeVar, Union
 
 Yield = TypeVar("Yield")
@@ -63,3 +66,24 @@ def result(self) -> Return:
         if self._result is Sentinel:
             raise ValueError("Generator hasn't completed")
         return cast(Return, self._result)
+
+
+@contextmanager
+def append_sys_path(path: Path) -> Generator[None, None, None]:
+    """
+    Append a path to ``sys.path`` temporarily, and then remove it again when done.
+
+    If the given path is already in ``sys.path``, it will not be added a second time,
+    and it will not be removed when leaving the context.
+    """
+    path_str = path.as_posix()
+
+    # not there: append to path, and remove it when leaving the context
+    if path_str not in sys.path:
+        sys.path.append(path_str)
+        yield
+        sys.path.remove(path_str)
+
+    # already there: do nothing, and don't remove it later
+    else:
+        yield