From 83497c7daed69ba8e5eba5894d8a83853875abff Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Fri, 8 May 2026 09:56:35 -0700 Subject: [PATCH] Fix subsystem mypy plugin to use get_base_class_hook instead of get_additional_deps The previous implementation piggybacked on get_additional_deps to modify abstract_status on FuncDef nodes for @option-decorated methods. This hook fires during import resolution, before mypy's semantic analysis passes run. Because mypy's semantic analyzer processes FuncDef nodes after this hook, any modifications made there do not persist into the type-checking phase, so the [empty-body] errors were never suppressed. Switch to get_base_class_hook, which fires during semantic analysis (after the class's own semanal pass), so setting abstract_status = IS_ABSTRACT on the underlying FuncDef sticks when the type checker runs. Since get_base_class_hook matches only direct base classes, enumerate the known levels of the SubsystemNg hierarchy explicitly: SubsystemNg itself, UniversalSubsystem, ContextualSubsystem, and GoalSubsystemNg. Any new intermediate base class added to the hierarchy will need to be listed here. --- src/python/pants/ng/subsystem_mypy_plugin.py | 28 ++++-- .../pants/ng/subsystem_mypy_plugin_test.py | 91 +++++++++++++++++++ 2 files changed, 112 insertions(+), 7 deletions(-) create mode 100644 src/python/pants/ng/subsystem_mypy_plugin_test.py diff --git a/src/python/pants/ng/subsystem_mypy_plugin.py b/src/python/pants/ng/subsystem_mypy_plugin.py index dfd9ea6a32d..e36a01dfc33 100644 --- a/src/python/pants/ng/subsystem_mypy_plugin.py +++ b/src/python/pants/ng/subsystem_mypy_plugin.py @@ -11,10 +11,9 @@ ClassDef, Decorator, FuncDef, - MypyFile, Statement, ) -from mypy.plugin import Plugin +from mypy.plugin import ClassDefContext, Plugin def _has_option_decorator(node: Decorator) -> bool: @@ -42,13 +41,28 @@ def _process_defns(defns: list[Statement]) -> None: _process_defns(defn.func.body.body) +def _process_subclass_hook(ctx: ClassDefContext) -> None: + _process_defns(ctx.cls.defs.body) + + +# NB: If we create more intermediate subclasses, they must be added here. +_SUBSYSTEM_BASE_CLASSES = frozenset( + { + "pants.ng.subsystem.SubsystemNg", + "pants.ng.subsystem.UniversalSubsystem", + "pants.ng.subsystem.ContextualSubsystem", + "pants.ng.goal.GoalSubsystemNg", + } +) + + class SubsystemPlugin(Plugin): - """Mypy plugin that processes files to find methods decorated with @option.""" + """Mypy plugin that processes SubsystemNg subclasses to find methods decorated with @option.""" - def get_additional_deps(self, file: MypyFile) -> list[tuple[int, str, int]]: - """A file-level hook that we piggyback on to process the parsed file.""" - _process_defns(file.defs) - return [] + def get_base_class_hook(self, fullname: str): + if fullname in _SUBSYSTEM_BASE_CLASSES: + return _process_subclass_hook + return None def plugin(version: str) -> Type[Plugin]: diff --git a/src/python/pants/ng/subsystem_mypy_plugin_test.py b/src/python/pants/ng/subsystem_mypy_plugin_test.py new file mode 100644 index 00000000000..001e2ec236d --- /dev/null +++ b/src/python/pants/ng/subsystem_mypy_plugin_test.py @@ -0,0 +1,91 @@ +# Copyright 2026 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). + +import tempfile +import textwrap +from pathlib import Path + +import mypy.api + +from pants.util.contextutil import environment_as + +_FOO_GOAL_SUBSYSTEM_SOURCE = textwrap.dedent("""\ + from pants.ng.goal import GoalSubsystemNg + from pants.ng.subsystem import option + + class FooSubsystem(GoalSubsystemNg): + options_scope = "foo" + help = "Run foo" + + @option(default=0, help="How much to foo") + def level(self) -> int: ... +""") + + +_BAR_SUBSYSTEM_SOURCE = textwrap.dedent("""\ + from pants.ng.subsystem import ContextualSubsystem, option + + class Bar(ContextualSubsystem): + options_scope = "bar" + help = "Options for bar." + + @option(default="A", help="how to bar") + def how_to(self) -> str: ... + + @option(default=false, help="whether to do the bar") + def do_it(self) -> bool: ... +""") + + +def _run_mypy(source: str, *, with_plugin: bool) -> tuple[str, str]: + """Run mypy on the given source string and return stdout.""" + # SubsystemNg and its subclasses depend on various code from the + # codebase, so we must point mypy at it. + import pants + + pants_src = str(Path(pants.__file__).parents[1]) + plugin_path = str(Path(__file__).parent / "subsystem_mypy_plugin.py") + + with ( + tempfile.NamedTemporaryFile(suffix=".py", mode="w") as src_f, + tempfile.NamedTemporaryFile(suffix=".ini", mode="w") as cfg_f, + ): + src_f.write(source) + src_f.flush() + cfg_f.write("[mypy]\n") + cfg_f.flush() + with environment_as(MYPYPATH=pants_src): + args = [ + src_f.name, + "--no-error-summary", + "--no-incremental", + f"--config-file={cfg_f.name}", + ] + if with_plugin: + args.extend(["--plugin", plugin_path]) + stdout, stderr, _code = mypy.api.run(args) + return stdout, stderr + + +def test_plugin_suppresses_empty_body_on_lint_subsystem() -> None: + stdout_noplugin, stderr_noplugin = _run_mypy(_FOO_GOAL_SUBSYSTEM_SOURCE, with_plugin=False) + assert "empty-body" in stdout_noplugin, ( + f"Expected [empty-body] error without plugin:\n{stdout_noplugin} (err: {stderr_noplugin})" + ) + + stdout_plugin, stderr_plugin = _run_mypy(_FOO_GOAL_SUBSYSTEM_SOURCE, with_plugin=True) + assert "empty-body" not in stdout_plugin, ( + f"Plugin did not suppress [empty-body] error:\n{stdout_plugin} (err: {stderr_plugin})" + ) + + +def test_plugin_suppresses_empty_body_on_contextual_subsystem() -> None: + stdout_noplugin, stderr_noplugin = _run_mypy(_BAR_SUBSYSTEM_SOURCE, with_plugin=False) + assert "empty-body" in stdout_noplugin, ( + f"Expected [empty-body] error without plugin:\n{stdout_noplugin} (err: {stderr_noplugin})" + ) + + stdout_plugin, stderr_plugin = _run_mypy(_BAR_SUBSYSTEM_SOURCE, with_plugin=True) + assert "empty-body" not in stdout_plugin, ( + f"Plugin did not suppress [empty-body] error:\n{stdout_plugin} (err: {stderr_plugin})" + )