Use Black internally for auto-formatting (pantsbuild#9157)

The Pants committers recently decided to use Black for automated formatting of the project. While we do not agree with all of its choices—particularly 4 space indentation—we decided the tool brings more benefits than harm.

### Manually fixing multiline strings in this PR
Black does not attempt to handle multiline strings because it could not safely do this without changing the AST. (Fortunately, docformatter fixes the majority of instances.)

We must manually fix all bad usages of triple quoted strings. To do this, we first ran [this script](https://gist.github.com/Eric-Arellano/4680bd4a6facaa52373d42ca0213220e) to find all instances of multiline strings. Then, we tracked this in https://docs.google.com/spreadsheets/d/1cL5hmWjHfoCdbdThgOxAmCBDJhp5NelmUkYgidXIwGw/edit#gid=0 and manually checked each of the 720 files. 329/720 files needed fixes.

### How to resolve merge conflicts
1. `git pull origin master` (do not rebase)
2. For any merge conflict:
    1. **Unconditionally accept your original version**. Do not try to manually fix merge conflicts.
    2. `./pants fmt2 path/to/merge_conflict.py`.
    3. Open the file and search for `"""`. Ensure that all multiline strings are properly formatted.

.isort.cfg

@@ -6,9 +6,8 @@ line_length=100
 multi_line_output=3
 include_trailing_comma=True
 force_grid_wrap=0
-use_parentheses=true
-indent=2
-lines_after_imports=2
+use_parentheses=True
+
 known_first_party=internal_backend,pants,pants_test
 default_section=THIRDPARTY
 not_skip=__init__.py

.travis.yml

@@ -381,9 +381,10 @@ matrix:
     - '3.6'
     - '3.7'
     script:
-    - travis-wait-enhanced --timeout 40m --interval 9m -- ./build-support/bin/ci.py
-      --remote-execution-enabled --githooks --sanity-checks --doc-gen --lint --python-version
-      3.6
+    - travis-wait-enhanced --timeout 60m --interval 9m -- ./build-support/bin/ci.py
+      --githooks --sanity-checks --doc-gen --python-version 3.6
+    - travis-wait-enhanced --timeout 25m --interval 9m -- ./build-support/bin/ci.py
+      --remote-execution-enabled --lint --python-version 3.6
     stage: Test Pants
     sudo: required
   - addons:
@@ -443,9 +444,10 @@ matrix:
     - '3.6'
     - '3.7'
     script:
-    - travis-wait-enhanced --timeout 40m --interval 9m -- ./build-support/bin/ci.py
-      --remote-execution-enabled --githooks --sanity-checks --doc-gen --lint --python-version
-      3.7
+    - travis-wait-enhanced --timeout 60m --interval 9m -- ./build-support/bin/ci.py
+      --githooks --sanity-checks --doc-gen --python-version 3.7
+    - travis-wait-enhanced --timeout 25m --interval 9m -- ./build-support/bin/ci.py
+      --remote-execution-enabled --lint --python-version 3.7
     stage: Test Pants (Cron)
     sudo: required
   - before_cache:

build-support/bin/BUILD

@@ -14,12 +14,6 @@ files(
   sources = ['*.py'],
 )
 
-python_binary(
-  name = 'black',
-  source = 'black.py',
-  tags = {'type_checked'},
-)
-
 python_binary(
   name = 'check_banned_imports',
   source = 'check_banned_imports.py',

build-support/bin/check_banned_imports.py

@@ -10,45 +10,45 @@
 
 
 def main() -> None:
-  rust_files = find_files("src/rust/engine", extension=".rs")
+    rust_files = find_files("src/rust/engine", extension=".rs")
 
-  check_banned_import(
-    rust_files,
-    bad_import_regex=r"^use std::sync::.*(Mutex|RwLock)",
-    correct_import_message="parking_lot::(Mutex|RwLock)"
-  )
+    check_banned_import(
+        rust_files,
+        bad_import_regex=r"^use std::sync::.*(Mutex|RwLock)",
+        correct_import_message="parking_lot::(Mutex|RwLock)",
+    )
 
 
 def find_files(*directories: str, extension: str) -> Set[str]:
-  return {
-    fp
-    for directory in directories
-    for fp in glob(f"{directory}/**/*{extension}", recursive=True)
-  }
+    return {
+        fp
+        for directory in directories
+        for fp in glob(f"{directory}/**/*{extension}", recursive=True)
+    }
 
 
 def filter_files(files: Iterable[str], *, snippet_regex: str) -> Set[str]:
-  """Only return files that contain the snippet_regex."""
-  regex = re.compile(snippet_regex)
-  result: Set[str] = set()
-  for fp in files:
-    with open(fp, 'r') as f:
-      if any(re.search(regex, line) for line in f.readlines()):
-        result.add(fp)
-  return result
+    """Only return files that contain the snippet_regex."""
+    regex = re.compile(snippet_regex)
+    result: Set[str] = set()
+    for fp in files:
+        with open(fp, "r") as f:
+            if any(re.search(regex, line) for line in f.readlines()):
+                result.add(fp)
+    return result
 
 
 def check_banned_import(
-  files: Iterable[str], *, bad_import_regex: str, correct_import_message: str
+    files: Iterable[str], *, bad_import_regex: str, correct_import_message: str
 ) -> None:
-  bad_files = filter_files(files, snippet_regex=bad_import_regex)
-  if bad_files:
-    bad_files_str = "\n".join(sorted(bad_files))
-    die(
-      f"Found forbidden imports matching `{bad_import_regex}`. Instead, you should use "
-      f"`{correct_import_message}`. Bad files:\n{bad_files_str}"
-    )
+    bad_files = filter_files(files, snippet_regex=bad_import_regex)
+    if bad_files:
+        bad_files_str = "\n".join(sorted(bad_files))
+        die(
+            f"Found forbidden imports matching `{bad_import_regex}`. Instead, you should use "
+            f"`{correct_import_message}`. Bad files:\n{bad_files_str}"
+        )
 
 
 if __name__ == "__main__":
-  main()
+    main()

build-support/bin/check_header.py

@@ -11,121 +11,130 @@
 
 from common import die
 
-
-EXPECTED_HEADER = dedent("""\
-  # Copyright YYYY Pants project contributors (see CONTRIBUTORS.md).
-  # Licensed under the Apache License, Version 2.0 (see LICENSE).
-
-  """)
+EXPECTED_HEADER = dedent(
+    """\
+    # Copyright YYYY Pants project contributors (see CONTRIBUTORS.md).
+    # Licensed under the Apache License, Version 2.0 (see LICENSE).
+    
+    """
+)
 
 EXPECTED_NUM_LINES = 3
 
 CURRENT_YEAR = str(datetime.datetime.now().year)
-CURRENT_CENTURY_REGEX = re.compile(r'20\d\d')
+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"),
+    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):
-  """This is only used for control flow and to propagate the `.message` field."""
+    """This is only used for control flow and to propagate the `.message` field."""
 
 
 def main() -> None:
-  args = create_parser().parse_args()
-  header_parse_failures = []
-  for directory in args.dirs:
-    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"""\
+    args = create_parser().parse_args()
+    header_parse_failures = []
+    for directory in args.dirs:
+        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"""\
 ERROR: All .py files other than __init__.py should start with the header:
 {EXPECTED_HEADER}
 
 ---
 
 The following {len(header_parse_failures)} file(s) do not conform:
-{failures}""")
+{failures}"""
+        )
 
 
 def create_parser() -> argparse.ArgumentParser:
-  parser = argparse.ArgumentParser(
-    description="Check that all .py files start with the appropriate header.",
-  )
-  parser.add_argument("dirs", nargs="+", type=Path,
-    help="The directories to check. Will recursively check subdirectories.",
-  )
-  parser.add_argument("-a", "--files-added", nargs="*", default=[], type=Path,
-    help="Any passed files will be checked for a current copyright year."
-  )
-  return parser
+    parser = argparse.ArgumentParser(
+        description="Check that all .py files start with the appropriate header.",
+    )
+    parser.add_argument(
+        "dirs",
+        nargs="+",
+        type=Path,
+        help="The directories to check. Will recursively check subdirectories.",
+    )
+    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_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
+    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(file_path)
-  check_header_present(file_path, lines)
-  check_copyright_year(
-    file_path, copyright_line=lines[0], is_newly_created=is_newly_created
-  )
-  check_matches_header(file_path, lines)
+    """Raises `HeaderCheckFailure` if the header doesn't match."""
+    lines = get_header_lines(file_path)
+    check_header_present(file_path, lines)
+    check_copyright_year(file_path, copyright_line=lines[0], is_newly_created=is_newly_created)
+    check_matches_header(file_path, lines)
 
 
 def get_header_lines(file_path: Path) -> List[str]:
-  try:
-    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 OSError as e:
-    raise HeaderCheckFailure(f"{file_path}: error while reading input ({e!r})")
-  # 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
+    try:
+        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 OSError as e:
+        raise HeaderCheckFailure(f"{file_path}: error while reading input ({e!r})")
+    # 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(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"{file_path}: missing the expected header")
+    num_nonempty_lines = len([line for line in lines if line])
+    if num_nonempty_lines < EXPECTED_NUM_LINES:
+        raise HeaderCheckFailure(f"{file_path}: missing the expected header")
 
 
 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'{file_path}: copyright year must be {CURRENT_YEAR} (was {year})')
-  elif not CURRENT_CENTURY_REGEX.match(year):
-    raise HeaderCheckFailure(
-      f"{file_path}: copyright year must match '{CURRENT_CENTURY_REGEX.pattern}' (was {year}): " +
-      f"current year is {CURRENT_YEAR}"
-    )
+    """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"{file_path}: copyright year must be {CURRENT_YEAR} (was {year})")
+    elif not CURRENT_CENTURY_REGEX.match(year):
+        raise HeaderCheckFailure(
+            f"{file_path}: copyright year must match '{CURRENT_CENTURY_REGEX.pattern}' (was {year}): "
+            + f"current year is {CURRENT_YEAR}"
+        )
 
 
 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"{file_path}: header does not match the expected header")
+    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"{file_path}: header does not match the expected header")
 
 
-if __name__ == '__main__':
-  main()
+if __name__ == "__main__":
+    main()