From 52770fdba083b4da807763a032273564b0373ec3 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 17 Sep 2024 16:49:31 -0500 Subject: [PATCH 1/9] Add wildcards to completely ignore files and add directory ignore --- Tools/build/check_warnings.py | 110 +++++++++++++++++++++++----------- 1 file changed, 76 insertions(+), 34 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index e58ee2a7cc85710..a3b6b1f81afd7f5 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -11,9 +11,42 @@ from typing import NamedTuple -class FileWarnings(NamedTuple): +class IgnoreRule(NamedTuple): name: str count: int + ignore_all: bool = False + is_directory: bool = False + +def parse_warning_ignore_file(file_path: str) -> set[IgnoreRule]: + """ + Parses the warning ignore file and returns a set of IgnoreRules + """ + files_with_expected_warnings = set() + with Path(file_path).open(encoding="UTF-8") as ignore_files: + files_with_expected_warnings = set() + for i, line in enumerate(ignore_files): + line = line.strip() + if line and not line.startswith("#"): + line_parts = line.split() + if len(line_parts) >= 2: + file_name = line_parts[0] + count = line_parts[1] + ignore_all = count == '*' + + is_directory = file_name.endswith('/') + + if is_directory and count != '*': + print(f"Error parsing ignore file: {file_path} at line: {i}") + print(f"Directory {file_name} must have count set to *") + sys.exit(1) + if ignore_all: + count = 0 + + files_with_expected_warnings.add( + IgnoreRule(file_name, int(count), ignore_all, is_directory) + ) + + return files_with_expected_warnings def extract_warnings_from_compiler_output( @@ -78,9 +111,22 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: return warnings_by_file +def is_file_ignored(file_path: str, ignore_files: set[IgnoreRule]) -> IgnoreRule | None: + """ + Returns the IgnoreRule object for the file if it is in the ignore list + """ + for ignore_file in ignore_files: + if ignore_file.is_directory: + if file_path.startswith(ignore_file.name): + return ignore_file + elif file_path == ignore_file.name: + return ignore_file + return None + + def get_unexpected_warnings( - files_with_expected_warnings: set[FileWarnings], - files_with_warnings: set[FileWarnings], + ignore_rules: set[IgnoreRule], + files_with_warnings: set[IgnoreRule], ) -> int: """ Returns failure status if warnings discovered in list of warnings @@ -89,14 +135,18 @@ def get_unexpected_warnings( """ unexpected_warnings = {} for file in files_with_warnings.keys(): - found_file_in_ignore_list = False - for ignore_file in files_with_expected_warnings: - if file == ignore_file.name: - if len(files_with_warnings[file]) > ignore_file.count: - unexpected_warnings[file] = (files_with_warnings[file], ignore_file.count) - found_file_in_ignore_list = True - break - if not found_file_in_ignore_list: + + ignore_rule = is_file_ignored(file, ignore_rules) + + if ignore_rule: + if ignore_rule.ignore_all: + continue + + if len(files_with_warnings[file]) > ignore_rule.count: + unexpected_warnings[file] = (files_with_warnings[file], ignore_rule.count) + continue + elif ignore_rule is None: + # If the file is not in the ignore list, then it is unexpected unexpected_warnings[file] = (files_with_warnings[file], 0) if unexpected_warnings: @@ -115,19 +165,21 @@ def get_unexpected_warnings( def get_unexpected_improvements( - files_with_expected_warnings: set[FileWarnings], - files_with_warnings: set[FileWarnings], + ignore_rules: set[IgnoreRule], + files_with_warnings: set[IgnoreRule], ) -> int: """ Returns failure status if there are no warnings in the list of warnings for a file that is in the list of files with expected warnings """ unexpected_improvements = [] - for file in files_with_expected_warnings: - if file.name not in files_with_warnings.keys(): - unexpected_improvements.append((file.name, file.count, 0)) - elif len(files_with_warnings[file.name]) < file.count: - unexpected_improvements.append((file.name, file.count, len(files_with_warnings[file.name]))) + for rule in ignore_rules: + if not rule.ignore_all and rule.name not in files_with_warnings.keys(): + if rule.name not in files_with_warnings.keys(): + import pdb; pdb.set_trace() + unexpected_improvements.append((rule.name, rule.count, 0)) + elif len(files_with_warnings[rule.name]) < rule.count: + unexpected_improvements.append((rule.name, rule.count, len(files_with_warnings[rule.name]))) if unexpected_improvements: print("Unexpected improvements:") @@ -202,7 +254,7 @@ def main(argv: list[str] | None = None) -> int: "Warning ignore file not specified." " Continuing without it (no warnings ignored)." ) - files_with_expected_warnings = set() + ignore_rules = set() else: if not Path(args.warning_ignore_file_path).is_file(): print( @@ -210,19 +262,9 @@ def main(argv: list[str] | None = None) -> int: f" {args.warning_ignore_file_path}" ) return 1 - with Path(args.warning_ignore_file_path).open( - encoding="UTF-8" - ) as clean_files: - # Files with expected warnings are stored as a set of tuples - # where the first element is the file name and the second element - # is the number of warnings expected in that file - files_with_expected_warnings = { - FileWarnings( - file.strip().split()[0], int(file.strip().split()[1]) - ) - for file in clean_files - if file.strip() and not file.startswith("#") - } + ignore_rules = parse_warning_ignore_file( + args.warning_ignore_file_path + ) with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() @@ -236,13 +278,13 @@ def main(argv: list[str] | None = None) -> int: files_with_warnings = get_warnings_by_file(warnings) status = get_unexpected_warnings( - files_with_expected_warnings, files_with_warnings + ignore_rules, files_with_warnings ) if args.fail_on_regression: exit_code |= status status = get_unexpected_improvements( - files_with_expected_warnings, files_with_warnings + ignore_rules, files_with_warnings ) if args.fail_on_improvement: exit_code |= status From ac70261ff01419f487171fdf2fc46046fbe8ede6 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 17 Sep 2024 17:06:30 -0500 Subject: [PATCH 2/9] Ignore all mimalloc recursive --- Tools/build/.warningignore_ubuntu | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/Tools/build/.warningignore_ubuntu b/Tools/build/.warningignore_ubuntu index d010152a229fae5..e5e7c9bbc2dfdf9 100644 --- a/Tools/build/.warningignore_ubuntu +++ b/Tools/build/.warningignore_ubuntu @@ -164,18 +164,7 @@ Objects/listobject.c 38 Objects/longobject.c 47 Objects/memoryobject.c 12 Objects/methodobject.c 1 -Objects/mimalloc/alloc.c 6 -Objects/mimalloc/arena.c 6 -Objects/mimalloc/heap.c 2 -Objects/mimalloc/init.c 2 -Objects/mimalloc/options.c 4 -Objects/mimalloc/os.c 4 -Objects/mimalloc/page-queue.c 2 -Objects/mimalloc/page.c 2 -Objects/mimalloc/prim/unix/prim.c 6 -Objects/mimalloc/random.c 1 -Objects/mimalloc/segment.c 11 -Objects/mimalloc/stats.c 5 +Objects/mimalloc/ * Objects/moduleobject.c 4 Objects/object.c 1 Objects/obmalloc.c 6 From b46439fb6cb241143591ac10be65e88f68062367 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 17 Sep 2024 17:07:35 -0500 Subject: [PATCH 3/9] Ignore all mimalloc recursively macos --- Tools/build/.warningignore_macos | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/Tools/build/.warningignore_macos b/Tools/build/.warningignore_macos index 2ed02ba6b634b0e..85f161503522bc7 100644 --- a/Tools/build/.warningignore_macos +++ b/Tools/build/.warningignore_macos @@ -138,18 +138,7 @@ Objects/listobject.c 43 Objects/longobject.c 46 Objects/memoryobject.c 6 Objects/methodobject.c 1 -Objects/mimalloc/alloc.c 6 -Objects/mimalloc/arena.c 6 -Objects/mimalloc/heap.c 1 -Objects/mimalloc/init.c 2 -Objects/mimalloc/options.c 1 -Objects/mimalloc/os.c 4 -Objects/mimalloc/page-queue.c 2 -Objects/mimalloc/page.c 1 -Objects/mimalloc/prim/osx/../unix/prim.c 2 -Objects/mimalloc/random.c 1 -Objects/mimalloc/segment.c 11 -Objects/mimalloc/stats.c 1 +Objects/mimalloc/ * Objects/moduleobject.c 2 Objects/object.c 1 Objects/obmalloc.c 6 From 7158ef77867428ae19a31e4354d171001729dffb Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 17 Sep 2024 17:15:00 -0500 Subject: [PATCH 4/9] Refactor variable names for rule language --- Tools/build/check_warnings.py | 85 ++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 35 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index a3b6b1f81afd7f5..1f3eafb45fd9e23 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -17,33 +17,40 @@ class IgnoreRule(NamedTuple): ignore_all: bool = False is_directory: bool = False + def parse_warning_ignore_file(file_path: str) -> set[IgnoreRule]: """ Parses the warning ignore file and returns a set of IgnoreRules """ files_with_expected_warnings = set() - with Path(file_path).open(encoding="UTF-8") as ignore_files: + with Path(file_path).open(encoding="UTF-8") as ignore_rules_file: files_with_expected_warnings = set() - for i, line in enumerate(ignore_files): + for i, line in enumerate(ignore_rules_file): line = line.strip() if line and not line.startswith("#"): line_parts = line.split() if len(line_parts) >= 2: file_name = line_parts[0] count = line_parts[1] - ignore_all = count == '*' - - is_directory = file_name.endswith('/') - - if is_directory and count != '*': - print(f"Error parsing ignore file: {file_path} at line: {i}") - print(f"Directory {file_name} must have count set to *") + ignore_all = count == "*" + is_directory = file_name.endswith("/") + + # Directories must have a wildcard count + if is_directory and count != "*": + print( + f"Error parsing ignore file: {file_path} at line: {i}" + ) + print( + f"Directory {file_name} must have count set to *" + ) sys.exit(1) if ignore_all: count = 0 - + files_with_expected_warnings.add( - IgnoreRule(file_name, int(count), ignore_all, is_directory) + IgnoreRule( + file_name, int(count), ignore_all, is_directory + ) ) return files_with_expected_warnings @@ -81,11 +88,15 @@ def extract_warnings_from_compiler_output( "line": match.group("line"), "column": match.group("column"), "message": match.group("message"), - "option": match.group("option").lstrip("[").rstrip("]"), + "option": match.group("option") + .lstrip("[") + .rstrip("]"), } ) except: - print(f"Error parsing compiler output. Unable to extract warning on line {i}:\n{line}") + print( + f"Error parsing compiler output. Unable to extract warning on line {i}:\n{line}" + ) sys.exit(1) return compiler_warnings @@ -111,16 +122,18 @@ def get_warnings_by_file(warnings: list[dict]) -> dict[str, list[dict]]: return warnings_by_file -def is_file_ignored(file_path: str, ignore_files: set[IgnoreRule]) -> IgnoreRule | None: +def is_file_ignored( + file_path: str, ignore_rules: set[IgnoreRule] +) -> IgnoreRule | None: """ - Returns the IgnoreRule object for the file if it is in the ignore list + Returns the IgnoreRule object for the file path if there is a related rule for it """ - for ignore_file in ignore_files: - if ignore_file.is_directory: - if file_path.startswith(ignore_file.name): - return ignore_file - elif file_path == ignore_file.name: - return ignore_file + for ignore_rule in ignore_rules: + if ignore_rule.is_directory: + if file_path.startswith(ignore_rule.name): + return ignore_rule + elif file_path == ignore_rule.name: + return ignore_rule return None @@ -143,7 +156,10 @@ def get_unexpected_warnings( continue if len(files_with_warnings[file]) > ignore_rule.count: - unexpected_warnings[file] = (files_with_warnings[file], ignore_rule.count) + unexpected_warnings[file] = ( + files_with_warnings[file], + ignore_rule.count, + ) continue elif ignore_rule is None: # If the file is not in the ignore list, then it is unexpected @@ -176,10 +192,15 @@ def get_unexpected_improvements( for rule in ignore_rules: if not rule.ignore_all and rule.name not in files_with_warnings.keys(): if rule.name not in files_with_warnings.keys(): - import pdb; pdb.set_trace() unexpected_improvements.append((rule.name, rule.count, 0)) elif len(files_with_warnings[rule.name]) < rule.count: - unexpected_improvements.append((rule.name, rule.count, len(files_with_warnings[rule.name]))) + unexpected_improvements.append( + ( + rule.name, + rule.count, + len(files_with_warnings[rule.name]), + ) + ) if unexpected_improvements: print("Unexpected improvements:") @@ -262,9 +283,7 @@ def main(argv: list[str] | None = None) -> int: f" {args.warning_ignore_file_path}" ) return 1 - ignore_rules = parse_warning_ignore_file( - args.warning_ignore_file_path - ) + ignore_rules = parse_warning_ignore_file(args.warning_ignore_file_path) with Path(args.compiler_output_file_path).open(encoding="UTF-8") as f: compiler_output_file_contents = f.read() @@ -272,27 +291,23 @@ def main(argv: list[str] | None = None) -> int: warnings = extract_warnings_from_compiler_output( compiler_output_file_contents, args.compiler_output_type, - args.path_prefix + args.path_prefix, ) files_with_warnings = get_warnings_by_file(warnings) - status = get_unexpected_warnings( - ignore_rules, files_with_warnings - ) + status = get_unexpected_warnings(ignore_rules, files_with_warnings) if args.fail_on_regression: exit_code |= status - status = get_unexpected_improvements( - ignore_rules, files_with_warnings - ) + status = get_unexpected_improvements(ignore_rules, files_with_warnings) if args.fail_on_improvement: exit_code |= status print( "For information about this tool and its configuration" " visit https://devguide.python.org/development-tools/warnings/" - ) + ) return exit_code From abb4fb73a4dc4fce2acc1d4515792c6a13163f01 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Tue, 17 Sep 2024 17:16:22 -0500 Subject: [PATCH 5/9] Change IgnoreRule name field to file_path --- Tools/build/check_warnings.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 1f3eafb45fd9e23..3f591a024e273ce 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -12,7 +12,7 @@ class IgnoreRule(NamedTuple): - name: str + file_path: str count: int ignore_all: bool = False is_directory: bool = False @@ -130,9 +130,9 @@ def is_file_ignored( """ for ignore_rule in ignore_rules: if ignore_rule.is_directory: - if file_path.startswith(ignore_rule.name): + if file_path.startswith(ignore_rule.file_path): return ignore_rule - elif file_path == ignore_rule.name: + elif file_path == ignore_rule.file_path: return ignore_rule return None @@ -190,15 +190,15 @@ def get_unexpected_improvements( """ unexpected_improvements = [] for rule in ignore_rules: - if not rule.ignore_all and rule.name not in files_with_warnings.keys(): - if rule.name not in files_with_warnings.keys(): - unexpected_improvements.append((rule.name, rule.count, 0)) - elif len(files_with_warnings[rule.name]) < rule.count: + if not rule.ignore_all and rule.file_path not in files_with_warnings.keys(): + if rule.file_path not in files_with_warnings.keys(): + unexpected_improvements.append((rule.file_path, rule.count, 0)) + elif len(files_with_warnings[rule.file_path]) < rule.count: unexpected_improvements.append( ( - rule.name, + rule.file_path, rule.count, - len(files_with_warnings[rule.name]), + len(files_with_warnings[rule.file_path]), ) ) From c2f856030c0e4cca622d863ab253405fd4773cc4 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Tue, 17 Sep 2024 22:21:59 +0000 Subject: [PATCH 6/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Tests/2024-09-17-22-21-58.gh-issue-124190.3fWhiX.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Tests/2024-09-17-22-21-58.gh-issue-124190.3fWhiX.rst diff --git a/Misc/NEWS.d/next/Tests/2024-09-17-22-21-58.gh-issue-124190.3fWhiX.rst b/Misc/NEWS.d/next/Tests/2024-09-17-22-21-58.gh-issue-124190.3fWhiX.rst new file mode 100644 index 000000000000000..819b1ca49235fc2 --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2024-09-17-22-21-58.gh-issue-124190.3fWhiX.rst @@ -0,0 +1 @@ +Add capability to ignore entire files or directories in check warning CI tool From ec93ec3f6e49e0c18d4e013ff5a994a72aa262e9 Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 18 Sep 2024 01:59:45 -0500 Subject: [PATCH 7/9] Rename ignore_rule to rule in is_file_ignored --- Tools/build/check_warnings.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 3f591a024e273ce..b885d9227d84aee 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -128,12 +128,12 @@ def is_file_ignored( """ Returns the IgnoreRule object for the file path if there is a related rule for it """ - for ignore_rule in ignore_rules: - if ignore_rule.is_directory: - if file_path.startswith(ignore_rule.file_path): - return ignore_rule - elif file_path == ignore_rule.file_path: - return ignore_rule + for rule in ignore_rules: + if rule.is_directory: + if file_path.startswith(rule.file_path): + return rule + elif file_path == rule.file_path: + return rule return None From cbc49a7288432a90b0fa355664bd016e5a102f8e Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 18 Sep 2024 02:04:42 -0500 Subject: [PATCH 8/9] All instances of IgnoreRule are now rule --- Tools/build/check_warnings.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index b885d9227d84aee..400c1174a94a0d0 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -149,19 +149,19 @@ def get_unexpected_warnings( unexpected_warnings = {} for file in files_with_warnings.keys(): - ignore_rule = is_file_ignored(file, ignore_rules) + rule = is_file_ignored(file, ignore_rules) - if ignore_rule: - if ignore_rule.ignore_all: + if rule: + if rule.ignore_all: continue - if len(files_with_warnings[file]) > ignore_rule.count: + if len(files_with_warnings[file]) > rule.count: unexpected_warnings[file] = ( files_with_warnings[file], - ignore_rule.count, + rule.count, ) continue - elif ignore_rule is None: + elif rule is None: # If the file is not in the ignore list, then it is unexpected unexpected_warnings[file] = (files_with_warnings[file], 0) From 4eb4db13e3085790b5b7ac987278ca1dd083d15d Mon Sep 17 00:00:00 2001 From: Nate Ohlson Date: Wed, 18 Sep 2024 02:16:09 -0500 Subject: [PATCH 9/9] Update docstring for getting unexpected warnings --- Tools/build/check_warnings.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Tools/build/check_warnings.py b/Tools/build/check_warnings.py index 400c1174a94a0d0..7210cc8365ead6a 100644 --- a/Tools/build/check_warnings.py +++ b/Tools/build/check_warnings.py @@ -185,8 +185,9 @@ def get_unexpected_improvements( files_with_warnings: set[IgnoreRule], ) -> int: """ - Returns failure status if there are no warnings in the list of warnings - for a file that is in the list of files with expected warnings + Returns failure status if the number of warnings for a file is greater + than the expected number of warnings for that file based on the ignore + rules """ unexpected_improvements = [] for rule in ignore_rules: