Add a fix goal (pantsbuild#17202)

This PR adds a `fix` goal, whose implementation is **literally** a copy-and-rename of `fmt`, along with the requisite `lint` changes. This, of course, isn't the long-term solution, but we need to discuss the relationship between the two, which informs how to implement all the "helper"/shared code. Also moved `autoflake` and `pyupgrade` to be fixers. Future PRs will clean up the duplication, and move `BUILD` file fixing from `update-build-files` here. Tested by "moving" `autoflake`, and running a few commans (`fix`, `lint`). Docs change(s) can come later, aside from what's in this PR (if anything). [ci skip-rust] [ci skip-build-wheels]
vkleen · Oct 13, 2022 · c455a03 · c455a03
1 parent c2166c5
commit c455a03
Show file tree

Hide file tree

Showing 15 changed files with 1,159 additions and 78 deletions.
diff --git a/pants.toml b/pants.toml
@@ -6,9 +6,9 @@ pythonpath = ["%(buildroot)s/pants-plugins"]
 backend_packages.add = [
   "pants.backend.build_files.fmt.black",
   "pants.backend.python",
-  "pants.backend.experimental.python.lint.autoflake",
   "pants.backend.experimental.python.packaging.pyoxidizer",
   "pants.backend.explorer",
+  "pants.backend.python.lint.autoflake",
   "pants.backend.python.lint.black",
   "pants.backend.python.lint.docformatter",
   "pants.backend.python.lint.flake8",

diff --git a/src/python/pants/backend/experimental/python/lint/autoflake/register.py b/src/python/pants/backend/experimental/python/lint/autoflake/register.py
@@ -1,14 +1,16 @@
 # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-"""Autoformatter for removing unused Python imports.
+import logging
 
-See https://github.com/myint/autoflake for details.
-"""
+from pants.backend.python.lint.autoflake.register import rules as autoflake_rules
 
-from pants.backend.python.lint.autoflake import rules as autoflake_rules
-from pants.backend.python.lint.autoflake import skip_field, subsystem
+logger = logging.getLogger(__name__)
 
 
 def rules():
-    return (*autoflake_rules.rules(), *skip_field.rules(), *subsystem.rules())
+    logger.warning(
+        "DEPRECATED: The autoflake plugin has moved to `pants.backend.python.lint.autoflake`"
+        + " (and from the `fmt` goal to the `fix` goal)."
+    )
+    return autoflake_rules()
diff --git a/src/python/pants/backend/experimental/python/lint/pyupgrade/register.py b/src/python/pants/backend/experimental/python/lint/pyupgrade/register.py
@@ -1,14 +1,17 @@
 # Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-"""https://github.com/asottile/pyupgrade.
 
-A tool  to automatically upgrade syntax for newer versions of the language.
-"""
+import logging
 
-from pants.backend.python.lint.pyupgrade import rules as pyupgrade_rules
-from pants.backend.python.lint.pyupgrade import skip_field, subsystem
+from pants.backend.python.lint.pyupgrade.register import rules as pyupgrade_rules
+
+logger = logging.getLogger(__name__)
 
 
 def rules():
-    return (*pyupgrade_rules.rules(), *skip_field.rules(), *subsystem.rules())
+    logger.warning(
+        "DEPRECATED: The pyupgrade plugin has moved to `pants.backend.python.lint.pyupgrade`"
+        + " (and from the `fmt` goal to the `fix` goal)."
+    )
+    return pyupgrade_rules()
diff --git a/src/python/pants/backend/python/lint/autoflake/register.py b/src/python/pants/backend/python/lint/autoflake/register.py
@@ -0,0 +1,14 @@
+# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+"""Autoformatter for removing unused Python imports.
+
+See https://github.com/myint/autoflake for details.
+"""
+
+from pants.backend.python.lint.autoflake import rules as autoflake_rules
+from pants.backend.python.lint.autoflake import skip_field, subsystem
+
+
+def rules():
+    return (*autoflake_rules.rules(), *skip_field.rules(), *subsystem.rules())
diff --git a/src/python/pants/backend/python/lint/autoflake/rules.py b/src/python/pants/backend/python/lint/autoflake/rules.py
@@ -8,7 +8,7 @@
 from pants.backend.python.target_types import PythonSourceField
 from pants.backend.python.util_rules import pex
 from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess
-from pants.core.goals.fmt import FmtResult, FmtTargetsRequest
+from pants.core.goals.fix import FixResult, FixTargetsRequest
 from pants.core.util_rules.partitions import PartitionerType
 from pants.engine.process import ProcessResult
 from pants.engine.rules import Get, collect_rules, rule
@@ -28,14 +28,14 @@ def opt_out(cls, tgt: Target) -> bool:
         return tgt.get(SkipAutoflakeField).value
 
 
-class AutoflakeRequest(FmtTargetsRequest):
+class AutoflakeRequest(FixTargetsRequest):
     field_set_type = AutoflakeFieldSet
     tool_subsystem = Autoflake
     partitioner_type = PartitionerType.DEFAULT_SINGLE_PARTITION
 
 
-@rule(desc="Format with Autoflake", level=LogLevel.DEBUG)
-async def autoflake_fmt(request: AutoflakeRequest.SubPartition, autoflake: Autoflake) -> FmtResult:
+@rule(desc="Fix with Autoflake", level=LogLevel.DEBUG)
+async def autoflake_fix(request: AutoflakeRequest.SubPartition, autoflake: Autoflake) -> FixResult:
     autoflake_pex = await Get(VenvPex, PexRequest, autoflake.to_pex_request())
 
     result = await Get(
@@ -53,7 +53,7 @@ async def autoflake_fmt(request: AutoflakeRequest.SubPartition, autoflake: Autof
             level=LogLevel.DEBUG,
         ),
     )
-    return await FmtResult.create(request, result, strip_chroot_path=True)
+    return await FixResult.create(request, result, strip_chroot_path=True)
 
 
 def rules():

diff --git a/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py b/src/python/pants/backend/python/lint/autoflake/rules_integration_test.py
@@ -11,7 +11,7 @@
 from pants.backend.python.lint.autoflake.subsystem import Autoflake
 from pants.backend.python.lint.autoflake.subsystem import rules as autoflake_subsystem_rules
 from pants.backend.python.target_types import PythonSourcesGeneratorTarget
-from pants.core.goals.fmt import FmtResult
+from pants.core.goals.fix import FixResult
 from pants.core.util_rules import config_files, source_files
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Address
@@ -31,7 +31,7 @@ def rule_runner() -> RuleRunner:
             *source_files.rules(),
             *config_files.rules(),
             *target_types_rules.rules(),
-            QueryRule(FmtResult, (AutoflakeRequest.SubPartition,)),
+            QueryRule(FixResult, (AutoflakeRequest.SubPartition,)),
             QueryRule(SourceFiles, (SourceFilesRequest,)),
         ],
         target_types=[PythonSourcesGeneratorTarget],
@@ -48,7 +48,7 @@ def run_autoflake(
     targets: list[Target],
     *,
     extra_args: list[str] | None = None,
-) -> FmtResult:
+) -> FixResult:
     rule_runner.set_options(
         ["--backend-packages=pants.backend.python.lint.autoflake", *(extra_args or ())],
         env_inherit={"PATH", "PYENV_ROOT", "HOME"},
@@ -60,15 +60,15 @@ def run_autoflake(
             SourceFilesRequest(field_set.source for field_set in field_sets),
         ],
     )
-    fmt_result = rule_runner.request(
-        FmtResult,
+    fix_result = rule_runner.request(
+        FixResult,
         [
             AutoflakeRequest.SubPartition(
                 "", input_sources.snapshot.files, key=None, snapshot=input_sources.snapshot
             ),
         ],
     )
-    return fmt_result
+    return fix_result
 
 
 def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot:
@@ -85,22 +85,22 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps
 def test_passing_source(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
     rule_runner.write_files({"f.py": GOOD_FILE, "BUILD": "python_sources(name='t')"})
     tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
-    fmt_result = run_autoflake(
+    fix_result = run_autoflake(
         rule_runner,
         [tgt],
         extra_args=[f"--autoflake-interpreter-constraints=['=={major_minor_interpreter}.*']"],
     )
-    assert fmt_result.stdout == ""
-    assert fmt_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE})
-    assert fmt_result.did_change is False
+    assert fix_result.stdout == ""
+    assert fix_result.output == get_snapshot(rule_runner, {"f.py": GOOD_FILE})
+    assert fix_result.did_change is False
 
 
 def test_failing_source(rule_runner: RuleRunner) -> None:
     rule_runner.write_files({"f.py": BAD_FILE, "BUILD": "python_sources(name='t')"})
     tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
-    fmt_result = run_autoflake(rule_runner, [tgt])
-    assert fmt_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE})
-    assert fmt_result.did_change is True
+    fix_result = run_autoflake(rule_runner, [tgt])
+    assert fix_result.output == get_snapshot(rule_runner, {"f.py": FIXED_BAD_FILE})
+    assert fix_result.did_change is True
 
 
 def test_multiple_targets(rule_runner: RuleRunner) -> None:
@@ -111,11 +111,11 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None:
         rule_runner.get_target(Address("", target_name="t", relative_file_path="good.py")),
         rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.py")),
     ]
-    fmt_result = run_autoflake(rule_runner, tgts)
-    assert fmt_result.output == get_snapshot(
+    fix_result = run_autoflake(rule_runner, tgts)
+    assert fix_result.output == get_snapshot(
         rule_runner, {"good.py": GOOD_FILE, "bad.py": FIXED_BAD_FILE}
     )
-    assert fmt_result.did_change is True
+    assert fix_result.did_change is True
 
 
 def test_stub_files(rule_runner: RuleRunner) -> None:
@@ -133,19 +133,19 @@ def test_stub_files(rule_runner: RuleRunner) -> None:
         rule_runner.get_target(Address("", target_name="t", relative_file_path="good.pyi")),
         rule_runner.get_target(Address("", target_name="t", relative_file_path="good.py")),
     ]
-    fmt_result = run_autoflake(rule_runner, good_tgts)
-    assert fmt_result.stdout == ""
-    assert fmt_result.output == get_snapshot(
+    fix_result = run_autoflake(rule_runner, good_tgts)
+    assert fix_result.stdout == ""
+    assert fix_result.output == get_snapshot(
         rule_runner, {"good.py": GOOD_FILE, "good.pyi": GOOD_FILE}
     )
-    assert not fmt_result.did_change
+    assert not fix_result.did_change
 
     bad_tgts = [
         rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.pyi")),
         rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.py")),
     ]
-    fmt_result = run_autoflake(rule_runner, bad_tgts)
-    assert fmt_result.output == get_snapshot(
+    fix_result = run_autoflake(rule_runner, bad_tgts)
+    assert fix_result.output == get_snapshot(
         rule_runner, {"bad.py": FIXED_BAD_FILE, "bad.pyi": FIXED_BAD_FILE}
     )
-    assert fmt_result.did_change
+    assert fix_result.did_change
diff --git a/src/python/pants/backend/python/lint/pyupgrade/register.py b/src/python/pants/backend/python/lint/pyupgrade/register.py
@@ -0,0 +1,14 @@
+# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+"""https://github.com/asottile/pyupgrade.
+
+A tool  to automatically upgrade syntax for newer versions of the language.
+"""
+
+from pants.backend.python.lint.pyupgrade import rules as pyupgrade_rules
+from pants.backend.python.lint.pyupgrade import skip_field, subsystem
+
+
+def rules():
+    return (*pyupgrade_rules.rules(), *skip_field.rules(), *subsystem.rules())
diff --git a/src/python/pants/backend/python/lint/pyupgrade/rules.py b/src/python/pants/backend/python/lint/pyupgrade/rules.py
@@ -9,7 +9,7 @@
 from pants.backend.python.target_types import PythonSourceField
 from pants.backend.python.util_rules import pex
 from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess
-from pants.core.goals.fmt import FmtResult, FmtTargetsRequest
+from pants.core.goals.fix import FixResult, FixTargetsRequest
 from pants.core.util_rules.partitions import PartitionerType
 from pants.engine.process import FallibleProcessResult
 from pants.engine.rules import Get, collect_rules, rule
@@ -29,14 +29,14 @@ def opt_out(cls, tgt: Target) -> bool:
         return tgt.get(SkipPyUpgradeField).value
 
 
-class PyUpgradeRequest(FmtTargetsRequest):
+class PyUpgradeRequest(FixTargetsRequest):
     field_set_type = PyUpgradeFieldSet
     tool_subsystem = PyUpgrade
     partitioner_type = PartitionerType.DEFAULT_SINGLE_PARTITION
 
 
-@rule(desc="Format with pyupgrade", level=LogLevel.DEBUG)
-async def pyupgrade_fmt(request: PyUpgradeRequest.SubPartition, pyupgrade: PyUpgrade) -> FmtResult:
+@rule(desc="Fix with pyupgrade", level=LogLevel.DEBUG)
+async def pyupgrade_fix(request: PyUpgradeRequest.SubPartition, pyupgrade: PyUpgrade) -> FixResult:
     pyupgrade_pex = await Get(VenvPex, PexRequest, pyupgrade.to_pex_request())
 
     result = await Get(
@@ -50,7 +50,7 @@ async def pyupgrade_fmt(request: PyUpgradeRequest.SubPartition, pyupgrade: PyUpg
             level=LogLevel.DEBUG,
         ),
     )
-    return await FmtResult.create(request, result)
+    return await FixResult.create(request, result)
 
 
 def rules():

diff --git a/src/python/pants/backend/python/lint/pyupgrade/rules_integration_test.py b/src/python/pants/backend/python/lint/pyupgrade/rules_integration_test.py
@@ -11,7 +11,7 @@
 from pants.backend.python.lint.pyupgrade.subsystem import PyUpgrade
 from pants.backend.python.lint.pyupgrade.subsystem import rules as pyupgrade_subsystem_rules
 from pants.backend.python.target_types import PythonSourcesGeneratorTarget
-from pants.core.goals.fmt import FmtResult
+from pants.core.goals.fix import FixResult
 from pants.core.util_rules import config_files, source_files
 from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest
 from pants.engine.addresses import Address
@@ -31,7 +31,7 @@ def rule_runner() -> RuleRunner:
             *source_files.rules(),
             *config_files.rules(),
             *target_types_rules.rules(),
-            QueryRule(FmtResult, (PyUpgradeRequest.SubPartition,)),
+            QueryRule(FixResult, (PyUpgradeRequest.SubPartition,)),
             QueryRule(SourceFiles, (SourceFilesRequest,)),
         ],
         target_types=[PythonSourcesGeneratorTarget],
@@ -54,7 +54,7 @@ def run_pyupgrade(
     *,
     extra_args: list[str] | None = None,
     pyupgrade_arg: str = "--py36-plus",
-) -> FmtResult:
+) -> FixResult:
     rule_runner.set_options(
         [
             "--backend-packages=pants.backend.python.lint.pyupgrade",
@@ -70,15 +70,15 @@ def run_pyupgrade(
             SourceFilesRequest(field_set.source for field_set in field_sets),
         ],
     )
-    fmt_result = rule_runner.request(
-        FmtResult,
+    fix_result = rule_runner.request(
+        FixResult,
         [
             PyUpgradeRequest.SubPartition(
                 "", input_sources.snapshot.files, key=None, snapshot=input_sources.snapshot
             ),
         ],
     )
-    return fmt_result
+    return fix_result
 
 
 def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snapshot:
@@ -95,21 +95,21 @@ def get_snapshot(rule_runner: RuleRunner, source_files: dict[str, str]) -> Snaps
 def test_passing(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
     rule_runner.write_files({"f.py": PY_36_GOOD_FILE, "BUILD": "python_sources(name='t')"})
     tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
-    fmt_result = run_pyupgrade(
+    fix_result = run_pyupgrade(
         rule_runner,
         [tgt],
         extra_args=[f"--pyupgrade-interpreter-constraints=['=={major_minor_interpreter}.*']"],
     )
-    assert fmt_result.output == get_snapshot(rule_runner, {"f.py": PY_36_GOOD_FILE})
-    assert fmt_result.did_change is False
+    assert fix_result.output == get_snapshot(rule_runner, {"f.py": PY_36_GOOD_FILE})
+    assert fix_result.did_change is False
 
 
 def test_failing(rule_runner: RuleRunner) -> None:
     rule_runner.write_files({"f.py": PY_36_BAD_FILE, "BUILD": "python_sources(name='t')"})
     tgt = rule_runner.get_target(Address("", target_name="t", relative_file_path="f.py"))
-    fmt_result = run_pyupgrade(rule_runner, [tgt])
-    assert fmt_result.output == get_snapshot(rule_runner, {"f.py": PY_36_FIXED_BAD_FILE})
-    assert fmt_result.did_change is True
+    fix_result = run_pyupgrade(rule_runner, [tgt])
+    assert fix_result.output == get_snapshot(rule_runner, {"f.py": PY_36_FIXED_BAD_FILE})
+    assert fix_result.did_change is True
 
 
 def test_multiple_targets(rule_runner: RuleRunner) -> None:
@@ -120,11 +120,11 @@ def test_multiple_targets(rule_runner: RuleRunner) -> None:
         rule_runner.get_target(Address("", target_name="t", relative_file_path="good.py")),
         rule_runner.get_target(Address("", target_name="t", relative_file_path="bad.py")),
     ]
-    fmt_result = run_pyupgrade(rule_runner, tgts)
-    assert fmt_result.output == get_snapshot(
+    fix_result = run_pyupgrade(rule_runner, tgts)
+    assert fix_result.output == get_snapshot(
         rule_runner, {"good.py": PY_36_GOOD_FILE, "bad.py": PY_36_FIXED_BAD_FILE}
     )
-    assert fmt_result.did_change is True
+    assert fix_result.did_change is True
 
 
 def test_passthrough_args(rule_runner: RuleRunner) -> None:
@@ -134,12 +134,12 @@ def test_passthrough_args(rule_runner: RuleRunner) -> None:
     tgt = rule_runner.get_target(
         Address("", target_name="t", relative_file_path="some_file_name.py")
     )
-    fmt_result = run_pyupgrade(
+    fix_result = run_pyupgrade(
         rule_runner,
         [tgt],
         pyupgrade_arg="--py38-plus",
     )
-    assert fmt_result.output == get_snapshot(
+    assert fix_result.output == get_snapshot(
         rule_runner, {"some_file_name.py": PY_38_FIXED_BAD_FILE}
     )
-    assert fmt_result.did_change is True
+    assert fix_result.did_change is True