Add whitelist of folders that must remain Python 2 compatible (pantsb…

…uild#7941)

Three of our folders may be run with Python 2, so they must keep their compatibility, e.g. `from __future__` imports.

If this impacted more code, we would want to create (restore) formal linters to enforce the compatibility. However, because the code is so small and CI will fail upon dropping Py2 support, we simply add comments to the files explaining this expectation and modify `check_headers.py` to allow these files to not use Py3 headers.

In the process, we refactor `check_headers.py` to use the `pathlib` module for much nicer path handling.

build-support/bin/check_header.py

@@ -4,10 +4,10 @@
 
 import argparse
 import datetime
-import os
 import re
+from pathlib import Path
 from textwrap import dedent
-from typing import Iterable, List
+from typing import List, Sequence
 
 from common import die
 
@@ -20,8 +20,14 @@
 
 EXPECTED_NUM_LINES = 3
 
-_current_year = str(datetime.datetime.now().year)
-_current_century_regex = re.compile(r'20\d\d')
+CURRENT_YEAR = str(datetime.datetime.now().year)
+CURRENT_CENTURY_REGEX = re.compile(r'20\d\d')
+
+PY2_DIRECTORIES = {
+  Path("contrib/python/src/python/pants/contrib/python/checks/checker"),
+  Path("src/python/pants/backend/python/tasks/coverage"),
+  Path("src/python/pants/backend/python/tasks/pytest"),
+}
 
 
 class HeaderCheckFailure(Exception):
@@ -32,7 +38,8 @@ def main() -> None:
   args = create_parser().parse_args()
   header_parse_failures = []
   for directory in args.dirs:
-    header_parse_failures.extend(check_dir(directory, args.files_added))
+    directory_failures = check_dir(directory=directory, newly_created_files=args.files_added)
+    header_parse_failures.extend(directory_failures)
   if header_parse_failures:
     failures = '\n  '.join(str(failure) for failure in header_parse_failures)
     die(f"""\
@@ -47,79 +54,77 @@ def main() -> None:
 
 def create_parser() -> argparse.ArgumentParser:
   parser = argparse.ArgumentParser(
-    description="Check that all .py files start with the appropriate header."
+    description="Check that all .py files start with the appropriate header.",
   )
-  parser.add_argument("dirs", nargs="+",
-    help="The directories to check. Will recursively check subdirectories."
+  parser.add_argument("dirs", nargs="+", type=Path,
+    help="The directories to check. Will recursively check subdirectories.",
   )
-  parser.add_argument("-a", "--files-added", nargs="*", default=[],
+  parser.add_argument("-a", "--files-added", nargs="*", default=[], type=Path,
     help="Any passed files will be checked for a current copyright year."
   )
   return parser
 
 
-def check_header(filename: str, *, is_newly_created: bool = False) -> None:
+def check_dir(*, directory: Path, newly_created_files: Sequence[Path]) -> List[HeaderCheckFailure]:
+  header_parse_failures: List[HeaderCheckFailure] = []
+  for fp in directory.rglob("*.py"):
+    if fp.name == '__init__.py' or fp.parent in PY2_DIRECTORIES:
+      continue
+    try:
+      check_header(fp, is_newly_created=fp in newly_created_files)
+    except HeaderCheckFailure as e:
+      header_parse_failures.append(e)
+  return header_parse_failures
+
+
+def check_header(file_path: Path, *, is_newly_created: bool = False) -> None:
   """Raises `HeaderCheckFailure` if the header doesn't match."""
-  lines = get_header_lines(filename)
-  check_header_present(filename, lines)
+  lines = get_header_lines(file_path)
+  check_header_present(file_path, lines)
   check_copyright_year(
-    filename, copyright_line=lines[0], is_newly_created=is_newly_created
+    file_path, copyright_line=lines[0], is_newly_created=is_newly_created
   )
-  check_matches_header(filename, lines)
+  check_matches_header(file_path, lines)
 
 
-def get_header_lines(filename: str) -> List[str]:
+def get_header_lines(file_path: Path) -> List[str]:
   try:
-    with open(filename, 'r') as f:
+    with file_path.open() as f:
       # We grab an extra line in case there is a shebang.
       lines = [f.readline() for _ in range(0, EXPECTED_NUM_LINES + 1)]
-  except IOError as e:
-    raise HeaderCheckFailure(f"{filename}: error while reading input ({e})")
+  except OSError as e:
+    raise HeaderCheckFailure(f"{file_path}: error while reading input ({e})")
   # If a shebang line is included, remove it. Otherwise, we will have conservatively grabbed
   # one extra line at the end for the shebang case that is no longer necessary.
   lines.pop(0 if lines[0].startswith("#!") else - 1)
   return lines
 
 
-def check_header_present(filename: str, lines: List[str]) -> None:
+def check_header_present(file_path: Path, lines: List[str]) -> None:
   num_nonempty_lines = len([line for line in lines if line])
   if num_nonempty_lines < EXPECTED_NUM_LINES:
-    raise HeaderCheckFailure(f"{filename}: missing the expected header")
+    raise HeaderCheckFailure(f"{file_path}: missing the expected header")
 
 
-def check_copyright_year(filename: str, *, copyright_line: str, is_newly_created: bool) -> None:
+def check_copyright_year(file_path: Path, *, copyright_line: str, is_newly_created: bool) -> None:
   """Check that copyright is current year if for a new file, else that it's within
   the current century."""
   year = copyright_line[12:16]
-  if is_newly_created and year != _current_year:
-    raise HeaderCheckFailure(f'{filename}: copyright year must be {_current_year} (was {year})')
-  elif not _current_century_regex.match(year):
+  if is_newly_created and year != CURRENT_YEAR:
+    raise HeaderCheckFailure(f'{file_path}: copyright year must be {CURRENT_YEAR} (was {year})')
+  elif not CURRENT_CENTURY_REGEX.match(year):
     raise HeaderCheckFailure(
-      f"{filename}: copyright year must match '{_current_century_regex.pattern}' (was {year}): " +
-      f"current year is {_current_year}"
+      f"{file_path}: copyright year must match '{CURRENT_CENTURY_REGEX.pattern}' (was {year}): " +
+      f"current year is {CURRENT_YEAR}"
     )
 
 
-def check_matches_header(filename: str, lines: List[str]) -> None:
+def check_matches_header(file_path: Path, lines: List[str]) -> None:
   copyright_line = lines[0]
   sanitized_lines = lines.copy()
   sanitized_lines[0] = "# Copyright YYYY" + copyright_line[16:]
   if "".join(sanitized_lines) != EXPECTED_HEADER:
-    raise HeaderCheckFailure(f"{filename}: header does not match the expected header")
-
-
-def check_dir(directory: str, newly_created_files: Iterable[str]) -> List[HeaderCheckFailure]:
-  header_parse_failures: List[HeaderCheckFailure] = []
-  for root, dirs, files in os.walk(directory):
-    for f in files:
-      if not f.endswith('.py') or os.path.basename(f) == '__init__.py' or root.endswith('contrib/python/checks/checker'):
-        continue
-      filename = os.path.join(root, f)
-      try:
-        check_header(filename, is_newly_created=filename in newly_created_files)
-      except HeaderCheckFailure as e:
-        header_parse_failures.append(e)
-  return header_parse_failures
+    raise HeaderCheckFailure(f"{file_path}: header does not match the expected header")
 
 
 if __name__ == '__main__':

src/python/pants/backend/python/tasks/coverage/plugin.py

@@ -1,6 +1,11 @@
+# coding=utf-8
 # Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+# NB: This file must keep Python 2 support because it is a resource that may be run with Python 2.
+
 import json
 import os
 

src/python/pants/backend/python/tasks/pytest/plugin.py

@@ -1,6 +1,11 @@
+# coding=utf-8
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from __future__ import absolute_import, division, print_function, unicode_literals
+
+# NB: This file must keep Python 2 support because it is a resource that may be run with Python 2.
+
 import json
 import os