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 <hta@webrtc.org> Reviewed-by: Jeremy Leconte <jleconte@webrtc.org> Commit-Queue: Dor Hen <dorhen@meta.com> Cr-Commit-Position: refs/heads/main@{#43029}
This commit is contained in:
Dor Hen
WebRTC LUCI CQ
parent
f3a33c0162
commit
59d592ebac
77
PRESUBMIT.py
77
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 <iostream>."""
|
||||
files = []
|
||||
files = set()
|
||||
pattern = input_api.re.compile(r'^#include\s*<iostream>',
|
||||
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 <iostream> in header files, since it inserts '
|
||||
'static initialization into every file including the header. '
|
||||
'Instead, #include <ostream>. 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 []
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user