From 59d592ebac8f22d44fd48d7af7a5335fca21563f Mon Sep 17 00:00:00 2001 From: Dor Hen Date: Mon, 16 Sep 2024 15:48:45 +0300 Subject: [PATCH] Replace list usage with set for files accumulation in PRESUBMIT to prevent duplication Wherever we don't include any extra information about the issue (e.g file *and line number*), there's no need to return a presubmit result with the file duplicated (it spams the console for no reason...) bug: none Change-Id: I11968f97f7c927b01f5cda6e56ea03e3ff47dfca Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362621 Reviewed-by: Harald Alvestrand Reviewed-by: Jeremy Leconte Commit-Queue: Dor Hen Cr-Commit-Position: refs/heads/main@{#43029} --- PRESUBMIT.py | 77 ++++++++++++++++++++++++++-------------------------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 77c00388d6..575243bd1c 100755 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -186,7 +186,7 @@ Related files: def CheckNativeApiHeaderChanges(input_api, output_api): """Checks to remind proper changing of native APIs.""" - files = [] + files = set() source_file_filter = lambda x: input_api.FilterSourceFile( x, files_to_check=[r'.+\.(gn|gni|h)$']) for f in input_api.AffectedSourceFiles(source_file_filter): @@ -195,20 +195,20 @@ def CheckNativeApiHeaderChanges(input_api, output_api): if path == 'api': # Special case: Subdirectories included. if dn == 'api' or dn.startswith('api/'): - files.append(f.LocalPath()) + files.add(f.LocalPath()) else: # Normal case: Subdirectories not included. if dn == path: - files.append(f.LocalPath()) + files.add(f.LocalPath()) if files: - return [output_api.PresubmitNotifyResult(API_CHANGE_MSG, files)] + return [output_api.PresubmitNotifyResult(API_CHANGE_MSG, list(files))] return [] def CheckNoIOStreamInHeaders(input_api, output_api, source_file_filter): """Checks to make sure no .h files include .""" - files = [] + files = set() pattern = input_api.re.compile(r'^#include\s*', input_api.re.MULTILINE) file_filter = lambda x: (input_api.FilterSourceFile(x) and @@ -218,7 +218,7 @@ def CheckNoIOStreamInHeaders(input_api, output_api, source_file_filter): continue contents = input_api.ReadFile(f) if pattern.search(contents): - files.append(f) + files.add(f) if len(files) > 0: return [ @@ -226,14 +226,14 @@ def CheckNoIOStreamInHeaders(input_api, output_api, source_file_filter): 'Do not #include in header files, since it inserts ' 'static initialization into every file including the header. ' 'Instead, #include . See http://crbug.com/94794', - files) + list(files)) ] return [] def CheckNoPragmaOnce(input_api, output_api, source_file_filter): """Make sure that banned functions are not used.""" - files = [] + files = set() pattern = input_api.re.compile(r'^#pragma\s+once', input_api.re.MULTILINE) file_filter = lambda x: (input_api.FilterSourceFile(x) and source_file_filter(x)) @@ -242,14 +242,14 @@ def CheckNoPragmaOnce(input_api, output_api, source_file_filter): continue contents = input_api.ReadFile(f) if pattern.search(contents): - files.append(f) + files.add(f) if files: return [ output_api.PresubmitError( 'Do not use #pragma once in header files.\n' 'See http://www.chromium.org/developers/coding-style' - '#TOC-File-headers', files) + '#TOC-File-headers', list(files)) ] return [] @@ -316,12 +316,12 @@ def CheckApprovedFilesLintClean(input_api, output_api, # work with not-yet-converted code, we're only applying it to new (or # moved/renamed) files and files not listed in CPPLINT_EXCEPTIONS. verbosity_level = 1 - files = [] + files = set() for f in input_api.AffectedSourceFiles(source_file_filter): # Note that moved/renamed files also count as added. if f.Action() == 'A' or not IsLintDisabled(disabled_paths, f.LocalPath()): - files.append(f.AbsoluteLocalPath()) + files.add(f.AbsoluteLocalPath()) for file_name in files: cpplint.ProcessFile(file_name, verbosity_level) @@ -342,7 +342,7 @@ def CheckNoSourcesAbove(input_api, gn_files, output_api): re.MULTILINE | re.DOTALL) file_pattern = input_api.re.compile(r'"((\.\./.*?)|(//.*?))"') violating_gn_files = set() - violating_source_entries = [] + violating_source_entries = set() for gn_file in gn_files: contents = input_api.ReadFile(gn_file) for source_block_match in source_pattern.finditer(contents): @@ -352,7 +352,7 @@ def CheckNoSourcesAbove(input_api, gn_files, output_api): source_block_match.group(1)): source_file = file_list_match.group(1) if 'overrides/' not in source_file: - violating_source_entries.append(source_file) + violating_source_entries.add(source_file) violating_gn_files.add(gn_file) if violating_gn_files: return [ @@ -363,7 +363,7 @@ def CheckNoSourcesAbove(input_api, gn_files, output_api): 'Invalid source entries:\n' '%s\n' 'Violating GN files:' % '\n'.join(violating_source_entries), - items=violating_gn_files) + items=list(violating_gn_files)) ] return [] @@ -622,9 +622,9 @@ def CheckGnChanges(input_api, output_api): files_to_check=(r'.+\.(gn|gni)$', ), files_to_skip=(r'.*/presubmit_checks_lib/testdata/.*', ))) - gn_files = [] + gn_files = set() for f in input_api.AffectedSourceFiles(file_filter): - gn_files.append(f) + gn_files.add(f) result = [] if gn_files: @@ -840,7 +840,7 @@ def RunPythonTests(input_api, output_api): def CheckUsageOfGoogleProtobufNamespace(input_api, output_api, source_file_filter): """Checks that the namespace google::protobuf has not been used.""" - files = [] + files = set() pattern = input_api.re.compile(r'google::protobuf') proto_utils_path = os.path.join('rtc_base', 'protobuf_utils.h') file_filter = lambda x: (input_api.FilterSourceFile(x) and @@ -850,14 +850,14 @@ def CheckUsageOfGoogleProtobufNamespace(input_api, output_api, continue contents = input_api.ReadFile(f) if pattern.search(contents): - files.append(f) + files.add(f) if files: return [ output_api.PresubmitError( 'Please avoid to use namespace `google::protobuf` directly.\n' 'Add a using directive in `%s` and include that header instead.' - % proto_utils_path, files) + % proto_utils_path, list(files)) ] return [] @@ -1117,19 +1117,19 @@ def CheckBannedAbslMakeUnique(input_api, output_api, source_file_filter): file_filter = lambda f: (f.LocalPath().endswith( ('.cc', '.h')) and source_file_filter(f)) - files = [] + files = set() for f in input_api.AffectedFiles(include_deletes=False, file_filter=file_filter): for _, line in f.ChangedContents(): if 'absl::make_unique' in line: - files.append(f) + files.add(f) break if files: return [ output_api.PresubmitError( 'Please use std::make_unique instead of absl::make_unique.\n' - 'Affected files:', files) + 'Affected files:', list(files)) ] return [] @@ -1142,20 +1142,20 @@ def CheckBannedAbslOptional(input_api, output_api, source_file_filter): file_filter = lambda f: (f.LocalPath().endswith( ('.cc', '.h')) and source_file_filter(f)) - files = [] + files = set() for f in input_api.AffectedFiles(include_deletes=False, file_filter=file_filter): for _, line in f.ChangedContents(): if absl_optional.search(line) or absl_optional_include.search( line): - files.append(f.LocalPath()) + files.add(f.LocalPath()) break if files: return [ output_api.PresubmitError( 'Please use std::optional instead of absl::optional.\n' - 'Affected files:', files) + 'Affected files:', list(files)) ] return [] @@ -1171,19 +1171,19 @@ def CheckConditionalIncludes(input_api, output_api, source_file_filter): for key, value in conditional_includes.items(): include_regex = re.compile('^#include ' + key + '((?!IWYU pragma|no-presubmit-check).)*$') - files = [] + files = set() for f in input_api.AffectedFiles(include_deletes=False, file_filter=file_filter): for _, line in f.ChangedContents(): if include_regex.search(line): - files.append(f.LocalPath()) + files.add(f.LocalPath()) break if files: results.append( output_api.PresubmitError( 'Please include ' + value + ' instead of ' + key + - '.\nAffected files:', files)) + '.\nAffected files:', list(files))) return results @@ -1193,7 +1193,7 @@ def CheckObjcApiSymbols(input_api, output_api, source_file_filter): file_filter = lambda f: (f.LocalPath().endswith( ('.h')) and source_file_filter(f)) - files = [] + files = set() file_filter = lambda x: (input_api.FilterSourceFile(x) and source_file_filter(x)) for f in input_api.AffectedSourceFiles(file_filter): @@ -1205,7 +1205,7 @@ def CheckObjcApiSymbols(input_api, output_api, source_file_filter): for match in rtc_objc_export.finditer(contents): export_block = match.group(0) if 'RTC_OBJC_TYPE' not in export_block: - files.append(f.LocalPath()) + files.add(f.LocalPath()) if len(files) > 0: return [ @@ -1214,7 +1214,7 @@ def CheckObjcApiSymbols(input_api, output_api, source_file_filter): + 'macro.\n\n' + 'For example:\n' + 'RTC_OBJC_EXPORT @protocol RTC_OBJC_TYPE(RtcFoo)\n\n' + 'RTC_OBJC_EXPORT @interface RTC_OBJC_TYPE(RtcFoo)\n\n' + - 'Please fix the following files:', files) + 'Please fix the following files:', list(files)) ] return [] @@ -1224,19 +1224,19 @@ def CheckAssertUsage(input_api, output_api, source_file_filter): file_filter = lambda f: (f.LocalPath().endswith( ('.cc', '.h', '.m', '.mm')) and source_file_filter(f)) - files = [] + files = set() for f in input_api.AffectedFiles(include_deletes=False, file_filter=file_filter): for _, line in f.ChangedContents(): if pattern.search(line): - files.append(f.LocalPath()) + files.add(f.LocalPath()) break if len(files) > 0: return [ output_api.PresubmitError( 'Usage of assert() has been detected in the following files, ' - 'please use RTC_DCHECK() instead.\n Files:', files) + 'please use RTC_DCHECK() instead.\n Files:', list(files)) ] return [] @@ -1247,7 +1247,7 @@ def CheckAbslMemoryInclude(input_api, output_api, source_file_filter): file_filter = lambda f: (f.LocalPath().endswith( ('.cc', '.h')) and source_file_filter(f)) - files = [] + files = set() for f in input_api.AffectedFiles(include_deletes=False, file_filter=file_filter): contents = input_api.ReadFile(f) @@ -1255,7 +1255,7 @@ def CheckAbslMemoryInclude(input_api, output_api, source_file_filter): continue for _, line in f.ChangedContents(): if 'absl::WrapUnique' in line: - files.append(f) + files.add(f) break if len(files) > 0: @@ -1263,7 +1263,8 @@ def CheckAbslMemoryInclude(input_api, output_api, source_file_filter): output_api.PresubmitError( 'Please include "absl/memory/memory.h" header for ' 'absl::WrapUnique.\nThis header may or may not be included ' - 'transitively depending on the C++ standard version.', files) + 'transitively depending on the C++ standard version.', + list(files)) ] return []