Support V2 linters running on Python 2 and Python 3 targets at the sa…

…me time (#9870)

### Problem

When using the default `--no-per-target-caching`, `./v2 lint ::` will fail if you have any targets with conflicting Python interpreter constraints, e.g. Python 2-only targets mixed with Python 3-only targets. This is a common problem due to Python 3 migrations.


It is not acceptable to say "just use `--per-target-caching`" because we want `./pants lint ::` to work regardless of this setting.

### Solution

The main change is for linter rules to now return `LintResults`, rather than a single `LintResult`. Typically, this will only have one element, but it can include more, such as partitioning by interpreter constraints. In JVM land, we might partition by JVM compatibility, for example.

Then, Pylint, Bandit, and Flake8 are rewritten to have new rules that take a `Partition` of the original input targets and return a single `LintResult`. Another rule does the partitioning and aggregates these results.

### Result

`./v2 lint ::` works on any Python codebase, regardless of interpreter constraints.

[ci skip-rust-tests]
[ci skip-jvm-tests]

src/python/pants/backend/python/lint/bandit/rules.py

@@ -15,7 +15,7 @@
 from pants.backend.python.subsystems import python_native_code, subprocess_environment
 from pants.backend.python.subsystems.subprocess_environment import SubprocessEncodingEnvironment
 from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources
-from pants.core.goals.lint import LintRequest, LintResult
+from pants.core.goals.lint import LintRequest, LintResult, LintResults
 from pants.core.util_rules import determine_source_files, strip_source_roots
 from pants.core.util_rules.determine_source_files import (
     AllSourceFilesRequest,
@@ -24,7 +24,7 @@
 )
 from pants.engine.fs import Digest, MergeDigests, PathGlobs, Snapshot
 from pants.engine.process import FallibleProcessResult, Process
-from pants.engine.rules import SubsystemRule, named_rule
+from pants.engine.rules import SubsystemRule, named_rule, rule
 from pants.engine.selectors import Get, MultiGet
 from pants.engine.target import FieldSetWithOrigin
 from pants.engine.unions import UnionRule
@@ -45,6 +45,12 @@ class BanditRequest(LintRequest):
     field_set_type = BanditFieldSet
 
 
+@dataclass(frozen=True)
+class BanditPartition:
+    field_sets: Tuple[BanditFieldSet, ...]
+    interpreter_constraints: PexInterpreterConstraints
+
+
 def generate_args(*, specified_source_files: SourceFiles, bandit: Bandit) -> Tuple[str, ...]:
     args = []
     if bandit.options.config is not None:
@@ -54,26 +60,21 @@ def generate_args(*, specified_source_files: SourceFiles, bandit: Bandit) -> Tup
     return tuple(args)
 
 
-@named_rule(desc="Lint using Bandit")
-async def bandit_lint(
-    request: BanditRequest,
+@rule
+async def bandit_lint_partition(
+    partition: BanditPartition,
     bandit: Bandit,
     python_setup: PythonSetup,
     subprocess_encoding_environment: SubprocessEncodingEnvironment,
 ) -> LintResult:
-    if bandit.options.skip:
-        return LintResult.noop()
-
-    # NB: Bandit output depends upon which Python interpreter version it's run with. See
-    # https://github.com/PyCQA/bandit#under-which-version-of-python-should-i-install-bandit.
-    interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
-        (field_set.compatibility for field_set in request.field_sets), python_setup=python_setup
-    ) or PexInterpreterConstraints(bandit.default_interpreter_constraints)
     requirements_pex_request = Get[Pex](
         PexRequest(
             output_filename="bandit.pex",
             requirements=PexRequirements(bandit.get_requirement_specs()),
-            interpreter_constraints=interpreter_constraints,
+            interpreter_constraints=(
+                partition.interpreter_constraints
+                or PexInterpreterConstraints(bandit.default_interpreter_constraints)
+            ),
             entry_point=bandit.get_entry_point(),
         )
     )
@@ -88,11 +89,11 @@ async def bandit_lint(
     )
 
     all_source_files_request = Get[SourceFiles](
-        AllSourceFilesRequest(field_set.sources for field_set in request.field_sets)
+        AllSourceFilesRequest(field_set.sources for field_set in partition.field_sets)
     )
     specified_source_files_request = Get[SourceFiles](
         SpecifiedSourceFilesRequest(
-            (field_set.sources, field_set.origin) for field_set in request.field_sets
+            (field_set.sources, field_set.origin) for field_set in partition.field_sets
         )
     )
 
@@ -115,7 +116,7 @@ async def bandit_lint(
     )
 
     address_references = ", ".join(
-        sorted(field_set.address.reference() for field_set in request.field_sets)
+        sorted(field_set.address.reference() for field_set in partition.field_sets)
     )
 
     process = requirements_pex.create_process(
@@ -124,15 +125,39 @@ async def bandit_lint(
         pex_path="./bandit.pex",
         pex_args=generate_args(specified_source_files=specified_source_files, bandit=bandit),
         input_digest=input_digest,
-        description=f"Run Bandit on {pluralize(len(request.field_sets), 'target')}: {address_references}.",
+        description=(
+            f"Run Bandit on {pluralize(len(partition.field_sets), 'target')}: {address_references}."
+        ),
     )
     result = await Get[FallibleProcessResult](Process, process)
     return LintResult.from_fallible_process_result(result, linter_name="Bandit")
 
 
+@named_rule(desc="Lint using Bandit")
+async def bandit_lint(
+    request: BanditRequest, bandit: Bandit, python_setup: PythonSetup
+) -> LintResults:
+    if bandit.options.skip:
+        return LintResults()
+
+    # NB: Bandit output depends upon which Python interpreter version it's run with
+    # ( https://github.com/PyCQA/bandit#under-which-version-of-python-should-i-install-bandit). We
+    # batch targets by their constraints to ensure, for example, that all Python 2 targets run
+    # together and all Python 3 targets run together.
+    constraints_to_field_sets = PexInterpreterConstraints.group_field_sets_by_constraints(
+        request.field_sets, python_setup
+    )
+    partitioned_results = await MultiGet(
+        Get[LintResult](BanditPartition(partition_field_sets, partition_compatibility))
+        for partition_compatibility, partition_field_sets in constraints_to_field_sets.items()
+    )
+    return LintResults(partitioned_results)
+
+
 def rules():
     return [
         bandit_lint,
+        bandit_lint_partition,
         SubsystemRule(Bandit),
         UnionRule(LintRequest, BanditRequest),
         *download_pex_bin.rules(),

src/python/pants/backend/python/lint/bandit/rules_integration_test.py

@@ -7,7 +7,7 @@
 from pants.backend.python.lint.bandit.rules import rules as bandit_rules
 from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonLibrary
 from pants.base.specs import FilesystemLiteralSpec, OriginSpec, SingleAddress
-from pants.core.goals.lint import LintResult
+from pants.core.goals.lint import LintResults
 from pants.engine.addresses import Address
 from pants.engine.fs import FileContent
 from pants.engine.rules import RootRule
@@ -54,7 +54,7 @@ def run_bandit(
         passthrough_args: Optional[str] = None,
         skip: bool = False,
         additional_args: Optional[List[str]] = None,
-    ) -> LintResult:
+    ) -> LintResults:
         args = ["--backend-packages2=pants.backend.python.lint.bandit"]
         if config:
             self.create_file(relpath=".bandit", contents=config)
@@ -66,7 +66,7 @@ def run_bandit(
         if additional_args:
             args.extend(additional_args)
         return self.request_single_product(
-            LintResult,
+            LintResults,
             Params(
                 BanditRequest(BanditFieldSet.create(tgt) for tgt in targets),
                 create_options_bootstrapper(args=args),
@@ -76,72 +76,94 @@ def run_bandit(
     def test_passing_source(self) -> None:
         target = self.make_target_with_origin([self.good_source])
         result = self.run_bandit([target])
-        assert result.exit_code == 0
-        assert "No issues identified." in result.stdout.strip()
+        assert len(result) == 1
+        assert result[0].exit_code == 0
+        assert "No issues identified." in result[0].stdout.strip()
 
     def test_failing_source(self) -> None:
         target = self.make_target_with_origin([self.bad_source])
         result = self.run_bandit([target])
-        assert result.exit_code == 1
-        assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result.stdout
+        assert len(result) == 1
+        assert result[0].exit_code == 1
+        assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result[0].stdout
 
     def test_mixed_sources(self) -> None:
         target = self.make_target_with_origin([self.good_source, self.bad_source])
         result = self.run_bandit([target])
-        assert result.exit_code == 1
-        assert "good.py" not in result.stdout
-        assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result.stdout
+        assert len(result) == 1
+        assert result[0].exit_code == 1
+        assert "good.py" not in result[0].stdout
+        assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result[0].stdout
 
     def test_multiple_targets(self) -> None:
         targets = [
             self.make_target_with_origin([self.good_source]),
             self.make_target_with_origin([self.bad_source]),
         ]
         result = self.run_bandit(targets)
-        assert result.exit_code == 1
-        assert "good.py" not in result.stdout
-        assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result.stdout
+        assert len(result) == 1
+        assert result[0].exit_code == 1
+        assert "good.py" not in result[0].stdout
+        assert "Issue: [B303:blacklist] Use of insecure MD2, MD4, MD5" in result[0].stdout
 
     def test_precise_file_args(self) -> None:
         target = self.make_target_with_origin(
             [self.good_source, self.bad_source],
             origin=FilesystemLiteralSpec(self.good_source.path),
         )
         result = self.run_bandit([target])
-        assert result.exit_code == 0
-        assert "No issues identified." in result.stdout
+        assert len(result) == 1
+        assert result[0].exit_code == 0
+        assert "No issues identified." in result[0].stdout
 
     @skip_unless_python27_and_python3_present
     def test_uses_correct_python_version(self) -> None:
         py2_target = self.make_target_with_origin(
             [self.py3_only_source], interpreter_constraints="CPython==2.7.*"
         )
         py2_result = self.run_bandit([py2_target])
-        assert py2_result.exit_code == 0
-        assert "py3.py (syntax error while parsing AST from file)" in py2_result.stdout
+        assert len(py2_result) == 1
+        assert py2_result[0].exit_code == 0
+        assert "py3.py (syntax error while parsing AST from file)" in py2_result[0].stdout
+
         py3_target = self.make_target_with_origin(
             [self.py3_only_source], interpreter_constraints="CPython>=3.6"
         )
         py3_result = self.run_bandit([py3_target])
-        assert py3_result.exit_code == 0
-        assert "No issues identified." in py3_result.stdout.strip()
+        assert len(py3_result) == 1
+        assert py3_result[0].exit_code == 0
+        assert "No issues identified." in py3_result[0].stdout
+
+        # Test that we partition incompatible targets when passed in a single batch. We expect Py2
+        # to still fail, but Py3 should pass.
+        combined_result = self.run_bandit([py2_target, py3_target])
+        assert len(combined_result) == 2
+        batched_py2_result, batched_py3_result = sorted(
+            combined_result, key=lambda result: result.stderr
+        )
+        assert batched_py2_result.exit_code == 0
+        assert "py3.py (syntax error while parsing AST from file)" in batched_py2_result.stdout
+        assert batched_py3_result.exit_code == 0
+        assert "No issues identified." in batched_py3_result.stdout
 
     def test_respects_config_file(self) -> None:
         target = self.make_target_with_origin([self.bad_source])
         result = self.run_bandit([target], config="skips: ['B303']\n")
-        assert result.exit_code == 0
-        assert "No issues identified." in result.stdout.strip()
+        assert len(result) == 1
+        assert result[0].exit_code == 0
+        assert "No issues identified." in result[0].stdout.strip()
 
     def test_respects_passthrough_args(self) -> None:
         target = self.make_target_with_origin([self.bad_source])
         result = self.run_bandit([target], passthrough_args="--skip B303")
-        assert result.exit_code == 0
-        assert "No issues identified." in result.stdout.strip()
+        assert len(result) == 1
+        assert result[0].exit_code == 0
+        assert "No issues identified." in result[0].stdout.strip()
 
     def test_skip(self) -> None:
         target = self.make_target_with_origin([self.bad_source])
         result = self.run_bandit([target], skip=True)
-        assert result == LintResult.noop()
+        assert not result
 
     def test_3rdparty_plugin(self) -> None:
         target = self.make_target_with_origin(
@@ -150,5 +172,6 @@ def test_3rdparty_plugin(self) -> None:
         result = self.run_bandit(
             [target], additional_args=["--bandit-extra-requirements=bandit-aws"]
         )
-        assert result.exit_code == 1
-        assert "Issue: [C100:hardcoded_aws_key]" in result.stdout
+        assert len(result) == 1
+        assert result[0].exit_code == 1
+        assert "Issue: [C100:hardcoded_aws_key]" in result[0].stdout

src/python/pants/backend/python/lint/black/rules.py

@@ -19,7 +19,7 @@
 from pants.backend.python.subsystems.subprocess_environment import SubprocessEncodingEnvironment
 from pants.backend.python.target_types import PythonSources
 from pants.core.goals.fmt import FmtRequest, FmtResult
-from pants.core.goals.lint import LintRequest, LintResult
+from pants.core.goals.lint import LintRequest, LintResult, LintResults
 from pants.core.util_rules import determine_source_files, strip_source_roots
 from pants.core.util_rules.determine_source_files import (
     AllSourceFilesRequest,
@@ -81,7 +81,7 @@ def generate_args(
 
 @rule
 async def setup(
-    request: SetupRequest,
+    setup_request: SetupRequest,
     black: Black,
     python_setup: PythonSetup,
     subprocess_encoding_environment: SubprocessEncodingEnvironment,
@@ -107,11 +107,11 @@ async def setup(
     )
 
     all_source_files_request = Get[SourceFiles](
-        AllSourceFilesRequest(field_set.sources for field_set in request.request.field_sets)
+        AllSourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets)
     )
     specified_source_files_request = Get[SourceFiles](
         SpecifiedSourceFilesRequest(
-            (field_set.sources, field_set.origin) for field_set in request.request.field_sets
+            (field_set.sources, field_set.origin) for field_set in setup_request.request.field_sets
         )
     )
 
@@ -120,16 +120,16 @@ async def setup(
         config_snapshot_request,
         specified_source_files_request,
     ]
-    if request.request.prior_formatter_result is None:
+    if setup_request.request.prior_formatter_result is None:
         requests.append(all_source_files_request)
     requirements_pex, config_snapshot, specified_source_files, *rest = cast(
         Union[Tuple[Pex, Snapshot, SourceFiles], Tuple[Pex, Snapshot, SourceFiles, SourceFiles]],
         await MultiGet(requests),
     )
 
     all_source_files_snapshot = (
-        request.request.prior_formatter_result
-        if request.request.prior_formatter_result
+        setup_request.request.prior_formatter_result
+        if setup_request.request.prior_formatter_result
         else rest[0].snapshot
     )
 
@@ -140,7 +140,7 @@ async def setup(
     )
 
     address_references = ", ".join(
-        sorted(field_set.address.reference() for field_set in request.request.field_sets)
+        sorted(field_set.address.reference() for field_set in setup_request.request.field_sets)
     )
 
     process = requirements_pex.create_process(
@@ -150,12 +150,12 @@ async def setup(
         pex_args=generate_args(
             specified_source_files=specified_source_files,
             black=black,
-            check_only=request.check_only,
+            check_only=setup_request.check_only,
         ),
         input_digest=input_digest,
         output_files=all_source_files_snapshot.files,
         description=(
-            f"Run Black on {pluralize(len(request.request.field_sets), 'target')}: {address_references}."
+            f"Run Black on {pluralize(len(setup_request.request.field_sets), 'target')}: {address_references}."
         ),
     )
     return Setup(process, original_digest=all_source_files_snapshot.digest)
@@ -176,13 +176,17 @@ async def black_fmt(field_sets: BlackRequest, black: Black) -> FmtResult:
 
 
 @named_rule(desc="Lint using Black")
-async def black_lint(field_sets: BlackRequest, black: Black) -> LintResult:
+async def black_lint(field_sets: BlackRequest, black: Black) -> LintResults:
     if black.options.skip:
-        return LintResult.noop()
+        return LintResults()
     setup = await Get[Setup](SetupRequest(field_sets, check_only=True))
     result = await Get[FallibleProcessResult](Process, setup.process)
-    return LintResult.from_fallible_process_result(
-        result, linter_name="Black", strip_chroot_path=True
+    return LintResults(
+        [
+            LintResult.from_fallible_process_result(
+                result, linter_name="Black", strip_chroot_path=True
+            )
+        ]
     )