Add --black-enable option to turn Black on and off

pants.ini

@@ -83,8 +83,10 @@ pants_ignore: +[
   ]
 
 [black]
+enable: False
 config: pyproject.toml
 
+
 [cache]
 # Caching is on globally by default, but we disable it here for development purposes.
 # It is explicitly re-enabled below for [cache.bootstrap] only.

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

@@ -6,19 +6,15 @@
 from pants.backend.python.python_requirement import PythonRequirement
 from pants.backend.python.python_requirements import PythonRequirements
 from pants.backend.python.rules import (
+  black,
   download_pex_bin,
   inject_init,
   pex,
   python_create_binary,
-  python_fmt,
   python_test_runner,
 )
-from pants.backend.python.subsystems.python_native_code import PythonNativeCode
-from pants.backend.python.subsystems.python_native_code import rules as python_native_code_rules
-from pants.backend.python.subsystems.subprocess_environment import SubprocessEnvironment
-from pants.backend.python.subsystems.subprocess_environment import (
-  rules as subprocess_environment_rules,
-)
+from pants.backend.python.subsystems import python_native_code, subprocess_environment
+from pants.backend.python.targets import formattable_python_target
 from pants.backend.python.targets.python_app import PythonApp
 from pants.backend.python.targets.python_binary import PythonBinary
 from pants.backend.python.targets.python_distribution import PythonDistribution
@@ -51,7 +47,7 @@
 
 
 def global_subsystems():
-  return (SubprocessEnvironment, PythonNativeCode)
+  return python_native_code.PythonNativeCode, subprocess_environment.SubprocessEnvironment
 
 
 def build_file_aliases():
@@ -98,12 +94,13 @@ def register_goals():
 
 def rules():
   return (
-    download_pex_bin.rules() +
-    inject_init.rules() +
-    python_fmt.rules() +
-    python_test_runner.rules() +
-    python_create_binary.rules() +
-    python_native_code_rules() +
-    pex.rules() +
-    subprocess_environment_rules()
+    *black.rules(),
+    *download_pex_bin.rules(),
+    *formattable_python_target.rules(),
+    *inject_init.rules(),
+    *pex.rules(),
+    *python_test_runner.rules(),
+    *python_create_binary.rules(),
+    *python_native_code.rules(),
+    *subprocess_environment.rules(),
   )

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

@@ -0,0 +1,156 @@
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from dataclasses import dataclass
+from pathlib import Path
+from typing import Optional, Tuple
+
+from pants.backend.python.rules.pex import (
+  CreatePex,
+  Pex,
+  PexInterpreterConstraints,
+  PexRequirements,
+)
+from pants.backend.python.subsystems.black import Black
+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 BlackSetup:
+  """This abstraction allows us to set up everything Black needs and, crucially, to cache this setup
+  so that it may be reused when running `fmt` vs. `lint` on the same target."""
+  config_path: Optional[Path]
+  resolved_requirements_pex: Pex
+  merged_input_files: Digest
+
+  def generate_pex_arg_list(self, *, files: Tuple[str, ...], check_only: bool) -> Tuple[str, ...]:
+    pex_args = []
+    if check_only:
+      pex_args.append("--check")
+    if self.config_path is not None:
+      pex_args.extend(["--config", self.config_path])
+    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="./black.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 Black for {target.address.reference()}',
+    )
+
+
+@rule
+async def setup_black(wrapped_target: FormattablePythonTarget, black: Black) -> BlackSetup:
+  config_path = black.get_options().config
+  config_snapshot = await Get(Snapshot, PathGlobs(include=(config_path,)))
+
+  resolved_requirements_pex = await Get(
+    Pex, CreatePex(
+      output_filename="black.pex",
+      requirements=PexRequirements(requirements=tuple(black.get_requirement_specs())),
+      interpreter_constraints=PexInterpreterConstraints(
+        constraint_set=tuple(black.default_interpreter_constraints)
+      ),
+      entry_point=black.get_entry_point(),
+    )
+  )
+  target = wrapped_target.target
+  sources_digest = target.sources.snapshot.directory_digest
+
+  all_input_digests = [
+    sources_digest,
+    resolved_requirements_pex.directory_digest,
+    config_snapshot.directory_digest,
+  ]
+  merged_input_files = await Get(
+    Digest,
+    DirectoriesToMerge(directories=tuple(all_input_digests)),
+  )
+  return BlackSetup(config_path, resolved_requirements_pex, merged_input_files)
+
+
+@rule
+async def fmt(
+  black: Black,
+  wrapped_target: FormattablePythonTarget,
+  black_setup: BlackSetup,
+  python_setup: PythonSetup,
+  subprocess_encoding_environment: SubprocessEncodingEnvironment,
+) -> FmtResult:
+  if not black.get_options().enable:
+    return FmtResult.noop()
+
+  request = black_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(
+  black: Black,
+  wrapped_target: FormattablePythonTarget,
+  black_setup: BlackSetup,
+  python_setup: PythonSetup,
+  subprocess_encoding_environment: SubprocessEncodingEnvironment,
+) -> LintResult:
+  if not black.get_options().enable:
+    return LintResult.noop()
+
+  request = black_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_black,
+    fmt,
+    lint,
+    optionable_rule(Black),
+    optionable_rule(PythonSetup),
+  ]

src/python/pants/backend/python/rules/python_fmt_integration_test.py → src/python/pants/backend/python/rules/black_integration_test.py

@@ -37,7 +37,7 @@ def build_directory():
     yield root_dir
 
 
-class PythonFmtIntegrationTest(PantsRunIntegrationTest):
+class BlackIntegrationTest(PantsRunIntegrationTest):
   def test_black_one_python_source_should_leave_one_file_unchanged(self):
     with build_directory() as root_dir:
       code = write_consistently_formatted_file(root_dir, "hello.py")

src/python/pants/backend/python/rules/black_test.py

@@ -0,0 +1,76 @@
+# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
+# Licensed under the Apache License, Version 2.0 (see LICENSE).
+
+from pants.backend.python.rules.black import BlackSetup, fmt, lint
+from pants.backend.python.rules.pex import Pex
+from pants.backend.python.subsystems.black import Black
+from pants.backend.python.subsystems.python_setup import PythonSetup
+from pants.backend.python.subsystems.subprocess_environment import SubprocessEnvironment
+from pants.backend.python.targets.formattable_python_target import FormattablePythonTarget
+from pants.engine.fs import EMPTY_DIRECTORY_DIGEST
+from pants.engine.isolated_process import (
+  ExecuteProcessRequest,
+  ExecuteProcessResult,
+  FallibleExecuteProcessResult,
+)
+from pants.engine.legacy.structs import TargetAdaptor
+from pants.rules.core.fmt import FmtResult
+from pants.rules.core.lint import LintResult
+from pants.testutil.engine.util import MockGet, run_rule
+from pants.testutil.subsystem.util import global_subsystem_instance
+from pants.testutil.test_base import TestBase
+
+
+class TestPythonTestRunner(TestBase):
+
+  def test_noops_when_disabled(self) -> None:
+    black_subsystem = global_subsystem_instance(Black, {Black.options_scope: {"enable": False}})
+    target = FormattablePythonTarget(target=TargetAdaptor())
+    mock_black_setup = BlackSetup(
+      config_path=None,
+      merged_input_files=EMPTY_DIRECTORY_DIGEST,
+      resolved_requirements_pex=Pex(
+        directory_digest=EMPTY_DIRECTORY_DIGEST, output_filename="./fake.pex"
+      ),
+    )
+    rule_args = [
+      black_subsystem,
+      target,
+      mock_black_setup,
+      PythonSetup.global_instance(),
+      SubprocessEnvironment.global_instance(),
+    ]
+
+    fmt_result: FmtResult = run_rule(
+      fmt,
+      rule_args=rule_args,
+      mock_gets=[
+        MockGet(
+          product_type=ExecuteProcessResult,
+          subject_type=ExecuteProcessRequest,
+          mock=lambda _: ExecuteProcessResult(
+            output_directory_digest=EMPTY_DIRECTORY_DIGEST, stdout=b"bad", stderr=b"bad",
+          ),
+        )
+      ],
+    )
+
+    lint_result: LintResult = run_rule(
+      lint,
+      rule_args=rule_args,
+      mock_gets=[
+        MockGet(
+          product_type=FallibleExecuteProcessResult,
+          subject_type=ExecuteProcessRequest,
+          mock=lambda _: FallibleExecuteProcessResult(
+            stdout=b"bad",
+            stderr=b"bad",
+            exit_code=127,
+            output_directory_digest=EMPTY_DIRECTORY_DIGEST,
+          ),
+        ),
+      ],
+    )
+
+    self.assertEqual(fmt_result, FmtResult.noop())
+    self.assertEqual(lint_result, LintResult.noop())