From ca8a33c9e6cd553398aae77a6a5c7bf435c6e315 Mon Sep 17 00:00:00 2001 From: zigai Date: Thu, 15 Oct 2026 23:21:14 +0200 Subject: [PATCH] fix(cli)!: keep the invoked Python environment --- README.md | 5 +- docs/guide/commands.md | 5 +- docs/guide/configuration.md | 8 +- docs/guide/integrations.md | 9 +- docs/guide/quickstart.md | 11 +- docs/index.md | 5 +- .../skills/create-rattle-lint-rules/SKILL.md | 4 +- .../references/configuration.md | 4 +- src/rattle/cli/app.py | 9 +- src/rattle/cli/environment.py | 70 -------- src/rattle/rule_loading.py | 34 +++- tests/config/test_rule_options.py | 89 +++++++++ tests/test_cli_environment.py | 170 ++++++++++++------ 13 files changed, 270 insertions(+), 153 deletions(-) diff --git a/README.md b/README.md index 47e4c0d..89be3db 100644 --- a/README.md +++ b/README.md @@ -94,7 +94,10 @@ enable = [ forbidden_calls = ["typing.cast"] ``` -Adjust the list to match the project, then lint the repository: +Adjust the list to match the project, then check the repository. Rattle uses +the invoked Python environment; install custom-rule dependencies there. For a +uv-managed project, install Rattle as a project dependency and use +`uv run python -m rattle check .`. ```bash rattle check . diff --git a/docs/guide/commands.md b/docs/guide/commands.md index b33cf01..47b5b7c 100644 --- a/docs/guide/commands.md +++ b/docs/guide/commands.md @@ -6,7 +6,10 @@ $ rattle COMMAND [OPTIONS] ... ``` -Use `--help` on any command to print its supported options. +Use `--help` on any command to print its supported options. Commands use the +invoked Python environment, which must contain any custom-rule dependencies. +For a uv project with Rattle installed as a project dependency, use +`uv run python -m rattle check .`. ## Check options diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md index 107a48b..8fb005f 100644 --- a/docs/guide/configuration.md +++ b/docs/guide/configuration.md @@ -44,10 +44,10 @@ Rattle accepts three selector forms: concrete rule (`module:rule-name`) - exact built-in rule names such as `use-f-string` -Rules bundled with Rattle, or available in the environment's `site-packages`, -can be referenced as a rule collection, as a group by their fully-qualified package -name, individually by adding a colon and the kebab-case rule name, or by the -built-in rule name: +Rules bundled with Rattle, or available in the invoked interpreter's +`site-packages`, can be referenced as a rule collection, by their fully-qualified +package name, individually by adding a colon and the kebab-case rule name, or by +the built-in rule name. Rule dependencies must also be available to that interpreter: ```toml enable = [ diff --git a/docs/guide/integrations.md b/docs/guide/integrations.md index 0236335..ec20009 100644 --- a/docs/guide/integrations.md +++ b/docs/guide/integrations.md @@ -14,7 +14,9 @@ To enable this, install the `lsp` extras, for example: $ pip install "rattle-lint[lsp]" ``` -Then configure an LSP client to launch and connect to the Rattle LSP server. +Configure the LSP client to launch Rattle in a Python environment containing +the `lsp` extras and any custom-rule dependencies. For project-managed +dependencies, use the project environment's Python followed by `-m rattle lsp`. See the {ref}`lsp_command` command reference for usage details. Examples of client setup: @@ -48,7 +50,10 @@ lspconfig.rattle.setup({}) Rattle can be included as a hook for [pre-commit](https://pre-commit.com). Once you have [installed pre-commit](https://pre-commit.com/#installation), -add one of Rattle's hooks to `.pre-commit-config.yaml`. +add one of Rattle's hooks to `.pre-commit-config.yaml`. Python hooks run in +pre-commit's environment; add installed rule packages and their dependencies +with `additional_dependencies`. Local rule sources are resolved from the +repository configuration. To run lint rules on commit: diff --git a/docs/guide/quickstart.md b/docs/guide/quickstart.md index aa2a007..3611ea4 100644 --- a/docs/guide/quickstart.md +++ b/docs/guide/quickstart.md @@ -10,6 +10,10 @@ Install Rattle from PyPI: $ pip install rattle-lint ``` +Install custom-rule dependencies in the same Python environment as Rattle. +For a uv-managed project, install Rattle as a project dependency and run +`uv run python -m rattle check .`. + By default, Rattle runs with no enabled lint rules. Enable rule collections in `pyproject.toml`: @@ -238,9 +242,10 @@ module path relative to the directory containing the config file. This allows Rattle to locate and import the rule without installing a plugin into the environment. -If your custom rule imports other libraries from the repo, those imports must -be relative imports, and they must stay within the same directory tree as the -configuration file. +If your custom rule imports other modules from the repo, use relative imports +within the configuration directory tree, or configure `enable-root-import`. +Third-party rule dependencies must be installed in the Python environment +running Rattle. ``` Once enabled, Rattle can run the new rule against the codebase: diff --git a/docs/index.md b/docs/index.md index f90fc88..299a18f 100644 --- a/docs/index.md +++ b/docs/index.md @@ -90,7 +90,10 @@ enable = [ forbidden_calls = ["typing.cast"] ``` -Adjust the list to match the project, then lint the repository: +Adjust the list to match the project, then check the repository. Rattle uses +the invoked Python environment; install custom-rule dependencies there. For a +uv-managed project, install Rattle as a project dependency and use +`uv run python -m rattle check .`. ```bash rattle check . diff --git a/src/rattle/.agents/skills/create-rattle-lint-rules/SKILL.md b/src/rattle/.agents/skills/create-rattle-lint-rules/SKILL.md index 9469001..8893306 100644 --- a/src/rattle/.agents/skills/create-rattle-lint-rules/SKILL.md +++ b/src/rattle/.agents/skills/create-rattle-lint-rules/SKILL.md @@ -151,7 +151,9 @@ editing configuration. Enable local rules with import selectors and use canonical kebab-case names in configuration and CLI commands. Target options at one concrete rule, never a -package or module selector. +package or module selector. Install rule dependencies in the Python environment +running Rattle. For a uv project with Rattle installed as a project dependency, +use `uv run python -m rattle check .`. Complete this step when `rattle rules ` resolves the intended rule, settings, and path scope without enabling unrelated rules. diff --git a/src/rattle/.agents/skills/create-rattle-lint-rules/references/configuration.md b/src/rattle/.agents/skills/create-rattle-lint-rules/references/configuration.md index 87ea6d8..fd8c39b 100644 --- a/src/rattle/.agents/skills/create-rattle-lint-rules/references/configuration.md +++ b/src/rattle/.agents/skills/create-rattle-lint-rules/references/configuration.md @@ -11,7 +11,9 @@ or applying path-specific scope. ## Installation and layout -Install Rattle with: +Install Rattle and any rule dependencies in the same Python environment. +For a uv project with Rattle installed as a project dependency, use +`uv run python -m rattle check .`. To install Rattle directly: ```bash pip install "rattle-lint>=2.1.0" diff --git a/src/rattle/cli/app.py b/src/rattle/cli/app.py index cdb973a..b8b9000 100644 --- a/src/rattle/cli/app.py +++ b/src/rattle/cli/app.py @@ -13,11 +13,7 @@ from rattle.cli.commands.lsp import lsp from rattle.cli.commands.rules import rules_command from rattle.cli.commands.validate import validate_command -from rattle.cli.environment import ( - _project_rattle_for_reexec, - _reexec_project_rattle, - _version, -) +from rattle.cli.environment import _version from rattle.cli.options import usage_error from rattle.config import CollectionError, ConfigError @@ -130,9 +126,6 @@ def main(args: list[str] | None = None) -> object: """Run the rattle CLI.""" if args is None: args = sys.argv[1:] - project_rattle = _project_rattle_for_reexec(args) - if project_rattle is not None: - _reexec_project_rattle(project_rattle, args) return build_app().run(args=_coalesce_repeated_list_options(args)) diff --git a/src/rattle/cli/environment.py b/src/rattle/cli/environment.py index 1d534f2..4a56127 100644 --- a/src/rattle/cli/environment.py +++ b/src/rattle/cli/environment.py @@ -3,17 +3,11 @@ import logging import os import sys -from pathlib import Path from rattle.__version__ import __version__ -from rattle.pyproject import TOMLDecodeError, load_pyproject -UV_REEXEC_ENV = "RATTLE_UV_RUN_REEXEC" -UV_REEXEC_DISABLE_ENV = "RATTLE_NO_UV_RUN_REEXEC" DEBUG_ENV = "RATTLE_DEBUG" METRICS_ENV = "RATTLE_METRICS" -UV_PROJECT_MARKERS = ("uv.lock",) -UV_REEXEC_COMMANDS = frozenset({"check", "rules", "validate", "lsp", "explain"}) TRUTHY_ENV_VALUES = frozenset({"1", "true", "yes", "on"}) @@ -28,67 +22,3 @@ def _env_flag(name: str) -> bool: def _configure_logging() -> None: level = logging.DEBUG if _env_flag(DEBUG_ENV) else logging.WARNING logging.basicConfig(level=level, stream=sys.stderr, format="%(levelname)s: %(message)s") - - -def _find_uv_project_root(path: Path) -> Path | None: - for directory in (path, *path.parents): - if any((directory / marker).is_file() for marker in UV_PROJECT_MARKERS): - return directory - - pyproject = directory / "pyproject.toml" - if not pyproject.is_file(): - continue - - try: - data = load_pyproject(pyproject) - except (OSError, UnicodeDecodeError, TOMLDecodeError): - continue - - tool = data.get("tool", {}) - if isinstance(tool, dict) and "uv" in tool: - return directory - - dependency_groups = data.get("dependency-groups") - if isinstance(dependency_groups, dict): - return directory - - return None - - -def _project_environment(root: Path) -> Path: - configured = os.environ.get("UV_PROJECT_ENVIRONMENT") - if not configured: - return root / ".venv" - - environment = Path(configured) - return environment if environment.is_absolute() else root / environment - - -def _project_rattle(root: Path) -> Path | None: - environment = _project_environment(root) - if environment.resolve() == Path(sys.prefix).resolve(): - return None - - executable = environment / ("Scripts/rattle.exe" if os.name == "nt" else "bin/rattle") - return executable if executable.is_file() else None - - -def _project_rattle_for_reexec(args: list[str]) -> Path | None: - if os.environ.get(UV_REEXEC_ENV) or os.environ.get(UV_REEXEC_DISABLE_ENV): - return None - - command = next((arg for arg in args if not arg.startswith("-")), None) - if command not in UV_REEXEC_COMMANDS: - return None - - root = _find_uv_project_root(Path.cwd()) - if root is None: - return None - - return _project_rattle(root) - - -def _reexec_project_rattle(executable: Path, args: list[str]) -> None: - env = os.environ.copy() - env[UV_REEXEC_ENV] = "1" - os.execve(executable, [str(executable), *args], env) # noqa: S606 diff --git a/src/rattle/rule_loading.py b/src/rattle/rule_loading.py index df4aeb1..627487a 100644 --- a/src/rattle/rule_loading.py +++ b/src/rattle/rule_loading.py @@ -238,6 +238,35 @@ def local_rule_loader(rule: QualifiedRule) -> Iterator[None]: sys.modules.pop(key, None) +def _rule_import_error(rule: QualifiedRule, error: ImportError) -> CollectionError: + module = f"{RATTLE_LOCAL_MODULE}{rule.module}" if rule.local else rule.module + if isinstance(error, ModuleNotFoundError) and error.name: + if module == error.name or module.startswith(f"{error.name}."): + reason = f"Rule module {rule.module!r} was not found." + action = "Check the configured rule selector and module location." + else: + missing = error.name + if missing.startswith(f"{RATTLE_LOCAL_MODULE}."): + missing = missing.removeprefix(RATTLE_LOCAL_MODULE) + action = "Check the rule's local import paths." + else: + action = ( + "Make the rule's dependencies available in this Python environment, " + "or run Rattle in the environment containing them." + ) + reason = f"Dependency module {missing!r} was not found while importing {rule.module!r}." + else: + reason = f"{type(error).__name__} while importing {rule.module!r}." + action = "Check the rule's imports and compatibility in this Python environment." + + location = f" from {rule.root}" if rule.local else "" + return CollectionError( + f"could not import rule(s) {rule}{location}\n" + f"{reason}\nPython interpreter: {sys.executable}\n{action}", + rule, + ) + + def find_rules(rule: QualifiedRule) -> Iterable[type[LintRule]]: """ Import the rule's qualified module name and return a list of collected rule classes. @@ -270,10 +299,7 @@ def find_rules(rule: QualifiedRule) -> Iterable[type[LintRule]]: yield module_rules[name] except ImportError as e: - if rule.local: - raise CollectionError(f"could not import rule(s) {rule} from {rule.root}", rule) from e - else: - raise CollectionError(f"could not import rule(s) {rule}", rule) from e + raise _rule_import_error(rule, e) from e def walk_module(module: ModuleType) -> dict[str, type[LintRule]]: diff --git a/tests/config/test_rule_options.py b/tests/config/test_rule_options.py index d2a40e5..3fcdd95 100644 --- a/tests/config/test_rule_options.py +++ b/tests/config/test_rule_options.py @@ -3,6 +3,7 @@ # 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 pathlib import Path from tempfile import TemporaryDirectory from textwrap import dedent @@ -443,3 +444,91 @@ def test_builtin_collections_resolve_rules() -> None: ]: rules = collect_rules(Config(enable=[QualifiedRule(collection)], python_version=None)) assert len(rules) > 0, f"Collection {collection} resolved 0 rules" + + +@pytest.mark.parametrize( + ("module", "source", "reason", "cause_type"), + [ + ( + ".rules", + None, + "Rule module '.rules' was not found", + ModuleNotFoundError, + ), + ( + "missing_rule_collection", + None, + "Rule module 'missing_rule_collection' was not found", + ModuleNotFoundError, + ), + ( + "missing_rule_collection.rules", + None, + "Rule module 'missing_rule_collection.rules' was not found", + ModuleNotFoundError, + ), + ( + ".rules", + "import missing_rule_dependency\n", + "Dependency module 'missing_rule_dependency' was not found", + ModuleNotFoundError, + ), + ( + ".rules", + "from .missing_support import VALUE\n", + "Dependency module '.missing_support' was not found", + ModuleNotFoundError, + ), + ( + ".rules", + 'raise ImportError("private import detail")\n', + "ImportError while importing '.rules'", + ImportError, + ), + ], +) +def test_rule_import_errors_explain_failure_and_preserve_cause( + tmp_path: Path, + module: str, + source: str | None, + reason: str, + cause_type: type[ImportError], +) -> None: + if source is not None: + (tmp_path / "rules.py").write_text(source, encoding="utf-8") + rule = QualifiedRule(module, local="." if module.startswith(".") else None, root=tmp_path) + + with pytest.raises(config.CollectionError) as caught: + list(config.find_rules(rule)) + + error = caught.value + assert error.rule == rule + assert reason in str(error) + assert f"Python interpreter: {sys.executable}" in str(error) + assert type(error.__cause__) is cause_type + assert "private import detail" not in str(error) + if cause_type is ImportError: + assert error.__cause__.args == ("private import detail",) + assert "Dependency module" not in str(error) + + +@pytest.mark.parametrize("strict", [False, True], ids=["non-strict", "strict"]) +def test_rule_import_errors_preserve_registry_loading_policy( + tmp_path: Path, caplog: pytest.LogCaptureFixture, strict: bool +) -> None: + (tmp_path / "rules.py").write_text("import missing_rule_dependency\n", encoding="utf-8") + rule = QualifiedRule(".rules", local=".", root=tmp_path) + + if strict: + with pytest.raises(config.CollectionError) as caught: + config.RuleRegistry.load([rule], root=tmp_path, strict=True) + assert caplog.records == [] + else: + registry = config.RuleRegistry.load([rule], root=tmp_path, strict=False) + with pytest.raises(config.CollectionError) as caught: + registry.resolve(rule) + assert "Dependency module 'missing_rule_dependency' was not found" in caplog.text + assert registry.imported_rules[rule] == () + + assert "Dependency module 'missing_rule_dependency' was not found" in str(caught.value) + assert isinstance(caught.value.__cause__, ModuleNotFoundError) diff --git a/tests/test_cli_environment.py b/tests/test_cli_environment.py index ffa7c4e..3a12d45 100644 --- a/tests/test_cli_environment.py +++ b/tests/test_cli_environment.py @@ -1,74 +1,130 @@ from __future__ import annotations +import json import os +import shutil +import subprocess import sys from pathlib import Path import pytest -from rattle.cli.environment import _project_rattle_for_reexec -EXECUTABLE = Path("Scripts/rattle.exe") if os.name == "nt" else Path("bin/rattle") - - -@pytest.fixture -def project(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: - (tmp_path / "uv.lock").write_text("version = 1\n", encoding="utf-8") - monkeypatch.chdir(tmp_path) - monkeypatch.delenv("UV_PROJECT_ENVIRONMENT", raising=False) - monkeypatch.delenv("RATTLE_UV_RUN_REEXEC", raising=False) - monkeypatch.delenv("RATTLE_NO_UV_RUN_REEXEC", raising=False) - monkeypatch.setattr(sys, "prefix", str(tmp_path / "global-tool")) - return tmp_path - - -def install_rattle(environment: Path) -> Path: - executable = environment / EXECUTABLE - executable.parent.mkdir(parents=True) - executable.write_text("", encoding="utf-8") - return executable - - -@pytest.mark.parametrize("options", [[], ["--fix"]]) -def test_hands_off_to_rattle_installed_in_project_environment( - project: Path, options: list[str] -) -> None: - executable = install_rattle(project / ".venv") - - assert _project_rattle_for_reexec(["check", *options, "."]) == executable - - -def test_stays_in_process_when_already_running_in_project_environment( - project: Path, monkeypatch: pytest.MonkeyPatch +@pytest.mark.parametrize( + ("marker", "configured"), + [ + pytest.param("lock", None, id="uv-lock"), + pytest.param("tool", None, id="uv-tool"), + pytest.param("groups", None, id="dependency-groups"), + pytest.param("lock", "envs/runtime", id="relative-environment"), + pytest.param("lock", "absolute", id="absolute-environment"), + ], +) +def test_check_uses_invoked_installation_despite_project_environment( + tmp_path: Path, marker: str, configured: str | None ) -> None: - install_rattle(project / ".venv") - monkeypatch.setattr(sys, "prefix", str(project / ".venv")) - - assert _project_rattle_for_reexec(["check", "."]) is None - + config = '[tool.rattle]\nroot=true\nenable=["no-redundant-f-string"]\n' + if marker == "lock": + (tmp_path / "uv.lock").write_text("version = 1\n", encoding="utf-8") + elif marker == "tool": + config += "[tool.uv]\n" + else: + config += "[dependency-groups]\ntest=[]\n" + (tmp_path / "pyproject.toml").write_text(config, encoding="utf-8") + path = tmp_path / "sample.py" + source = 'value = f"text"\n' + path.write_text(source, encoding="utf-8") + environment = os.environ | {"RATTLE_DISABLE_CACHE": "1"} + environment.pop("UV_PROJECT_ENVIRONMENT", None) + if configured == "absolute": + project_environment = tmp_path / "runtime" + environment["UV_PROJECT_ENVIRONMENT"] = str(project_environment) + elif configured is not None: + project_environment = tmp_path / configured + environment["UV_PROJECT_ENVIRONMENT"] = configured + else: + project_environment = tmp_path / ".venv" + executable = project_environment / ("Scripts/rattle.exe" if os.name == "nt" else "bin/rattle") + executable.parent.mkdir(parents=True) + shutil.copy2(Path(sys.executable).resolve(), executable) + + result = subprocess.run( + [sys.executable, "-m", "rattle", "check", "--json", path.as_posix()], + cwd=tmp_path, + env=environment, + capture_output=True, + text=True, + check=False, + ) -def test_stays_in_process_when_project_environment_lacks_rattle(project: Path) -> None: - (project / ".venv").mkdir() + assert result.returncode == 1, result.stderr + assert [diagnostic["rule"] for diagnostic in json.loads(result.stdout)] == [ + "no-redundant-f-string" + ] + assert path.read_text(encoding="utf-8") == source - assert _project_rattle_for_reexec(["check", "."]) is None +def test_local_rules_import_dependencies_in_invoked_interpreter(tmp_path: Path) -> None: + (tmp_path / "pyproject.toml").write_text( + '[tool.rattle]\nroot=true\nenable=[".rules"]\n', encoding="utf-8" + ) + (tmp_path / "settings.py").write_text('MESSAGE = "Use another name"\n', encoding="utf-8") + (tmp_path / "rules.py").write_text( + "import sys\n" + "from rattle import LintRule\n" + "from .settings import MESSAGE\n" + "class NameCheck(LintRule):\n" + " def visit_Name(self, node):\n" + " if node.value == 'blocked':\n" + " self.report(node, f'{MESSAGE}: {sys.executable}')\n", + encoding="utf-8", + ) + path = tmp_path / "sample.py" + path.write_text("blocked = 0\n", encoding="utf-8") + + result = subprocess.run( + [sys.executable, "-m", "rattle", "check", "--json", path.as_posix()], + cwd=tmp_path, + env=os.environ | {"RATTLE_DISABLE_CACHE": "1"}, + capture_output=True, + text=True, + check=False, + ) -@pytest.mark.parametrize("configured", ["envs/ci", "absolute"]) -def test_uses_configured_project_environment( - project: Path, monkeypatch: pytest.MonkeyPatch, configured: str + assert result.returncode == 1, result.stderr + assert [diagnostic["message"] for diagnostic in json.loads(result.stdout)] == [ + f"Use another name: {sys.executable}" + ] + + +@pytest.mark.parametrize( + ("arguments", "expected_exit"), + [ + pytest.param(["check", "--rules", ".rules", "sample.py"], 2, id="explicit-rule"), + pytest.param(["validate", "pyproject.toml"], 1, id="validation"), + pytest.param(["rules"], 0, id="non-strict-listing"), + ], +) +def test_rule_dependency_errors_identify_invoked_interpreter( + tmp_path: Path, arguments: list[str], expected_exit: int ) -> None: - environment = project / "envs" / "ci" if configured == "envs/ci" else project / "elsewhere" - monkeypatch.setenv( - "UV_PROJECT_ENVIRONMENT", configured if configured == "envs/ci" else str(environment) + (tmp_path / "pyproject.toml").write_text( + '[tool.rattle]\nroot=true\nenable=[".rules"]\n', encoding="utf-8" + ) + (tmp_path / "rules.py").write_text("import missing_rule_dependency\n", encoding="utf-8") + (tmp_path / "sample.py").write_text("value = 0\n", encoding="utf-8") + + result = subprocess.run( + [sys.executable, "-m", "rattle", *arguments], + cwd=tmp_path, + env=os.environ | {"RATTLE_DISABLE_CACHE": "1"}, + capture_output=True, + text=True, + check=False, ) - executable = install_rattle(environment) - install_rattle(project / ".venv") - - assert _project_rattle_for_reexec(["check", "."]) == executable - - -def test_stays_in_process_after_handoff(project: Path, monkeypatch: pytest.MonkeyPatch) -> None: - install_rattle(project / ".venv") - monkeypatch.setenv("RATTLE_UV_RUN_REEXEC", "1") - assert _project_rattle_for_reexec(["check", "."]) is None + assert result.returncode == expected_exit, result.stderr + assert "Dependency module 'missing_rule_dependency' was not found" in result.stderr + assert f"Python interpreter: {sys.executable}" in result.stderr + assert "dependencies" in result.stderr + assert "environment" in result.stderr