Skip to content

Commit 2227fc4

Browse files
committed
test: minor fixes & improvements for files linter test
Updates the lint-files.py lint test: * Use a context manager when opening files, so that files are closed. * Use the -z flag when shelling out to git ls-files so that we can catch newlines and other weird control characters in filenames
1 parent 1fcf66a commit 2227fc4

File tree

1 file changed

+12
-16
lines changed

1 file changed

+12
-16
lines changed

test/lint/lint-files.py

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
from subprocess import check_output
1414
from typing import Optional, NoReturn
1515

16-
CMD_ALL_FILES = "git ls-files --full-name"
17-
CMD_SOURCE_FILES = 'git ls-files --full-name -- "*.[cC][pP][pP]" "*.[hH]" "*.[pP][yY]" "*.[sS][hH]"'
16+
CMD_ALL_FILES = "git ls-files -z --full-name"
17+
CMD_SOURCE_FILES = 'git ls-files -z --full-name -- "*.[cC][pP][pP]" "*.[hH]" "*.[pP][yY]" "*.[sS][hH]"'
1818
CMD_SHEBANG_FILES = "git grep --full-name --line-number -I '^#!'"
1919
ALLOWED_FILENAME_REGEXP = "^[a-zA-Z0-9/_.@][a-zA-Z0-9/_.@-]*$"
2020
ALLOWED_SOURCE_FILENAME_REGEXP = "^[a-z0-9_./-]+$"
@@ -72,16 +72,13 @@ def check_all_filenames() -> int:
7272
Checks every file in the repository against an allowed regexp to make sure only lowercase or uppercase
7373
alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.
7474
"""
75-
# We avoid using rstrip() to ensure we catch filenames which accidentally include trailing whitespace
76-
filenames = check_output(CMD_ALL_FILES, shell=True).decode("utf8").split("\n")
77-
filenames = [filename for filename in filenames if filename != ""] # removes the trailing empty list element
78-
75+
filenames = check_output(CMD_ALL_FILES, shell=True).decode("utf8").rstrip("\0").split("\0")
7976
filename_regex = re.compile(ALLOWED_FILENAME_REGEXP)
8077
failed_tests = 0
8178
for filename in filenames:
8279
if not filename_regex.match(filename):
8380
print(
84-
f"""File "{filename}" does not not match the allowed filename regexp ('{ALLOWED_FILENAME_REGEXP}')."""
81+
f"""File {repr(filename)} does not not match the allowed filename regexp ('{ALLOWED_FILENAME_REGEXP}')."""
8582
)
8683
failed_tests += 1
8784
return failed_tests
@@ -94,17 +91,14 @@ def check_source_filenames() -> int:
9491
9592
Additionally there is an exception regexp for directories or files which are excepted from matching this regexp.
9693
"""
97-
# We avoid using rstrip() to ensure we catch filenames which accidentally include trailing whitespace
98-
filenames = check_output(CMD_SOURCE_FILES, shell=True).decode("utf8").split("\n")
99-
filenames = [filename for filename in filenames if filename != ""] # removes the trailing empty list element
100-
94+
filenames = check_output(CMD_SOURCE_FILES, shell=True).decode("utf8").rstrip("\0").split("\0")
10195
filename_regex = re.compile(ALLOWED_SOURCE_FILENAME_REGEXP)
10296
filename_exception_regex = re.compile(ALLOWED_SOURCE_FILENAME_EXCEPTION_REGEXP)
10397
failed_tests = 0
10498
for filename in filenames:
10599
if not filename_regex.match(filename) and not filename_exception_regex.match(filename):
106100
print(
107-
f"""File "{filename}" does not not match the allowed source filename regexp ('{ALLOWED_SOURCE_FILENAME_REGEXP}'), or the exception regexp ({ALLOWED_SOURCE_FILENAME_EXCEPTION_REGEXP})."""
101+
f"""File {repr(filename)} does not not match the allowed source filename regexp ('{ALLOWED_SOURCE_FILENAME_REGEXP}'), or the exception regexp ({ALLOWED_SOURCE_FILENAME_EXCEPTION_REGEXP})."""
108102
)
109103
failed_tests += 1
110104
return failed_tests
@@ -116,15 +110,16 @@ def check_all_file_permissions() -> int:
116110
117111
Additionally checks that for executable files, the file contains a shebang line
118112
"""
119-
filenames = check_output(CMD_ALL_FILES, shell=True).decode("utf8").strip().split("\n")
113+
filenames = check_output(CMD_ALL_FILES, shell=True).decode("utf8").rstrip("\0").split("\0")
120114
failed_tests = 0
121115
for filename in filenames:
122116
file_meta = FileMeta(filename)
123117
if file_meta.permissions == ALLOWED_PERMISSION_EXECUTABLES:
124-
shebang = open(filename, "rb").readline().rstrip(b"\n")
118+
with open(filename, "rb") as f:
119+
shebang = f.readline().rstrip(b"\n")
125120

126121
# For any file with executable permissions the first line must contain a shebang
127-
if shebang[:2] != b"#!":
122+
if not shebang.startswith(b"#!"):
128123
print(
129124
f"""File "{filename}" has permission {ALLOWED_PERMISSION_EXECUTABLES} (executable) and is thus expected to contain a shebang '#!'. Add shebang or do "chmod {ALLOWED_PERMISSION_NON_EXECUTABLES} {filename}" to make it non-executable."""
130125
)
@@ -176,7 +171,8 @@ def check_shebang_file_permissions() -> int:
176171

177172
# *.py files which don't contain an `if __name__ == '__main__'` are not expected to be executed directly
178173
if file_meta.extension == "py":
179-
file_data = open(filename, "r", encoding="utf8").read()
174+
with open(filename, "r", encoding="utf8") as f:
175+
file_data = f.read()
180176
if not re.search("""if __name__ == ['"]__main__['"]:""", file_data):
181177
continue
182178

0 commit comments

Comments
 (0)