diff --git a/PRESUBMIT.py b/PRESUBMIT.py
index b95248a7380fa..52e46bf169bf1 100644
--- a/PRESUBMIT.py
+++ b/PRESUBMIT.py
@@ -144,6 +144,9 @@ class BanRule:
# expression that will be matched against the path of the file being checked
# relative to the root of the source tree.
excluded_paths: Optional[Sequence[str]] = None
+ # If True, surfaces any violation as a Gerrit comment on the CL after
+ # running the CQ.
+ surface_as_gerrit_lint: Optional[bool] = None
_BANNED_JAVA_IMPORTS : Sequence[BanRule] = (
@@ -316,6 +319,7 @@ _BANNED_JAVA_FUNCTIONS : Sequence[BanRule] = (
r'^infra/', # This is permitted in infra/ folder.
r'^tools/', # This is permitted in tools/ folder.
],
+ surface_as_gerrit_lint=True,
),
)
@@ -2227,6 +2231,7 @@ _BANNED_CPP_FUNCTIONS: Sequence[BanRule] = (
r'^infra/', # This is permitted in infra/ folder.
r'^tools/', # This is permitted in tools/ folder.
],
+ surface_as_gerrit_lint=True,
),
)
@@ -2996,8 +3001,7 @@ def _GetMessageForMatchingType(input_api, affected_file, line_number, line,
def CheckNoBannedFunctions(input_api, output_api):
"""Make sure that banned functions are not used."""
- warnings = []
- errors = []
+ results= []
def IsExcludedFile(affected_file, excluded_paths):
if not excluded_paths:
@@ -3027,13 +3031,27 @@ def CheckNoBannedFunctions(input_api, output_api):
if IsExcludedFile(affected_file, ban_rule.excluded_paths):
return
- problems = _GetMessageForMatchingType(input_api, f, line_num, line,
- ban_rule)
- if problems:
+ message = _GetMessageForMatchingType(input_api, f, line_num, line,
+ ban_rule)
+ if message:
+ result_loc = []
+ if ban_rule.surface_as_gerrit_lint:
+ result_loc.append(output_api.PresubmitResultLocation(
+ file_path=affected_file.LocalPath(),
+ start_line=line_num,
+ end_line=line_num,
+ ))
if ban_rule.treat_as_error is not None and ban_rule.treat_as_error:
- errors.extend(problems)
+ results.append(
+ output_api.PresubmitError('A banned function was used.\n' +
+ '\n'.join(message),
+ locations=result_loc))
+
else:
- warnings.extend(problems)
+ results.append(
+ output_api.PresubmitPromptWarning('A banned function was used.\n' +
+ '\n'.join(message),
+ locations=result_loc))
file_filter = lambda f: f.LocalPath().endswith(('.java'))
for f in input_api.AffectedFiles(file_filter=file_filter):
@@ -3096,17 +3114,8 @@ def CheckNoBannedFunctions(input_api, output_api):
for ban_rule in _BANNED_MOJOM_PATTERNS:
CheckForMatch(f, line_num, line, ban_rule)
+ return results
- result = []
- if (warnings):
- result.append(
- output_api.PresubmitPromptWarning('Banned functions were used.\n' +
- '\n'.join(warnings)))
- if (errors):
- result.append(
- output_api.PresubmitError('Banned functions were used.\n' +
- '\n'.join(errors)))
- return result
def _CheckAndroidNoBannedImports(input_api, output_api):
"""Make sure that banned java imports are not used."""
diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py
index 364fdba1c33a1..6fa5eb342765e 100755
--- a/PRESUBMIT_test.py
+++ b/PRESUBMIT_test.py
@@ -2979,35 +2979,34 @@ class BannedTypeCheckTest(unittest.TestCase):
]
errors = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- self.assertEqual(2, len(errors))
+ self.assertEqual(12, len(errors))
self.assertTrue(
'some/java/problematic/diskread.java' in errors[0].message)
self.assertTrue(
- 'some/java/problematic/diskwrite.java' in errors[0].message)
- self.assertFalse('some/java/ok/diskwrite.java' in errors[0].message)
- self.assertFalse('some/java/ok/diskwrite.java' in errors[1].message)
+ 'some/java/problematic/diskwrite.java' in errors[1].message)
+ self.assertTrue(all('some/java/ok/diskwrite.java' not in e.message for e in errors))
self.assertTrue(
- 'some/java/problematic/waitidleforsync.java' in errors[0].message)
+ 'some/java/problematic/waitidleforsync.java' in errors[2].message)
self.assertTrue(
- 'some/java/problematic/registerreceiver.java' in errors[1].message)
+ 'some/java/problematic/registerreceiver.java' in errors[3].message)
self.assertTrue(
- 'some/java/problematic/property.java' in errors[0].message)
+ 'some/java/problematic/property.java' in errors[4].message)
self.assertTrue(
- 'some/java/problematic/requestlayout.java' in errors[0].message)
+ 'some/java/problematic/requestlayout.java' in errors[5].message)
self.assertTrue(
- 'some/java/problematic/lastprofile.java' in errors[0].message)
+ 'some/java/problematic/lastprofile.java' in errors[6].message)
self.assertTrue(
- 'some/java/problematic/getdrawable1.java' in errors[0].message)
+ 'some/java/problematic/getdrawable1.java' in errors[7].message)
self.assertTrue(
- 'some/java/problematic/getdrawable2.java' in errors[0].message)
+ 'some/java/problematic/getdrawable2.java' in errors[8].message)
self.assertTrue('some/java/problematic/announceForAccessibility.java'
- in errors[0].message)
+ in errors[9].message)
self.assertTrue(
'some/java/problematic/accessibilityTypeAnnouncement.java' in
- errors[0].message)
+ errors[10].message)
self.assertTrue(
'content/java/problematic/desktopandroid.java' in
- errors[0].message)
+ errors[11].message)
def testBannedCppFunctions(self):
@@ -3035,29 +3034,36 @@ class BannedTypeCheckTest(unittest.TestCase):
['std::ranges::subrange(first, last)']),
MockFile('views_usage.cc', ['std::views::all(vec)']),
MockFile('content/desktop_android.cc',
- ['#if BUILDFLAG(IS_DESKTOP_ANDROID)']),
+ [
+ '// some first line',
+ '#if BUILDFLAG(IS_DESKTOP_ANDROID)',
+ '// some third line',
+ ]),
]
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- # warnings are results[0], errors are results[1]
- self.assertEqual(2, len(results))
- self.assertTrue('some/cpp/problematic/file.cc' in results[1].message)
+ # Each entry in results corresponds to a BanRule with a violation, in
+ # the order they were encountered.
+ self.assertEqual(8, len(results))
+ self.assertTrue('some/cpp/problematic/file.cc' in results[0].message)
self.assertTrue(
- 'third_party/blink/problematic/file.cc' in results[0].message)
- self.assertTrue('some/cpp/ok/file.cc' not in results[1].message)
- self.assertTrue('some/cpp/problematic/file2.cc' in results[0].message)
- self.assertTrue('some/cpp/problematic/file3.cc' in results[0].message)
- self.assertTrue('some/cpp/problematic/file4.cc' in results[0].message)
- self.assertFalse('some/cpp/nocheck/file.cc' in results[0].message)
- self.assertFalse('some/cpp/nocheck/file.cc' in results[1].message)
- self.assertFalse('some/cpp/comment/file.cc' in results[0].message)
- self.assertFalse('some/cpp/comment/file.cc' in results[1].message)
- self.assertFalse('allowed_ranges_usage.cc' in results[0].message)
- self.assertFalse('allowed_ranges_usage.cc' in results[1].message)
- self.assertTrue('banned_ranges_usage.cc' in results[1].message)
- self.assertTrue('views_usage.cc' in results[1].message)
- self.assertTrue('content/desktop_android.cc' in results[0].message)
+ 'third_party/blink/problematic/file.cc' in results[1].message)
+ self.assertTrue(all('some/cpp/ok/file.cc' not in r.message for r in results))
+ self.assertTrue('some/cpp/problematic/file2.cc' in results[2].message)
+ self.assertTrue('some/cpp/problematic/file3.cc' in results[3].message)
+ self.assertTrue('some/cpp/problematic/file4.cc' in results[4].message)
+ self.assertTrue(all('some/cpp/nocheck/file.cc' not in r.message for r in results))
+ self.assertTrue(all('some/cpp/comment/file.cc' not in r.message for r in results))
+ self.assertTrue(all('allowed_ranges_usage.cc' not in r.message for r in results))
+ self.assertTrue('banned_ranges_usage.cc' in results[5].message)
+ self.assertTrue('views_usage.cc' in results[6].message)
+ self.assertTrue('content/desktop_android.cc' in results[7].message)
+
+ # Check ResultLocation data. Line nums start at 1.
+ self.assertEqual(results[7].locations[0].file_path, 'content/desktop_android.cc')
+ self.assertEqual(results[7].locations[0].start_line, 2)
+ self.assertEqual(results[7].locations[0].end_line, 2)
def testBannedCppRandomFunctions(self):
banned_rngs = [
@@ -3092,15 +3098,14 @@ class BannedTypeCheckTest(unittest.TestCase):
]
results = PRESUBMIT.CheckNoBannedFunctions(input_api,
MockOutputApi())
- self.assertEqual(1, len(results), banned_rng)
+ self.assertEqual(2, len(results), banned_rng)
self.assertTrue(
'some/cpp/problematic/file.cc' in results[0].message,
banned_rng)
self.assertTrue(
- 'third_party/blink/problematic/file.cc' in results[0].message,
+ 'third_party/blink/problematic/file.cc' in results[1].message,
banned_rng)
- self.assertFalse('third_party/ok/file.cc' in results[0].message,
- banned_rng)
+ self.assertTrue(all('third_party/ok/file.cc' not in r.message for r in results))
def testBannedIosObjcFunctions(self):
input_api = MockInputApi()
@@ -3120,12 +3125,12 @@ class BannedTypeCheckTest(unittest.TestCase):
]
errors = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- self.assertEqual(1, len(errors))
+ self.assertEqual(3, len(errors))
self.assertTrue('some/ios/file.mm' in errors[0].message)
- self.assertTrue('another/ios_file.mm' in errors[0].message)
- self.assertTrue('some/mac/file.mm' not in errors[0].message)
- self.assertTrue('some/ios/file_egtest.mm' in errors[0].message)
- self.assertTrue('some/ios/file_unittest.mm' not in errors[0].message)
+ self.assertTrue('another/ios_file.mm' in errors[1].message)
+ self.assertTrue(all('some/mac/file.mm' not in e.message for e in errors))
+ self.assertTrue('some/ios/file_egtest.mm' in errors[2].message)
+ self.assertTrue(all('some/ios/file_unittest.mm' not in e.message for e in errors))
def testBannedMojoFunctions(self):
input_api = MockInputApi()
@@ -3137,7 +3142,8 @@ class BannedTypeCheckTest(unittest.TestCase):
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- # warnings are results[0], errors are results[1]
+ # Each entry in results corresponds to a BanRule with a violation, in
+ # the order they were encountered.
self.assertEqual(1, len(results))
self.assertTrue('some/cpp/problematic/file2.cc' in results[0].message)
self.assertTrue(
@@ -3161,7 +3167,8 @@ class BannedTypeCheckTest(unittest.TestCase):
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- # warnings are results[0], errors are results[1]
+ # Each entry in results corresponds to a BanRule with a violation, in
+ # the order they were encountered.
self.assertEqual(1, len(results))
self.assertTrue('bad.mojom' in results[0].message)
self.assertTrue('good.mojom' not in results[0].message)
@@ -3184,23 +3191,23 @@ class BannedTypeCheckTest(unittest.TestCase):
['string extension_id']),
]
- # warnings are results[0], errors are results[1]
+ # Each entry in results corresponds to a BanRule with a violation, in
+ # the order they were encountered.
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- # Only warnings and no errors.
- self.assertEqual(1, len(results))
+ self.assertEqual(3, len(results))
# Pattern test assertions.
self.assertTrue('bad.mojom' in results[0].message)
- self.assertTrue('bad_struct.mojom' in results[0].message)
- self.assertTrue('good.mojom' not in results[0].message)
- self.assertTrue('good_struct.mojom' not in results[0].message)
+ self.assertTrue('bad_struct.mojom' in results[1].message)
+ self.assertTrue(all('good.mojom' not in r.message for r in results))
+ self.assertTrue(all('good_struct.mojom' not in r.message for r in results))
# Path exclusion assertions.
self.assertTrue('some/included/extensions/path/bad_extension_id.mojom'
- in results[0].message)
- self.assertTrue('some/excluded/path/bad_extension_id.mojom' not in
- results[0].message)
+ in results[2].message)
+ self.assertTrue(all('some/excluded/path/bad_extension_id.mojom' not in r.message for r in results))
+
class NoProductionCodeUsingTestOnlyFunctionsTest(unittest.TestCase):
@@ -5732,20 +5739,19 @@ class CheckDeprecatedSyncConsentFunctionsTest(unittest.TestCase):
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- self.assertEqual(1, len(results))
- self.assertFalse(
- 'chrome/browser/android/file.cc' in results[0].message),
+ self.assertEqual(7, len(results))
+ self.assertTrue(all('chrome/browser/android/file.cc' not in r.message for r in results))
self.assertTrue('chrome/android/file.cc' in results[0].message),
- self.assertTrue('ios/file.mm' in results[0].message),
- self.assertTrue('components/foo/ios/file.cc' in results[0].message),
+ self.assertTrue('ios/file.mm' in results[1].message),
+ self.assertTrue('components/foo/ios/file.cc' in results[2].message),
self.assertTrue(
- 'components/foo/delegate_android.cc' in results[0].message),
+ 'components/foo/delegate_android.cc' in results[3].message),
self.assertTrue(
- 'components/foo/delegate_ios.cc' in results[0].message),
+ 'components/foo/delegate_ios.cc' in results[4].message),
self.assertTrue(
- 'components/foo/android_delegate.cc' in results[0].message),
+ 'components/foo/android_delegate.cc' in results[5].message),
self.assertTrue(
- 'components/foo/ios_delegate.cc' in results[0].message),
+ 'components/foo/ios_delegate.cc' in results[6].message),
def testCppNonMobilePlatformPath(self):
input_api = MockInputApi()
@@ -5771,12 +5777,12 @@ class CheckDeprecatedSyncConsentFunctionsTest(unittest.TestCase):
results = PRESUBMIT.CheckNoBannedFunctions(input_api, MockOutputApi())
- self.assertEqual(1, len(results))
- self.assertFalse('components/foo/file1.java' in results[0].message),
+ self.assertEqual(4, len(results))
+ self.assertTrue(all('components/foo/file1.java' not in r.message for r in results))
self.assertTrue('components/foo/file2.java' in results[0].message),
- self.assertTrue('chrome/foo/file3.java' in results[0].message),
- self.assertTrue('chrome/foo/file4.java' in results[0].message),
- self.assertTrue('chrome/foo/file5.java' in results[0].message),
+ self.assertTrue('chrome/foo/file3.java' in results[1].message),
+ self.assertTrue('chrome/foo/file4.java' in results[2].message),
+ self.assertTrue('chrome/foo/file5.java' in results[3].message),
class CheckAnonymousNamespaceTest(unittest.TestCase):
diff --git a/PRESUBMIT_test_mocks.py b/PRESUBMIT_test_mocks.py
index 131a7777979c2..baca10648edcb 100644
--- a/PRESUBMIT_test_mocks.py
+++ b/PRESUBMIT_test_mocks.py
@@ -210,42 +210,46 @@ class MockOutputApi(object):
class PresubmitResult(object):
- def __init__(self, message, items=None, long_text=''):
+ def __init__(self, message, items=None, long_text='', locations=[]):
self.message = message
self.items = items
self.long_text = long_text
+ self.locations = locations
def __repr__(self):
return self.message
class PresubmitError(PresubmitResult):
- def __init__(self, message, items=None, long_text=''):
- MockOutputApi.PresubmitResult.__init__(self, message, items,
- long_text)
+ def __init__(self, *args, **kwargs):
+ MockOutputApi.PresubmitResult.__init__(self, *args, **kwargs)
self.type = 'error'
class PresubmitPromptWarning(PresubmitResult):
- def __init__(self, message, items=None, long_text=''):
- MockOutputApi.PresubmitResult.__init__(self, message, items,
- long_text)
+ def __init__(self, *args, **kwargs):
+ MockOutputApi.PresubmitResult.__init__(self, *args, **kwargs)
self.type = 'warning'
class PresubmitNotifyResult(PresubmitResult):
- def __init__(self, message, items=None, long_text=''):
- MockOutputApi.PresubmitResult.__init__(self, message, items,
- long_text)
+ def __init__(self, *args, **kwargs):
+ MockOutputApi.PresubmitResult.__init__(self, *args, **kwargs)
self.type = 'notify'
class PresubmitPromptOrNotify(PresubmitResult):
- def __init__(self, message, items=None, long_text=''):
- MockOutputApi.PresubmitResult.__init__(self, message, items,
- long_text)
+ def __init__(self, *args, **kwargs):
+ MockOutputApi.PresubmitResult.__init__(self, *args, **kwargs)
self.type = 'promptOrNotify'
+ class PresubmitResultLocation(object):
+
+ def __init__(self, file_path, start_line, end_line):
+ self.file_path = file_path
+ self.start_line = start_line
+ self.end_line = end_line
+
def __init__(self):
self.more_cc = []