Restore pycodestyle (pantsbuild#8471)

### Problem

As part of the effort to migrate the codebase to black, pycodestyle was disabled under the assumption that the code would soon be formatted with black, making pycodestyle unnecessary. (See pantsbuild@bdd2b9e)

Migrating to Black has taken longer than expected, so disabling pycodestyle has started to build up technical debt.

### Solution

The reason pycodestyle formatting was a problem at the time was because unit tests for Black used existing files in our codebase, and some of those needed to be formatted consistently with how black would format them.

Instead, craft temporary files on demand to test Black's functionality.

Also, pay-off the technical debt that accumulated since disabling pycodestyle by fixing all linting errors that crept up.

### Result

Python linting is back, the codebase lints cleanly and Black tests don't rely on indentation of python files in our codebase anymore.

contrib/python/tests/python/pants_test/contrib/python/checks/checker/test_pyflakes.py

@@ -25,10 +25,6 @@ def test_pyflakes_unused_import(self):
       # rather than with Pants.
       self.assertNit('import os', 'F401', expected_line_number='001-002')
 
-  def test_pyflakes_ignore(self):
-    plugin = self.get_plugin('import os', ignore=['F401'])
-    self.assertEqual([], list(plugin.nits()))
-
   def test_pyflakes_ignore(self):
     plugin = self.get_plugin('import os', ignore=['UnusedImport'])
     self.assertEqual([], list(plugin.nits()))

examples/src/python/example/hello/greet/greet.py

@@ -5,5 +5,5 @@
 
 
 def greet(greetee):
-    """Given the name, return a greeting for a person of that name."""
-    return green("Hello, {}!".format(greetee))
+  """Given the name, return a greeting for a person of that name."""
+  return green("Hello, {}!".format(greetee))

examples/src/python/example/hello/main/main.py

@@ -7,6 +7,6 @@
 
 
 if __name__ == "__main__":
-    greetees = sys.argv[1:] or ["world"]
-    for greetee in greetees:
-        print(greet(greetee))
+  greetees = sys.argv[1:] or ["world"]
+  for greetee in greetees:
+    print(greet(greetee))

pants.ini

@@ -260,9 +260,6 @@ skip: True
 # --closure option is removed. See comment in the task code for details.
 transitive: True
 
-[lint.pythonstyle]
-skip: True
-
 [lint.scalafmt]
 skip: True
 

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

@@ -15,16 +15,11 @@
   DirectoriesToMerge,
   DirectoryWithPrefixToAdd,
 )
-from pants.engine.isolated_process import (
-  ExecuteProcessRequest,
-  ExecuteProcessResult,
-  MultiPlatformExecuteProcessRequest,
-)
+from pants.engine.isolated_process import ExecuteProcessResult, MultiPlatformExecuteProcessRequest
 from pants.engine.legacy.structs import PythonTargetAdaptor, TargetAdaptor
 from pants.engine.platform import Platform, PlatformConstraint
-from pants.engine.rules import RootRule, optionable_rule, rule
+from pants.engine.rules import optionable_rule, rule
 from pants.engine.selectors import Get
-from pants.util.strutil import create_path_env_var
 
 
 @dataclass(frozen=True)
@@ -46,6 +41,7 @@ def create_from_adaptors(cls, adaptors: Tuple[TargetAdaptor, ...], additional_re
       all_target_requirements.extend(additional_requirements)
     return PexRequirements(requirements=tuple(sorted(all_target_requirements)))
 
+
 @dataclass(frozen=True)
 class PexInterpreterContraints:
   constraint_set: FrozenSet[str] = frozenset()

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

@@ -6,7 +6,6 @@
 from pants.backend.python.subsystems.python_setup import PythonSetup
 from pants.backend.python.targets.python_binary import PythonBinary
 from pants.engine.fs import Digest, DirectoriesToMerge
-from pants.engine.isolated_process import ExecuteProcessRequest, FallibleExecuteProcessResult
 from pants.engine.legacy.graph import BuildFileAddresses, HydratedTarget, TransitiveHydratedTargets
 from pants.engine.legacy.structs import PythonBinaryAdaptor
 from pants.engine.rules import UnionRule, rule

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

@@ -29,7 +29,7 @@
 # containing python files so we can have one single type used as an input in the run_black rule.
 @dataclass(frozen=True)
 class FormattablePythonTarget:
-    target: Any
+  target: Any
 
 
 @rule
@@ -93,6 +93,7 @@ def run_black(
     stderr=result.stderr.decode(),
   )
 
+
 # TODO: remove this workaround once https://github.com/pantsbuild/pants/issues/8343 is addressed
 @rule
 def target_adaptor(target: PythonTargetAdaptor) -> FormattablePythonTarget:

src/python/pants/engine/rules.py

@@ -511,7 +511,7 @@ def __str__(self):
 
   @property
   def output_type(self):
-      return self._output_type
+    return self._output_type
 
   @property
   def dependency_rules(self):

src/python/pants/rules/core/binary.py

@@ -2,17 +2,14 @@
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
 from dataclasses import dataclass
-from typing import Any
 
-from pants.backend.python.rules.pex import Pex
 from pants.build_graph.address import Address
 from pants.engine.addressable import BuildFileAddresses
 from pants.engine.console import Console
 from pants.engine.fs import Digest, DirectoryToMaterialize, Workspace
 from pants.engine.goal import Goal, LineOriented
 from pants.engine.legacy.graph import HydratedTarget
-from pants.engine.legacy.structs import PythonBinaryAdaptor
-from pants.engine.rules import console_rule, optionable_rule, rule, union
+from pants.engine.rules import console_rule, rule, union
 from pants.engine.selectors import Get
 
 

src/python/pants/scm/git.py

@@ -12,7 +12,7 @@
 from pants.scm.scm import Scm
 from pants.util.contextutil import pushd
 from pants.util.memo import memoized_method
-from pants.util.strutil import ensure_binary, ensure_text
+from pants.util.strutil import ensure_text
 
 
 # 40 is Linux's hard-coded limit for total symlinks followed when resolving a path.

src/python/pants/util/objects.py

@@ -1,13 +1,10 @@
 # Copyright 2016 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-import re
 from abc import ABC, abstractmethod
 from collections import OrderedDict, namedtuple
 from collections.abc import Iterable
 
-from twitter.common.collections import OrderedSet
-
 from pants.util.memo import memoized_classproperty
 from pants.util.meta import classproperty
 from pants.util.strutil import pluralize

testprojects/src/python/python_distribution/ctypes/ctypes_python_pkg/ctypes_wrapper.py

@@ -19,6 +19,7 @@ def get_generated_shared_lib(lib_name):
 asdf_c_lib = ctypes.CDLL(asdf_c_lib_path)
 asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path)
 
+
 def f(x):
   added = asdf_c_lib.add_three(x)
   multiplied = asdf_cpp_lib.multiply_by_three(added)

testprojects/src/python/python_distribution/ctypes_with_extra_compiler_flags/ctypes_python_pkg/ctypes_wrapper.py

@@ -16,6 +16,7 @@ def get_generated_shared_lib(lib_name):
 asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp_ctypes-with-extra-compiler-flags')
 asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path)
 
+
 def f(x):
   multiplied = asdf_cpp_lib.multiply_by_something(42)
   return multiplied

testprojects/src/python/python_distribution/ctypes_with_third_party/ctypes_python_pkg/ctypes_wrapper.py

@@ -16,6 +16,7 @@ def get_generated_shared_lib(lib_name):
 asdf_cpp_lib_path = get_generated_shared_lib('asdf-cpp_ctypes-with-third-party')
 asdf_cpp_lib = ctypes.CDLL(asdf_cpp_lib_path)
 
+
 def f(x):
   added = x + 3
   multiplied = asdf_cpp_lib.multiply_by_three(added)

testprojects/src/python/python_distribution/setup_requires/setup.py

@@ -1,7 +1,6 @@
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-import os
 from pathlib import Path
 from setuptools import find_packages, setup
 

testprojects/src/python/python_distribution/setup_requires/test_setup_requires.py

@@ -1,8 +1,6 @@
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-import re
-
 from checksum import checksum
 
 

tests/python/pants_test/backend/python/rules/test_python_fmt_integration.py

@@ -1,36 +1,74 @@
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+import os
+from contextlib import contextmanager
 from os.path import relpath
 from pathlib import Path
 
-from pants.util.contextutil import temporary_file, temporary_file_path
-from pants_test.pants_run_integration_test import PantsRunIntegrationTest, ensure_daemon
+from pants.util.contextutil import temporary_dir, temporary_file_path
+from pants.util.dirutil import safe_file_dump
+from pants_test.pants_run_integration_test import PantsRunIntegrationTest
+
+
+def write_build_file(root_dir):
+  safe_file_dump(os.path.join(root_dir, "BUILD"), "python_library()")
+
+INCONSISTENTLY_FORMATTED_HELLO: str = "def hello():x=42"
+CONSISTENTLY_FORMATTED_HELLO: str = "def hello():\n    x = 42\n"
+
+
+def write_inconsistently_formatted_file(root_dir, filename) -> Path:
+  filepath = os.path.join(root_dir, filename)
+  safe_file_dump(filepath, INCONSISTENTLY_FORMATTED_HELLO)
+  return Path(filepath)
+
+
+def write_consistently_formatted_file(root_dir, filename) -> Path:
+  filepath = os.path.join(root_dir, filename)
+  safe_file_dump(filepath, CONSISTENTLY_FORMATTED_HELLO)
+  return Path(filepath)
+
+
+@contextmanager
+def build_directory():
+  with temporary_dir(root_dir=".") as root_dir:
+    write_build_file(root_dir)
+    yield root_dir
 
 
 class PythonFmtIntegrationTest(PantsRunIntegrationTest):
   def test_black_one_python_source_should_leave_one_file_unchanged(self):
-    command = [
-      'fmt-v2',
-      'examples/src/python/example/hello/main:main'
+    with build_directory() as root_dir:
+      code = write_consistently_formatted_file(root_dir, "hello.py")
+      command = [
+        'fmt-v2',
+        f'{root_dir}'
       ]
-    pants_run = self.run_pants(command=command)
-    self.assert_success(pants_run)
+      pants_run = self.run_pants(command=command)
+      self.assert_success(pants_run)
+      after_formatting = code.read_text()
+      self.assertEqual(CONSISTENTLY_FORMATTED_HELLO, after_formatting)
     self.assertNotIn("reformatted", pants_run.stderr_data)
     self.assertIn("1 file left unchanged", pants_run.stderr_data)
 
-
   def test_black_two_python_sources_should_leave_two_files_unchanged(self):
-    command = [
-      'fmt-v2',
-      'examples/src/python/example/hello/greet:greet'
+    with build_directory() as root_dir:
+      foo = write_consistently_formatted_file(root_dir, "foo.py")
+      bar = write_consistently_formatted_file(root_dir, "bar.py")
+      command = [
+        'fmt-v2',
+        f'{root_dir}'
       ]
-    pants_run = self.run_pants(command=command)
-    self.assert_success(pants_run)
+      pants_run = self.run_pants(command=command)
+      self.assert_success(pants_run)
+      foo_after_formatting = foo.read_text()
+      self.assertEqual(CONSISTENTLY_FORMATTED_HELLO, foo_after_formatting)
+      bar_after_formatting = bar.read_text()
+      self.assertEqual(CONSISTENTLY_FORMATTED_HELLO, bar_after_formatting)
     self.assertNotIn("reformatted", pants_run.stderr_data)
     self.assertIn("2 files left unchanged", pants_run.stderr_data)
 
-
   def test_black_should_pickup_default_config(self):
     # If the default config (pyproject.toml is picked up, the compilation_failure target will be skipped
     command = [
@@ -43,7 +81,6 @@ def test_black_should_pickup_default_config(self):
     self.assertNotIn("unchanged", pants_run.stderr_data)
     self.assertIn("Nothing to do", pants_run.stderr_data)
 
-
   def test_black_should_pickup_non_default_config(self):
     # If a valid toml file without a black configuration section is picked up,
     # Black won't skip the compilation_failure and will fail
@@ -59,20 +96,16 @@ def test_black_should_pickup_non_default_config(self):
     self.assertNotIn("unchanged", pants_run.stderr_data)
     self.assertIn("1 file failed to reformat", pants_run.stderr_data)
 
-
   def test_black_should_format_python_code(self):
-    # Open file in the greet target as the BUILD file globs python files from there
-    with temporary_file(root_dir="./examples/src/python/example/hello/greet/", suffix=".py") as code:
-      file_name = code.name
-      code.write(b"x     = 42")
-      code.close()
+    with build_directory() as root_dir:
+      code = write_inconsistently_formatted_file(root_dir, "hello.py")
       command = [
         'fmt-v2',
-        'examples/src/python/example/hello/greet:greet'
-        ]
+        f'{root_dir}'
+      ]
       pants_run = self.run_pants(command=command)
-      formatted = Path(file_name).read_text();
-      self.assertEqual("x = 42\n", formatted)
-    self.assert_success(pants_run)
+      self.assert_success(pants_run)
+      after_formatting = code.read_text()
+      self.assertEqual(CONSISTENTLY_FORMATTED_HELLO, after_formatting)
     self.assertIn("1 file reformatted", pants_run.stderr_data)
-    self.assertIn("2 files left unchanged", pants_run.stderr_data)
+    self.assertNotIn("unchanged", pants_run.stderr_data)

tests/python/pants_test/base/test_hash_utils.py

@@ -187,6 +187,7 @@ class Test(Enum):
       pig = 2
     self.assertEqual(self._coercing_json_encode([Test.dog, Test.cat, Test.pig]), '[0, 1, 2]')
 
+
 class JsonHashingTest(unittest.TestCase):
 
   def test_known_checksums(self):

tests/python/pants_test/task_test_base.py

@@ -335,7 +335,7 @@ class TaskInvocationResult:
     this_task: Task
     after_tasks: Tuple[Task, ...]
 
-  def invoke_tasks(self, target_closure=None, **context_kwargs) -> TaskInvocationResult:
+  def invoke_tasks(self, target_closure=None, **context_kwargs) -> 'TaskInvocationResult':
     """Create and execute the declaratively specified tasks in order.
 
     Create instances of and execute task types in `self.run_before_task_types()`, then

tests/python/pants_test/util/test_objects.py

@@ -323,6 +323,7 @@ class ReturnsNotImplemented:
   def __eq__(self, other):
     return NotImplemented
 
+
 class DatatypeTest(TestBase):
 
   def test_eq_with_not_implemented_super(self):