signalapp/Signal-iOS
signalapp/Signal-iOS
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases 4 Wiki Activity

Clean up precommit.py

Browse Source
This commit is contained in:
Max Radermacher 2024-04-15 14:02:27 -05:00 committed by GitHub
parent ef0ab38224
commit 74e66ecba2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 64 additions and 218 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.github/workflows/main.yml vendored
View File

@ -103,7 +103,7 @@ jobs:
- name: Lint files changed in the PR
run: |
python3 Scripts/precommit.py --ref origin/${{ github.base_ref }} --skip_license_header_checks
Scripts/precommit.py --ref origin/${{ github.base_ref }}
# https://help.github.com/en/actions/reference/development-tools-for-github-actions#logging-commands
git diff --name-only | sed -E 's|(.*)|::error file=\1::Incorrectly formatted (Scripts/precommit.py)|'

20
Scripts/lint/lint-license-headers
View File

@ -14,20 +14,6 @@ COPYRIGHT_LINE_RE = re.compile(r"^// Copyright 2\d\d\d Signal Messenger, LLC\n$"
SPDX_LINE = "// SPDX-License-Identifier: AGPL-3.0-only\n"
def git_ls_files() -> Iterable[Path]:
file_path_strs = subprocess.check_output(
["git", "ls-files"], text=True
).splitlines()
for file_path_str in file_path_strs:
yield Path(file_path_str)
def paths_to_process() -> Iterable[Path]:
for path in git_ls_files():
if path.suffix in EXTENSIONS_TO_CHECK:
yield path
def read_first_lines(line_count: int, path: Path) -> list[str]:
if line_count == 0:
return ""
@ -110,13 +96,15 @@ def main() -> None:
parser = argparse.ArgumentParser(
description="Check license headers across the project"
)
parser.add_argument("path", nargs="*", help="A path to process")
parser.add_argument(
"--fix", action="store_true", help="Attempt to auto-add license headers"
)
should_fix = parser.parse_args().fix
ns = parser.parse_args()
should_fix = ns.fix
all_good = True
for path in paths_to_process():
for path in ns.path:
if not has_valid_license_header(path):
if should_fix:
add_license_header_to(path)

260
Scripts/precommit.py
View File

@ -8,13 +8,10 @@ from typing import Iterable
from pathlib import Path
from lint.util import EXTENSIONS_TO_CHECK
git_repo_path = os.path.abspath(
subprocess.check_output(["git", "rev-parse", "--show-toplevel"], text=True).strip()
)
CLANG_FORMAT_EXTS = set([".m", ".mm", ".h"])
def sort_forward_decl_statement_block(text, filepath, filename, file_extension):
def sort_forward_decl_statement_block(text):
lines = text.split("\n")
lines = [line.strip() for line in lines if line.strip()]
lines = list(set(lines))
@ -36,13 +33,13 @@ def find_matching_section(text, match_test):
# Absorb any leading empty lines.
while first_matching_line_index > 0:
prev_line = lines[first_matching_line_index - 1]
if prev_line.strip():
if prev_line.strip() != "":
break
first_matching_line_index = first_matching_line_index - 1
first_non_matching_line_index = None
for index, line in enumerate(lines[first_matching_line_index:]):
if not line.strip():
if line.strip() == "":
# Absorb any trailing empty lines.
continue
if not match_test(line):
@ -62,22 +59,14 @@ def find_matching_section(text, match_test):
return text0, text1, text2
def sort_matching_blocks(
sort_name, filepath, filename, file_extension, text, match_func, sort_func
):
def sort_matching_blocks(sort_name, filepath, text, match_func, sort_func):
unprocessed = text
processed = None
while True:
section = find_matching_section(unprocessed, match_func)
# print '\t', 'sort_matching_blocks', section
if not section:
if processed:
processed = "\n".join(
(
processed,
unprocessed,
)
)
processed = "\n".join((processed, unprocessed))
else:
processed = unprocessed
break
@ -85,36 +74,12 @@ def sort_matching_blocks(
text0, text1, text2 = section
if processed:
processed = "\n".join(
(
processed,
text0,
)
)
processed = "\n".join((processed, text0))
else:
processed = text0
# print 'before:'
# temp_lines = text1.split('\n')
# for index, line in enumerate(temp_lines):
# if index < 3 or index + 3 >= len(temp_lines):
# print '\t', index, line
# # print text1
# print
text1 = sort_func(text1, filepath, filename, file_extension)
# print 'after:'
# # print text1
# temp_lines = text1.split('\n')
# for index, line in enumerate(temp_lines):
# if index < 3 or index + 3 >= len(temp_lines):
# print '\t', index, line
# print
processed = "\n".join(
(
processed,
text1,
)
)
text1 = sort_func(text1)
processed = "\n".join((processed, text1))
if text2:
unprocessed = text2
else:
@ -139,30 +104,24 @@ def find_forward_protocol_statement_section(text):
return find_matching_section(text, is_forward_protocol_statement)
def sort_forward_class_statements(filepath, filename, file_extension, text):
# print 'sort_class_statements', filepath
def sort_forward_class_statements(filepath, file_extension, text):
if file_extension not in (".h", ".m", ".mm"):
return text
return sort_matching_blocks(
"sort_class_statements",
filepath,
filename,
file_extension,
text,
find_forward_class_statement_section,
sort_forward_decl_statement_block,
)
def sort_forward_protocol_statements(filepath, filename, file_extension, text):
# print 'sort_class_statements', filepath
def sort_forward_protocol_statements(filepath, file_extension, text):
if file_extension not in (".h", ".m", ".mm"):
return text
return sort_matching_blocks(
"sort_forward_protocol_statements",
filepath,
filename,
file_extension,
text,
find_forward_protocol_statement_section,
sort_forward_decl_statement_block,
@ -174,46 +133,35 @@ def get_ext(file: str) -> str:
def process(filepath):
short_filepath = filepath[len(git_repo_path) :]
if short_filepath.startswith(os.sep):
short_filepath = short_filepath[len(os.sep) :]
filename = os.path.basename(filepath)
if filename.startswith("."):
raise Exception("shouldn't call process with dotfile")
file_ext = get_ext(filename)
file_ext = get_ext(filepath)
with open(filepath, "rt") as f:
text = f.read()
original_text = text
text = sort_forward_class_statements(filepath, filename, file_ext, text)
text = sort_forward_protocol_statements(filepath, filename, file_ext, text)
text = sort_forward_class_statements(filepath, file_ext, text)
text = sort_forward_protocol_statements(filepath, file_ext, text)
text = text.strip() + "\n"
if original_text == text:
return
print("Updating:", short_filepath)
with open(filepath, "wt") as f:
f.write(text)
def get_file_paths_in(path: str) -> Iterable[str]:
for rootdir, _, filenames in os.walk(path):
for filename in filenames:
yield os.path.abspath(os.path.join(rootdir, filename))
def get_file_paths_for_commands(commands: Iterable[list[str]]) -> Iterable[str]:
for command in commands:
lines = subprocess.check_output(command, text=True).split("\n")
for line in lines:
file_path = os.path.abspath(os.path.join(git_repo_path, line))
if os.path.exists(file_path):
yield file_path
def get_file_paths_for_commit(commit):
return (
subprocess.run(
["git", "diff", "--name-only", "--diff-filter=ACMR", commit],
check=True,
capture_output=True,
encoding="utf8",
)
.stdout.rstrip()
.split("\n")
)
def should_process_file(file_path: str) -> bool:
@ -223,136 +171,57 @@ def should_process_file(file_path: str) -> bool:
for component in Path(file_path).parts:
if component.startswith("."):
return False
if component.endswith(".framework"):
return False
if component in (
"Pods",
"ThirdParty",
"Carthage",
):
if component in ("Pods", "ThirdParty"):
return False
return True
def lint_swift_files(file_paths: set[str]) -> bool:
swift_file_paths = list(filter(lambda f: get_ext(f) == ".swift", file_paths))
file_count = len(swift_file_paths)
if file_count < 1:
def swiftlint(file_paths):
file_paths = list(filter(lambda f: get_ext(f) == ".swift", file_paths))
if len(file_paths) == 0:
return True
env = os.environ.copy()
env["SCRIPT_INPUT_FILE_COUNT"] = str(file_count)
for i, file_path in enumerate(swift_file_paths):
env[f"SCRIPT_INPUT_FILE_{i}"] = file_path
subprocess.run(
["swiftlint", "lint", "--fix", "--use-script-input-files"],
env=env,
)
proc = subprocess.run(
["swiftlint", "lint", "--strict", "--use-script-input-files"],
env=env,
)
subprocess.run(["swiftlint", "lint", "--quiet", "--fix", *file_paths])
proc = subprocess.run(["swiftlint", "lint", "--quiet", "--strict", *file_paths])
return proc.returncode == 0
def check_diff_for_keywords():
objc_keywords = [
"OWSAbstractMethod\\(",
"OWSAssert\\(",
"OWSCAssert\\(",
"OWSFail\\(",
"OWSCFail\\(",
"ows_add_overflow\\(",
"ows_sub_overflow\\(",
]
swift_keywords = [
"owsFail\\(",
"precondition\\(",
"fatalError\\(",
"dispatchPrecondition\\(",
"preconditionFailure\\(",
"notImplemented\\(",
]
keywords = objc_keywords + swift_keywords
matching_expression = "|".join(keywords)
command_line = (
'git diff --staged | grep --color=always -C 3 -E "%s"' % matching_expression
)
try:
output = subprocess.check_output(command_line, shell=True, text=True)
except subprocess.CalledProcessError as e:
# > man grep
# EXIT STATUS
# The grep utility exits with one of the following values:
# 0 One or more lines were selected.
# 1 No lines were selected.
# >1 An error occurred.
if e.returncode == 1:
# no keywords in diff output
return
else:
# some other error - bad grep expression?
raise e
if len(output) > 0:
print("⚠️ keywords detected in diff:")
print(output)
def clang_format(file_paths):
file_paths = list(filter(lambda f: get_ext(f) in CLANG_FORMAT_EXTS, file_paths))
if len(file_paths) == 0:
return True
proc = subprocess.run(["clang-format", "-i", *file_paths])
return proc.returncode == 0
if __name__ == "__main__":
parser = argparse.ArgumentParser(description="Precommit script.")
parser = argparse.ArgumentParser(description="lint & format files")
parser.add_argument("path", nargs="*", help="a path to process")
parser.add_argument(
"--all", action="store_true", help="process all files in or below current dir"
"--ref",
metavar="commit-sha",
help="process paths that have changed since this commit",
)
parser.add_argument("--path", help="used to specify a path to process.")
parser.add_argument(
"--ref", help="process all files that have changed since the given ref"
)
parser.add_argument(
"--skip_license_header_checks",
action="store_true",
help="A temporary flag that will skip license header checks. We plan to remove this flag soon.",
)
args = parser.parse_args()
ns = parser.parse_args()
all_file_paths: Iterable[str] = []
clang_format_commit = "HEAD"
if args.all:
all_file_paths = get_file_paths_in(git_repo_path)
elif args.path:
all_file_paths = get_file_paths_in(args.path)
elif args.ref:
all_file_paths = get_file_paths_for_commands(
[["git", "diff", "--name-only", "--diff-filter=ACMR", args.ref, "HEAD"]]
)
clang_format_commit = args.ref
if len(ns.path) > 0:
file_paths = ns.path
else:
all_file_paths = get_file_paths_for_commands(
[
["git", "diff", "--cached", "--name-only", "--diff-filter=ACMR"],
["git", "diff", "--name-only", "--diff-filter=ACMR"],
]
)
file_paths = set(filter(should_process_file, all_file_paths))
file_paths = get_file_paths_for_commit(ns.ref or "HEAD")
file_paths = sorted(set(filter(should_process_file, file_paths)))
result = True
if not args.skip_license_header_checks:
proc = subprocess.run(["Scripts/lint/lint-license-headers", "--fix"])
if proc.returncode != 0:
result = False
print("Checking license headers...", flush=True)
proc = subprocess.run(["Scripts/lint/lint-license-headers", "--fix", *file_paths])
if proc.returncode != 0:
result = False
print("")
print("Running SwiftLint...", flush=True)
if not lint_swift_files(file_paths):
print("Running swiftlint...", flush=True)
if not swiftlint(file_paths):
result = False
print("")
@ -362,25 +231,14 @@ if __name__ == "__main__":
print("")
print("Sorting Xcode project...", flush=True)
subprocess.run(["Scripts/sort-Xcode-project-file", "Signal.xcodeproj"])
proc = subprocess.run(["Scripts/sort-Xcode-project-file", "Signal.xcodeproj"])
if proc.returncode != 0:
result = False
print("")
print("Running clang-format...", flush=True)
# we don't want to format .proto files, so we specify every other supported extension
subprocess.run(
[
"git",
"clang-format",
"--extensions",
"c,h,m,mm,cc,cp,cpp,c++,cxx,hh,hxx,cu,java,js,ts,cs",
"--commit",
clang_format_commit,
]
)
print("")
print("Checking for keywords...", flush=True)
check_diff_for_keywords()
if not clang_format(file_paths):
result = False
print("")
if not result: