Fix Black and MyPy to work by default with Python 3.8+ code

[ci skip-rust]
[ci skip-build-wheels]

pants.toml

@@ -75,9 +75,6 @@ config = ["pyproject.toml", "examples/.isort.cfg"]
 
 [mypy]
 config = "build-support/mypy/mypy.ini"
-# TODO(John Sirois): Remove once proper interpreter selection is performed automatically as part of
-#  https://github.com/pantsbuild/pants/issues/10131
-interpreter_constraints=["CPython>=3.6"]
 
 [pants-releases]
 release_notes = """

src/python/pants/backend/python/goals/setup_py.py

@@ -9,7 +9,7 @@
 from abc import ABC, abstractmethod
 from collections import abc, defaultdict
 from dataclasses import dataclass
-from typing import Any, Dict, Iterable, List, Mapping, Set, Tuple, cast
+from typing import Any, Dict, List, Mapping, Set, Tuple, cast
 
 from pants.backend.python.macros.python_artifact import PythonArtifact
 from pants.backend.python.subsystems.setuptools import Setuptools
@@ -26,7 +26,6 @@
     PexProcess,
     PexRequest,
     PexRequirements,
-    parse_interpreter_constraint,
 )
 from pants.backend.python.util_rules.python_sources import (
     PythonSourceFilesRequest,
@@ -405,14 +404,19 @@ async def run_setup_pys(
         )
         exported_targets = list(FrozenOrderedSet(owners))
 
-    py2 = is_python2(
-        python_setup.compatibilities_or_constraints(
-            target_with_origin.target.get(PythonInterpreterCompatibility).value
+    interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
+        (
+            target_with_origin.target[PythonInterpreterCompatibility]
             for target_with_origin in targets_with_origins
-        )
+            if target_with_origin.target.has_field(PythonInterpreterCompatibility)
+        ),
+        python_setup,
     )
     chroots = await MultiGet(
-        Get(SetupPyChroot, SetupPyChrootRequest(exported_target, py2))
+        Get(
+            SetupPyChroot,
+            SetupPyChrootRequest(exported_target, py2=interpreter_constraints.includes_python2()),
+        )
         for exported_target in exported_targets
     )
 
@@ -958,21 +962,6 @@ def has_args(call_node: ast.Call, required_arg_ids: Tuple[str, ...]) -> bool:
     return False
 
 
-def is_python2(compatibilities_or_constraints: Iterable[str]) -> bool:
-    """Checks if we should assume python2 code."""
-
-    def iter_reqs():
-        for constraint in compatibilities_or_constraints:
-            yield parse_interpreter_constraint(constraint)
-
-    for req in iter_reqs():
-        for python_27_ver in range(0, 18):  # The last python 2.7 version was 2.7.18.
-            if req.specifier.contains(f"2.7.{python_27_ver}"):
-                # At least one constraint limits us to Python 2, so assume that.
-                return True
-    return False
-
-
 def rules():
     return [
         *python_sources_rules(),

src/python/pants/backend/python/goals/setup_py_test.py

@@ -30,7 +30,6 @@
     get_owned_dependencies,
     get_requirements,
     get_sources,
-    is_python2,
     validate_args,
 )
 from pants.backend.python.macros.python_artifact import PythonArtifact
@@ -808,32 +807,3 @@ def test_declares_pkg_resources_namespace_package(python_src: str) -> None:
 )
 def test_does_not_declare_pkg_resources_namespace_package(python_src: str) -> None:
     assert not declares_pkg_resources_namespace_package(python_src)
-
-
-@pytest.mark.parametrize(
-    "constraints",
-    [
-        ["CPython>=2.7,<3"],
-        ["CPython>=2.7,<3", "CPython>=3.6"],
-        ["CPython>=2.7.13"],
-        ["CPython>=2.7.13,<2.7.16"],
-        ["CPython>=2.7.13,!=2.7.16"],
-        ["PyPy>=2.7,<3"],
-    ],
-)
-def test_is_python2(constraints):
-    assert is_python2(constraints)
-
-
-@pytest.mark.parametrize(
-    "constraints",
-    [
-        ["CPython>=3.6"],
-        ["CPython>=3.7"],
-        ["CPython>=3.6", "CPython>=3.8"],
-        ["CPython!=2.7.*"],
-        ["PyPy>=3.6"],
-    ],
-)
-def test_is_not_python2(constraints):
-    assert not is_python2(constraints)

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

@@ -8,7 +8,7 @@
 
 from pants.backend.python.lint.black.subsystem import Black
 from pants.backend.python.lint.python_fmt import PythonFmtRequest
-from pants.backend.python.target_types import PythonSources
+from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources
 from pants.backend.python.util_rules import pex
 from pants.backend.python.util_rules.pex import (
     Pex,
@@ -26,6 +26,7 @@
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import FieldSet
 from pants.engine.unions import UnionRule
+from pants.python.python_setup import PythonSetup
 from pants.util.logging import LogLevel
 from pants.util.strutil import pluralize
 
@@ -35,6 +36,7 @@ class BlackFieldSet(FieldSet):
     required_fields = (PythonSources,)
 
     sources: PythonSources
+    interpreter_constraints: PythonInterpreterCompatibility
 
 
 class BlackRequest(PythonFmtRequest, LintRequest):
@@ -71,14 +73,31 @@ def generate_args(*, source_files: SourceFiles, black: Black, check_only: bool)
 
 
 @rule(level=LogLevel.DEBUG)
-async def setup_black(setup_request: SetupRequest, black: Black) -> Setup:
-    requirements_pex_request = Get(
+async def setup_black(
+    setup_request: SetupRequest, black: Black, python_setup: PythonSetup
+) -> Setup:
+    # Black requires 3.6+ but uses the typed-ast library to work with 2.7, 3.4, 3.5, 3.6, and 3.7.
+    # However, typed-ast does not understand 3.8, so instead we must run Black with Python 3.8 when
+    # relevant. We only do this if if <3.8 can't be used, as we don't want a loose requirement like
+    # `>=3.6` to result in requiring Python 3.8, which would error if 3.8 is not installed on the
+    # machine.
+    all_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
+        (field_set.interpreter_constraints for field_set in setup_request.request.field_sets),
+        python_setup,
+    )
+    tool_interpreter_constraints = PexInterpreterConstraints(
+        black.interpreter_constraints
+        if not all_interpreter_constraints.requires_python38_or_newer()
+        else ("CPython>=3.8",)
+    )
+
+    black_pex_request = Get(
         Pex,
         PexRequest(
             output_filename="black.pex",
             internal_only=True,
             requirements=PexRequirements(black.all_requirements),
-            interpreter_constraints=PexInterpreterConstraints(black.interpreter_constraints),
+            interpreter_constraints=tool_interpreter_constraints,
             entry_point=black.entry_point,
         ),
     )
@@ -97,8 +116,8 @@ async def setup_black(setup_request: SetupRequest, black: Black) -> Setup:
         SourceFilesRequest(field_set.sources for field_set in setup_request.request.field_sets),
     )
 
-    source_files, requirements_pex, config_digest = await MultiGet(
-        source_files_request, requirements_pex_request, config_digest_request
+    source_files, black_pex, config_digest = await MultiGet(
+        source_files_request, black_pex_request, config_digest_request
     )
     source_files_snapshot = (
         source_files.snapshot
@@ -108,13 +127,13 @@ async def setup_black(setup_request: SetupRequest, black: Black) -> Setup:
 
     input_digest = await Get(
         Digest,
-        MergeDigests((source_files_snapshot.digest, requirements_pex.digest, config_digest)),
+        MergeDigests((source_files_snapshot.digest, black_pex.digest, config_digest)),
     )
 
     process = await Get(
         Process,
         PexProcess(
-            requirements_pex,
+            black_pex,
             argv=generate_args(
                 source_files=source_files, black=black, check_only=setup_request.check_only
             ),

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

@@ -1,6 +1,7 @@
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from textwrap import dedent
 from typing import List, Optional, Sequence, Tuple
 
 import pytest
@@ -17,6 +18,7 @@
 from pants.engine.target import Target
 from pants.option.options_bootstrapper import OptionsBootstrapper
 from pants.testutil.option_util import create_options_bootstrapper
+from pants.testutil.python_interpreter_selection import skip_unless_python38_present
 from pants.testutil.rule_runner import RuleRunner
 
 
@@ -28,22 +30,32 @@ def rule_runner() -> RuleRunner:
             QueryRule(LintResults, (BlackRequest, OptionsBootstrapper)),
             QueryRule(FmtResult, (BlackRequest, OptionsBootstrapper)),
             QueryRule(SourceFiles, (SourceFilesRequest, OptionsBootstrapper)),
-        ]
+        ],
+        target_types=[PythonLibrary],
     )
 
 
-GOOD_SOURCE = FileContent(path="good.py", content=b'animal = "Koala"\n')
-BAD_SOURCE = FileContent(path="bad.py", content=b'name=    "Anakin"\n')
-FIXED_BAD_SOURCE = FileContent(path="bad.py", content=b'name = "Anakin"\n')
+GOOD_SOURCE = FileContent("good.py", b'animal = "Koala"\n')
+BAD_SOURCE = FileContent("bad.py", b'name=    "Anakin"\n')
+FIXED_BAD_SOURCE = FileContent("bad.py", b'name = "Anakin"\n')
 # Note the single quotes, which Black does not like by default. To get Black to pass, it will
 # need to successfully read our config/CLI args.
-NEEDS_CONFIG_SOURCE = FileContent(path="needs_config.py", content=b"animal = 'Koala'\n")
+NEEDS_CONFIG_SOURCE = FileContent("needs_config.py", b"animal = 'Koala'\n")
 
 
-def make_target(rule_runner: RuleRunner, source_files: List[FileContent]) -> Target:
+def make_target(
+    rule_runner: RuleRunner,
+    source_files: List[FileContent],
+    *,
+    name: str = "target",
+    interpreter_constraints: Optional[str] = None,
+) -> Target:
     for source_file in source_files:
-        rule_runner.create_file(f"{source_file.path}", source_file.content.decode())
-    return PythonLibrary({}, address=Address.parse(":target"))
+        rule_runner.create_file(source_file.path, source_file.content.decode())
+    rule_runner.add_to_build_file(
+        "", f"python_library(name='{name}', compatibility={repr(interpreter_constraints)})\n"
+    )
+    return rule_runner.get_target(Address("", target_name=name))
 
 
 def run_black(
@@ -122,7 +134,10 @@ def test_mixed_sources(rule_runner: RuleRunner) -> None:
 
 
 def test_multiple_targets(rule_runner: RuleRunner) -> None:
-    targets = [make_target(rule_runner, [GOOD_SOURCE]), make_target(rule_runner, [BAD_SOURCE])]
+    targets = [
+        make_target(rule_runner, [GOOD_SOURCE], name="good"),
+        make_target(rule_runner, [BAD_SOURCE], name="bad"),
+    ]
     lint_results, fmt_result = run_black(rule_runner, targets)
     assert len(lint_results) == 1
     assert lint_results[0].exit_code == 1
@@ -164,3 +179,33 @@ def test_skip(rule_runner: RuleRunner) -> None:
     assert not lint_results
     assert fmt_result.skipped is True
     assert fmt_result.did_change is False
+
+
+@skip_unless_python38_present
+def test_works_with_python38(rule_runner: RuleRunner) -> None:
+    """Black's typed-ast dependency does not understand Python 3.8, so we must instead run Black
+    with Python 3.8 when relevant."""
+    py38_sources = FileContent(
+        "py38.py",
+        dedent(
+            """\
+            import datetime
+
+            x = True
+            if y := x:
+                print("x is truthy and now assigned to y")
+
+
+            class Foo:
+                pass
+            """
+        ).encode(),
+    )
+    target = make_target(rule_runner, [py38_sources], interpreter_constraints=">=3.8")
+    lint_results, fmt_result = run_black(rule_runner, [target])
+    assert len(lint_results) == 1
+    assert lint_results[0].exit_code == 0
+    assert "1 file would be left unchanged" in lint_results[0].stderr
+    assert "1 file left unchanged" in fmt_result.stderr
+    assert fmt_result.output == get_digest(rule_runner, [py38_sources])
+    assert fmt_result.did_change is False

src/python/pants/backend/python/typecheck/mypy/rules.py

@@ -1,10 +1,11 @@
 # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+import logging
 from dataclasses import dataclass
 from typing import Tuple
 
-from pants.backend.python.target_types import PythonSources
+from pants.backend.python.target_types import PythonInterpreterCompatibility, PythonSources
 from pants.backend.python.typecheck.mypy.subsystem import MyPy
 from pants.backend.python.util_rules.pex import (
     Pex,
@@ -34,9 +35,12 @@
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import FieldSet, TransitiveTargets
 from pants.engine.unions import UnionRule
+from pants.python.python_setup import PythonSetup
 from pants.util.logging import LogLevel
 from pants.util.strutil import pluralize
 
+logger = logging.getLogger(__name__)
+
 
 @dataclass(frozen=True)
 class MyPyFieldSet(FieldSet):
@@ -60,15 +64,43 @@ def generate_args(mypy: MyPy, *, file_list_path: str) -> Tuple[str, ...]:
 
 # TODO(#10131): Improve performance, e.g. by leveraging the MyPy cache.
 # TODO(#10131): Support plugins and type stubs.
+# TODO(#10131): Support third-party requirements.
 @rule(desc="Typecheck using MyPy", level=LogLevel.DEBUG)
-async def mypy_typecheck(request: MyPyRequest, mypy: MyPy) -> TypecheckResults:
+async def mypy_typecheck(
+    request: MyPyRequest, mypy: MyPy, python_setup: PythonSetup
+) -> TypecheckResults:
     if mypy.skip:
         return TypecheckResults([], typechecker_name="MyPy")
 
     transitive_targets = await Get(
         TransitiveTargets, Addresses(fs.address for fs in request.field_sets)
     )
 
+    # Interpreter constraints are tricky with MyPy:
+    #  * MyPy requires running with Python 3.5+. If run with Python 3.5 - 3.7, MyPy can understand
+    #     Python 2.7 and 3.4-3.7 thanks to the typed-ast library, but it can't understand 3.8+ If
+    #     run with Python 3.8, it can understand 2.7 and 3.4-3.8. So, we need to check if the user
+    #     has code that requires Python 3.8+, and if so, use a tighter requirement. We only do this
+    #     if <3.8 can't be used, as we don't want a loose requirement like `>=3.6` to result in
+    #     requiring Python 3.8, which would error if 3.8 is not installed on the machine.
+    #  * We must resolve third-party dependencies. This should use whatever the actual code's
+    #     constraints are, as the constraints for the tool can be different than for the
+    #     requirements.
+    #  * The runner Pex should use the same constraints as the tool Pex.
+    all_interpreter_constraints = PexInterpreterConstraints.create_from_compatibility_fields(
+        (
+            tgt[PythonInterpreterCompatibility]
+            for tgt in transitive_targets.closure
+            if tgt.has_field(PythonInterpreterCompatibility)
+        ),
+        python_setup,
+    )
+    tool_interpreter_constraints = PexInterpreterConstraints(
+        mypy.interpreter_constraints
+        if not all_interpreter_constraints.requires_python38_or_newer()
+        else ("CPython>=3.8",)
+    )
+
     prepared_sources_request = Get(
         PythonSourceFiles,
         PythonSourceFilesRequest(transitive_targets.closure),
@@ -79,12 +111,7 @@ async def mypy_typecheck(request: MyPyRequest, mypy: MyPy) -> TypecheckResults:
             output_filename="mypy.pex",
             internal_only=True,
             requirements=PexRequirements(mypy.all_requirements),
-            # NB: This only determines what MyPy is run with. The user can specify what version
-            # their code is with `--python-version`. See
-            # https://mypy.readthedocs.io/en/stable/config_file.html#platform-configuration. We do
-            # not auto-configure this for simplicity and to avoid Pants magically setting values for
-            # users.
-            interpreter_constraints=PexInterpreterConstraints(mypy.interpreter_constraints),
+            interpreter_constraints=tool_interpreter_constraints,
             entry_point=mypy.entry_point,
         ),
     )