Add V2 implementation of isort (pantsbuild#8689)

### Problem

We want isort to work with `fmt-v2` and `lint-v2`. Moving from V1 to V2 is particularly helpful for caching - isort will only run over targets that have any changes to their source file(s).

### Solution

Leverage the infrastructure that we set up for Black to port isort. 

**Note**: there is much duplication between the Black and isort implementations. Likely, this could be factored out. I do not think we should do this, yet, though, as we are too early in the discovery process for what V2 linters and formatters will look like. For example, we should first implement a linter like Pylint _and_ a formatter or linter for another language. Then, we will be able to see what is truly generalizable.

### Result

Users may now enable the isort plugin by adding this to their `pants.ini`:

```ini
[GLOBAL]

backend_packages: +[
    "pants.backend.python.lint.isort",
  ]
```

Then they may run `./pants lint-v2` and `./pants fmt-v2`.

There is still some follow-up work, like fixing `fmt.py` and `lint.py` to not run against targets that have `sources` defined but which is empty, but this provides the overall functionality.

BUILD

@@ -26,6 +26,11 @@ files(
   source = '.gitignore',
 )
 
+files(
+  name = 'isort_cfg',
+  source = '.isort.cfg',
+)
+
 files(
   name = 'scalajs_3rdparty_directory',
   sources = rglobs('contrib/scalajs/3rdparty/*'),

pants.ini

@@ -45,6 +45,7 @@ pythonpath: [
   ]
 
 backend_packages: +[
+    "pants.backend.python.lint.isort",
     "pants.backend.docgen",
     "pants.contrib.avro",
     "pants.contrib.awslambda.python",
@@ -244,6 +245,10 @@ eslint_setupdir: %(pants_supportdir)s/eslint
 eslint_config: %(pants_supportdir)s/eslint/.eslintrc
 eslint_ignore: %(pants_supportdir)s/eslint/.eslintignore
 
+[isort]
+config: .isort.cfg
+
+
 [lint]
 transitive: False
 

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

@@ -31,7 +31,7 @@
 
 class BlackIntegrationTest(TestBase):
 
-  good_source = FileContent(path="test/good.py", content=b'name = "Anakin"\n')
+  good_source = FileContent(path="test/good.py", content=b'animal = "Koala"\n')
   bad_source = FileContent(path="test/bad.py", content=b'name=    "Anakin"\n')
   fixed_bad_source = FileContent(path="test/bad.py", content=b'name = "Anakin"\n')
 
@@ -117,7 +117,7 @@ def test_multiple_mixed_sources(self) -> None:
 
   def test_respects_config_file(self) -> None:
     # Note the single quotes, which Black does not like by default.
-    source = FileContent(path="test/good.py", content=b"name = 'Anakin'\n")
+    source = FileContent(path="test/good.py", content=b"animal = 'Koala'\n")
     lint_result, fmt_result = self.run_black(
       [source], config="[tool.black]\nskip-string-normalization = 'true'\n"
     )

src/python/pants/backend/python/lint/isort/BUILD

@@ -0,0 +1,45 @@
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+python_library(
+  dependencies=[
+    '3rdparty/python:dataclasses',
+    'src/python/pants/backend/python/subsystems',
+    'src/python/pants/backend/python/rules',
+    'src/python/pants/backend/python/targets',
+    'src/python/pants/engine:fs',
+    'src/python/pants/engine:isolated_process',
+    'src/python/pants/engine:rules',
+    'src/python/pants/engine:selectors',
+    'src/python/pants/option',
+    'src/python/pants/rules/core',
+  ],
+)
+
+python_tests(
+  name='tests',
+  sources=globs('*_test.py', exclude=[globs('*_integration_test.py')]),
+  dependencies=[
+    ':isort',
+  ],
+)
+
+
+python_tests(
+  name='integration',
+  sources=globs('*_integration_test.py'),
+  dependencies=[
+    ':isort',
+    'src/python/pants/backend/python/subsystems',
+    'src/python/pants/backend/python/targets',
+    'src/python/pants/engine:fs',
+    'src/python/pants/engine:rules',
+    'src/python/pants/engine:selectors',
+    'src/python/pants/engine/legacy:structs',
+    'src/python/pants/rules/core',
+    'src/python/pants/source',
+    'src/python/pants/testutil:test_base',
+    'src/python/pants/testutil/subsystem',
+  ],
+  tags = {'integration'},
+)

src/python/pants/backend/python/lint/isort/register.py

@@ -0,0 +1,13 @@
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+
+from pants.backend.python.lint.isort import rules as isort_rules
+from pants.backend.python.targets import formattable_python_target
+
+
+def rules():
+  return (
+    *isort_rules.rules(),
+    *formattable_python_target.rules(),
+  )

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

@@ -0,0 +1,146 @@
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from dataclasses import dataclass
+from typing import Tuple
+
+from pants.backend.python.lint.isort.subsystem import Isort
+from pants.backend.python.rules.pex import (
+  CreatePex,
+  Pex,
+  PexInterpreterConstraints,
+  PexRequirements,
+)
+from pants.backend.python.subsystems.python_setup import PythonSetup
+from pants.backend.python.subsystems.subprocess_environment import SubprocessEncodingEnvironment
+from pants.backend.python.targets.formattable_python_target import FormattablePythonTarget
+from pants.engine.fs import Digest, DirectoriesToMerge, PathGlobs, Snapshot
+from pants.engine.isolated_process import (
+  ExecuteProcessRequest,
+  ExecuteProcessResult,
+  FallibleExecuteProcessResult,
+)
+from pants.engine.rules import optionable_rule, rule
+from pants.engine.selectors import Get
+from pants.rules.core.fmt import FmtResult
+from pants.rules.core.lint import LintResult
+
+
+@dataclass(frozen=True)
+class IsortSetup:
+  """This abstraction is used to deduplicate the implementations for the `fmt` and `lint` rules,
+  which only differ in whether or not to append `--check-only` to the isort CLI args."""
+  resolved_requirements_pex: Pex
+  merged_input_files: Digest
+
+  @staticmethod
+  def generate_pex_arg_list(*, files: Tuple[str, ...], check_only: bool) -> Tuple[str, ...]:
+    pex_args = []
+    if check_only:
+      pex_args.append("--check-only")
+    pex_args.extend(files)
+    return tuple(pex_args)
+
+  def create_execute_request(
+    self,
+    *,
+    wrapped_target: FormattablePythonTarget,
+    python_setup: PythonSetup,
+    subprocess_encoding_environment: SubprocessEncodingEnvironment,
+    check_only: bool,
+  ) -> ExecuteProcessRequest:
+    target = wrapped_target.target
+    return self.resolved_requirements_pex.create_execute_request(
+      python_setup=python_setup,
+      subprocess_encoding_environment=subprocess_encoding_environment,
+      pex_path="./isort.pex",
+      pex_args=self.generate_pex_arg_list(
+        files=target.sources.snapshot.files, check_only=check_only
+      ),
+      input_files=self.merged_input_files,
+      output_files=target.sources.snapshot.files,
+      description=f'Run isort for {target.address.reference()}',
+    )
+
+
+@rule
+async def setup_isort(wrapped_target: FormattablePythonTarget, isort: Isort) -> IsortSetup:
+  # NB: isort auto-discovers config. We ensure that the config is included in the input files.
+  config_path = isort.get_options().config
+  config_snapshot = await Get(Snapshot, PathGlobs(include=(config_path,)))
+  resolved_requirements_pex = await Get(
+    Pex, CreatePex(
+      output_filename="isort.pex",
+      requirements=PexRequirements(requirements=tuple(isort.get_requirement_specs())),
+      interpreter_constraints=PexInterpreterConstraints(
+        constraint_set=tuple(isort.default_interpreter_constraints)
+      ),
+      entry_point=isort.get_entry_point(),
+    )
+  )
+
+  sources_digest = wrapped_target.target.sources.snapshot.directory_digest
+
+  merged_input_files = await Get(
+    Digest,
+    DirectoriesToMerge(
+      directories=(
+        sources_digest,
+        resolved_requirements_pex.directory_digest,
+        config_snapshot.directory_digest,
+      )
+    ),
+  )
+  return IsortSetup(resolved_requirements_pex, merged_input_files)
+
+
+@rule
+async def fmt(
+  wrapped_target: FormattablePythonTarget,
+  isort_setup: IsortSetup,
+  python_setup: PythonSetup,
+  subprocess_encoding_environment: SubprocessEncodingEnvironment,
+) -> FmtResult:
+  request = isort_setup.create_execute_request(
+    wrapped_target=wrapped_target,
+    python_setup=python_setup,
+    subprocess_encoding_environment=subprocess_encoding_environment,
+    check_only=False
+  )
+  result = await Get(ExecuteProcessResult, ExecuteProcessRequest, request)
+  return FmtResult(
+    digest=result.output_directory_digest,
+    stdout=result.stdout.decode(),
+    stderr=result.stderr.decode(),
+  )
+
+
+@rule
+async def lint(
+  wrapped_target: FormattablePythonTarget,
+  isort_setup: IsortSetup,
+  python_setup: PythonSetup,
+  subprocess_encoding_environment: SubprocessEncodingEnvironment,
+) -> LintResult:
+  request = isort_setup.create_execute_request(
+    wrapped_target=wrapped_target,
+    python_setup=python_setup,
+    subprocess_encoding_environment=subprocess_encoding_environment,
+    check_only=True
+  )
+  result = await Get(FallibleExecuteProcessResult, ExecuteProcessRequest, request)
+  return LintResult(
+    exit_code=result.exit_code,
+    stdout=result.stdout.decode(),
+    stderr=result.stderr.decode(),
+  )
+
+
+def rules():
+  return [
+    setup_isort,
+    fmt,
+    lint,
+    optionable_rule(Isort),
+    optionable_rule(PythonSetup),
+  ]

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

@@ -0,0 +1,140 @@
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from typing import List, Optional, Tuple
+
+from pants.backend.python.lint.isort.rules import IsortSetup, fmt, lint, setup_isort
+from pants.backend.python.lint.isort.subsystem import Isort
+from pants.backend.python.rules.download_pex_bin import download_pex_bin
+from pants.backend.python.rules.pex import CreatePex, create_pex
+from pants.backend.python.subsystems.python_native_code import (
+  PythonNativeCode,
+  create_pex_native_build_environment,
+)
+from pants.backend.python.subsystems.python_setup import PythonSetup
+from pants.backend.python.subsystems.subprocess_environment import (
+  SubprocessEnvironment,
+  create_subprocess_encoding_environment,
+)
+from pants.backend.python.targets.formattable_python_target import FormattablePythonTarget
+from pants.build_graph.address import Address
+from pants.engine.fs import Digest, FileContent, InputFilesContent, Snapshot
+from pants.engine.legacy.structs import TargetAdaptor
+from pants.engine.rules import RootRule
+from pants.engine.selectors import Params
+from pants.rules.core.fmt import FmtResult
+from pants.rules.core.lint import LintResult
+from pants.source.wrapped_globs import EagerFilesetWithSpec
+from pants.testutil.subsystem.util import global_subsystem_instance, init_subsystems
+from pants.testutil.test_base import TestBase
+
+
+class IsortIntegrationTest(TestBase):
+
+  good_source = FileContent(path="test/good.py", content=b'from animals import cat, dog\n')
+  bad_source = FileContent(path="test/bad.py", content=b'from colors import green, blue\n')
+  fixed_bad_source = FileContent(path="test/bad.py", content=b'from colors import blue, green\n')
+
+  @classmethod
+  def rules(cls):
+    return (
+      *super().rules(),
+      fmt,
+      lint,
+      setup_isort,
+      create_pex,
+      create_subprocess_encoding_environment,
+      create_pex_native_build_environment,
+      download_pex_bin,
+      RootRule(CreatePex),
+      RootRule(FormattablePythonTarget),
+      RootRule(Isort),
+      RootRule(IsortSetup),
+      RootRule(PythonSetup),
+      RootRule(PythonNativeCode),
+      RootRule(SubprocessEnvironment),
+    )
+
+  def setUp(self):
+    super().setUp()
+    init_subsystems([Isort, PythonSetup, PythonNativeCode, SubprocessEnvironment])
+
+  def run_isort(
+    self, source_files: List[FileContent], *, config: Optional[str] = None
+  ) -> Tuple[LintResult, FmtResult]:
+    if config is not None:
+      self.create_file(relpath=".isort.cfg", contents=config)
+    input_snapshot = self.request_single_product(Snapshot, InputFilesContent(source_files))
+    target = FormattablePythonTarget(
+      TargetAdaptor(
+        sources=EagerFilesetWithSpec('test', {'globs': []}, snapshot=input_snapshot),
+        address=Address.parse("test:target"),
+      )
+    )
+    isort_subsystem = global_subsystem_instance(
+      Isort, options={Isort.options_scope: {"config": ".isort.cfg" if config else None}}
+    )
+    isort_setup = self.request_single_product(
+      IsortSetup,
+      Params(
+        target,
+        isort_subsystem,
+        PythonNativeCode.global_instance(),
+        PythonSetup.global_instance(),
+        SubprocessEnvironment.global_instance(),
+      )
+    )
+    fmt_and_lint_params = Params(
+      target, isort_setup, PythonSetup.global_instance(), SubprocessEnvironment.global_instance()
+    )
+    lint_result = self.request_single_product(LintResult, fmt_and_lint_params)
+    fmt_result = self.request_single_product(FmtResult, fmt_and_lint_params)
+    return lint_result, fmt_result
+
+  def get_digest(self, source_files: List[FileContent]) -> Digest:
+    return self.request_single_product(Digest, InputFilesContent(source_files))
+
+  def test_single_passing_source(self) -> None:
+    lint_result, fmt_result = self.run_isort([self.good_source])
+    assert lint_result.exit_code == 0
+    assert lint_result.stdout == ""
+    assert fmt_result.stdout == ""
+    assert fmt_result.digest == self.get_digest([self.good_source])
+
+  def test_single_failing_source(self) -> None:
+    lint_result, fmt_result = self.run_isort([self.bad_source])
+    assert lint_result.exit_code == 1
+    assert "test/bad.py Imports are incorrectly sorted" in lint_result.stdout
+    assert "Fixing" in fmt_result.stdout
+    assert "test/bad.py" in fmt_result.stdout
+    assert fmt_result.digest == self.get_digest([self.fixed_bad_source])
+
+  def test_multiple_mixed_sources(self) -> None:
+    lint_result, fmt_result = self.run_isort([self.good_source, self.bad_source])
+    assert lint_result.exit_code == 1
+    assert "test/bad.py Imports are incorrectly sorted" in lint_result.stdout
+    assert "test/good.py" not in lint_result.stdout
+    assert "Fixing" in fmt_result.stdout and "test/bad.py" in fmt_result.stdout
+    assert "test/good.py" not in fmt_result.stdout
+    assert fmt_result.digest == self.get_digest([self.good_source, self.fixed_bad_source])
+
+  def test_respects_config_file(self) -> None:
+    # Normally isort wants "as imports" on a new line, so `source` should not be formatted with a
+    # default isort run. We configure our settings to instead combine the two lines. If the config
+    # is picked up, then `source` should be reformatted by isort.
+    source = FileContent(
+      path="test/config.py",
+      content=b"from colors import blue\nfrom colors import green as verde"
+    )
+    fixed_source = FileContent(
+      path="test/config.py",
+      content=b"from colors import blue, green as verde\n"
+    )
+    lint_result, fmt_result = self.run_isort(
+      [source], config="[settings]\ncombine_as_imports=True\n"
+    )
+    assert lint_result.exit_code == 1
+    assert "test/config.py Imports are incorrectly sorted" in lint_result.stdout
+    assert "Fixing" in fmt_result.stdout
+    assert "test/config.py" in fmt_result.stdout
+    assert fmt_result.digest == self.get_digest([fixed_source])