Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support in `check_header.py` for Python 3-style headers #7713

Merged
merged 4 commits into from May 14, 2019
Merged
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -12,7 +12,7 @@
from common import die


EXPECTED_HEADER = dedent("""\
EXPECTED_HEADER_PY2 = dedent("""\
# coding=utf-8
# Copyright YYYY Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).
@@ -21,6 +21,12 @@
""")

EXPECTED_HEADER_PY3 = dedent("""\
# Copyright YYYY Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).
""")


_current_year = str(datetime.datetime.now().year)
_current_century_regex = re.compile(r'20\d\d')
@@ -38,9 +44,12 @@ def main() -> None:
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 following header:
ERROR: All .py files other than __init__.py should start with the header:
{EXPECTED_HEADER_PY3}
If they must support Python 2 still, they should start with the header:
{EXPECTED_HEADER_PY2}
{EXPECTED_HEADER}
---
The following {len(header_parse_failures)} file(s) do not conform:
@@ -62,21 +71,28 @@ def create_parser() -> argparse.ArgumentParser:

def check_header(filename: str, *, is_newly_created: bool = False) -> None:
"""Raises `HeaderCheckFailure` if the header doesn't match."""
expected_num_py2_lines = 6
expected_num_py3_lines = 3
try:
with open(filename, 'r') as f:
first_lines = [f.readline() for _ in range(0, 7)]
# We grab an extra line in case there is a shebang.
first_lines = [f.readline() for _ in range(0, expected_num_py2_lines + 1)]
except IOError as e:
raise HeaderCheckFailure(f"{filename}: 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.
first_lines.pop(0 if first_lines[0].startswith("#!") else - 1)
# Check that the first lines even exists. Note that first_lines will always have an entry
# for each line, even if the file is completely empty.
if len([line for line in first_lines if line]) < 4:
if len([line for line in first_lines if line]) < expected_num_py3_lines:
raise HeaderCheckFailure(f"{filename}: missing the expected header")
is_py3_file = all("from __future__" not in line for line in first_lines)
if is_py3_file:
This conversation was marked as resolved by Eric-Arellano

This comment has been minimized.

Copy link
@blorente

blorente May 14, 2019

Contributor

I think I'd be more confident of the correctness of this script if it totally bifurcated at this point (i.e.

if is_py3_file:
  # end-to-end validation of Py3
else:
  # end-to-end validation of py23

MY worry is that one could craft a header such that it's not valid py2 or py3, but somehow passes if "".join(first_lines) not in (EXPECTED_HEADER_PY2, EXPECTED_HEADER_PY3). Now, I realize this is impossible with the code in this change, just musing through a concern :)

first_lines = first_lines[:expected_num_py3_lines]
# Check copyright year. If it's a new file, it should be the current year. Else, it should
# be within the current century.
copyright_line = first_lines[1]
copyright_line_index = 0 if is_py3_file else 1
copyright_line = first_lines[copyright_line_index]
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})')
@@ -86,8 +102,8 @@ def check_header(filename: str, *, is_newly_created: bool = False) -> None:
f"current year is {_current_year}"
)
# Replace copyright_line with sanitized year.
first_lines[1] = "# Copyright YYYY" + copyright_line[16:]
if "".join(first_lines) != EXPECTED_HEADER:
first_lines[copyright_line_index] = "# Copyright YYYY" + copyright_line[16:]
if "".join(first_lines) not in (EXPECTED_HEADER_PY2, EXPECTED_HEADER_PY3):
raise HeaderCheckFailure(f"{filename}: header does not match the expected header")


@@ -127,12 +127,9 @@ def assert_header_check(added_files, expected_excerpt):

# Check that a file with a typo in the header fails
safe_file_dump(new_py_path, dedent("""\
# coding=utf-8
# Copyright {} Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the MIT License, Version 3.3 (see LICENSE).
from __future__ import absolute_import, division, print_function, unicode_literals
""".format(cur_year))
)
assert_header_check(
@@ -142,12 +139,9 @@ def assert_header_check(added_files, expected_excerpt):

# Check that a file without a valid copyright year fails.
safe_file_dump(new_py_path, dedent("""\
# coding=utf-8
# Copyright YYYY 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
""")
)
assert_header_check(
@@ -161,12 +155,9 @@ def assert_header_check(added_files, expected_excerpt):
# Check that a newly added file must have the current year.
last_year = str(cur_year_num - 1)
safe_file_dump(new_py_path, dedent("""\
# coding=utf-8
# Copyright {} 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
""".format(last_year))
)
rel_new_py_path = os.path.relpath(new_py_path, worktree)
@@ -175,6 +166,18 @@ def assert_header_check(added_files, expected_excerpt):
expected_excerpt="subdir/file.py: copyright year must be {} (was {})".format(cur_year, last_year)
)

# Check that we also support Python 2-style headers.
safe_file_dump(new_py_path, dedent("""\
# coding=utf-8
# Copyright {} 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
""".format(cur_year))
)
self._assert_subprocess_success(worktree, [header_check_script, 'subdir'])

# Check that a file isn't checked against the current year if it is not passed as an
# arg to the script.
# Use the same file as last time, with last year's copyright date.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.