Reland "Add _CheckAndroidNullAwayAnnotatedClasses PRESUBMIT check"
Doing a simpler check, not trying to figure out if the annotation is in the class declaration or on a method declaration. It would be very rare for a method to be annotation and not the class. This reverts commitae8498e751
. Reason for revert: Fixed version Original change's description: > Revert "Add _CheckAndroidNullAwayAnnotatedClasses PRESUBMIT check" > > This reverts commitcfe861e365
. > > Reason for revert: Doesn't ignore comments > > Original change's description: > > Add _CheckAndroidNullAwayAnnotatedClasses PRESUBMIT check > > > > Warn when a .java file is missing @NullMarked and @NullUnmarked. > > > > Temporarily disabled in //android_webview and //chrome while migrating. > > > > Bug: 404884589 > > Change-Id: I38e331bcec6f3a3f2147c0999fd4dca8fef145d5 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6373892 > > Commit-Queue: Henrique Nakashima <hnakashima@chromium.org> > > Reviewed-by: Andrew Grieve <agrieve@chromium.org> > > Cr-Commit-Position: refs/heads/main@{#1435461} > > Bug: 404884589 > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Change-Id: I669658f3f21cab9365f0c50b4e0191a1e6a608a0 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6377728 > Auto-Submit: Henrique Nakashima <hnakashima@chromium.org> > Commit-Queue: Andrew Grieve <agrieve@chromium.org> > Reviewed-by: Andrew Grieve <agrieve@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1435593} Bug: 404884589 Change-Id: I11d0ed28bf1941e72df5dcc457fb9d7e62dc89ac Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6381838 Auto-Submit: Henrique Nakashima <hnakashima@chromium.org> Reviewed-by: Andrew Grieve <agrieve@chromium.org> Commit-Queue: Andrew Grieve <agrieve@chromium.org> Cr-Commit-Position: refs/heads/main@{#1436182}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
2aa1bd20d7
commit
224ee248b1
43
PRESUBMIT.py
43
PRESUBMIT.py
@@ -5957,6 +5957,7 @@ def ChecksAndroidSpecificOnUpload(input_api, output_api):
|
|||||||
results.extend(_CheckNewImagesWarning(input_api, output_api))
|
results.extend(_CheckNewImagesWarning(input_api, output_api))
|
||||||
results.extend(_CheckAndroidNoBannedImports(input_api, output_api))
|
results.extend(_CheckAndroidNoBannedImports(input_api, output_api))
|
||||||
results.extend(_CheckAndroidInfoBarDeprecation(input_api, output_api))
|
results.extend(_CheckAndroidInfoBarDeprecation(input_api, output_api))
|
||||||
|
results.extend(_CheckAndroidNullAwayAnnotatedClasses(input_api, output_api))
|
||||||
return results
|
return results
|
||||||
|
|
||||||
|
|
||||||
@@ -7578,6 +7579,48 @@ a subclass of it), or use "@Rule BaseRobolectricTestRule".
|
|||||||
return results
|
return results
|
||||||
|
|
||||||
|
|
||||||
|
def _CheckAndroidNullAwayAnnotatedClasses(input_api, output_api):
|
||||||
|
"""Checks that Java classes/interfaces/annotations are null-annotated."""
|
||||||
|
|
||||||
|
nullmarked_annotation = input_api.re.compile(r'^\s*@(NullMarked|NullUnmarked)')
|
||||||
|
|
||||||
|
missing_annotation_errors = []
|
||||||
|
|
||||||
|
def _FilterFile(affected_file):
|
||||||
|
return input_api.FilterSourceFile(
|
||||||
|
affected_file,
|
||||||
|
files_to_skip=(_EXCLUDED_PATHS + _TEST_CODE_EXCLUDED_PATHS + input_api.
|
||||||
|
DEFAULT_FILES_TO_SKIP + (
|
||||||
|
r'.*Test.*\.java',
|
||||||
|
r'^android_webview/.*', # Temporary, crbug.com/389129271
|
||||||
|
r'^build/.*',
|
||||||
|
r'^chrome/.*', # Temporary, crbug.com/389129271
|
||||||
|
r'^chromecast/.*',
|
||||||
|
r'^components/cronet/.*',
|
||||||
|
r'^tools/.*',
|
||||||
|
)),
|
||||||
|
files_to_check=[r'.*\.java$'])
|
||||||
|
|
||||||
|
for f in input_api.AffectedSourceFiles(_FilterFile):
|
||||||
|
for line in f.NewContents():
|
||||||
|
if nullmarked_annotation.search(line):
|
||||||
|
break
|
||||||
|
else:
|
||||||
|
missing_annotation_errors.append(str(f.LocalPath()))
|
||||||
|
|
||||||
|
results = []
|
||||||
|
|
||||||
|
if missing_annotation_errors:
|
||||||
|
results.append(
|
||||||
|
output_api.PresubmitPromptWarning(
|
||||||
|
"""
|
||||||
|
Please add @NullMarked and fix the NullAway warnings in the following files
|
||||||
|
(see https://chromium.googlesource.com/chromium/src/+/main/styleguide/java/nullaway.md):
|
||||||
|
""", missing_annotation_errors))
|
||||||
|
|
||||||
|
return results
|
||||||
|
|
||||||
|
|
||||||
def CheckNoJsInIos(input_api, output_api):
|
def CheckNoJsInIos(input_api, output_api):
|
||||||
"""Checks to make sure that JavaScript files are not used on iOS."""
|
"""Checks to make sure that JavaScript files are not used on iOS."""
|
||||||
|
|
||||||
|
@@ -5138,6 +5138,84 @@ class CheckAndroidTestAnnotations(unittest.TestCase):
|
|||||||
self.assertIn('OneTest.java', errors[0].items[0])
|
self.assertIn('OneTest.java', errors[0].items[0])
|
||||||
|
|
||||||
|
|
||||||
|
class CheckAndroidNullAwayAnnotatedClasses(unittest.TestCase):
|
||||||
|
"""Test the _CheckAndroidNullAwayAnnotatedClasses presubmit check."""
|
||||||
|
|
||||||
|
def testDetectsInClasses(self):
|
||||||
|
"""Tests that missing @NullMarked or @NullUnmarked are correctly flagged in classes."""
|
||||||
|
mock_input = MockInputApi()
|
||||||
|
mock_input.files = [
|
||||||
|
MockFile('path/OneMissing.java', ['public class OneMissing']),
|
||||||
|
MockFile('path/TwoMarked.java', [
|
||||||
|
'@NullMarked',
|
||||||
|
'public class TwoMarked {',
|
||||||
|
]),
|
||||||
|
MockFile('path/ThreeMarked.java', [
|
||||||
|
'@NullUnmarked',
|
||||||
|
'public class ThreeMarked {',
|
||||||
|
]),
|
||||||
|
MockFile('path/FourMissing.java', ['class FourMissing']),
|
||||||
|
]
|
||||||
|
results = PRESUBMIT._CheckAndroidNullAwayAnnotatedClasses(mock_input, MockOutputApi())
|
||||||
|
self.assertEqual(1, len(results))
|
||||||
|
self.assertEqual('warning', results[0].type)
|
||||||
|
self.assertEqual(2, len(results[0].items))
|
||||||
|
self.assertIn('OneMissing.java', results[0].items[0])
|
||||||
|
self.assertIn('FourMissing.java', results[0].items[1])
|
||||||
|
|
||||||
|
def testDetectsInAnnotations(self):
|
||||||
|
"""Tests that missing @NullMarked or @NullUnmarked are correctly flagged in annotations."""
|
||||||
|
mock_input = MockInputApi()
|
||||||
|
mock_input.files = [
|
||||||
|
MockFile('path/OneMissing.java', ['@interface OneMissing']),
|
||||||
|
MockFile('path/TwoMarked.java', [
|
||||||
|
'@NullMarked',
|
||||||
|
'@interface TwoMarked {',
|
||||||
|
]),
|
||||||
|
]
|
||||||
|
results = PRESUBMIT._CheckAndroidNullAwayAnnotatedClasses(mock_input, MockOutputApi())
|
||||||
|
self.assertEqual(1, len(results))
|
||||||
|
self.assertEqual('warning', results[0].type)
|
||||||
|
self.assertEqual(1, len(results[0].items))
|
||||||
|
self.assertIn('OneMissing.java', results[0].items[0])
|
||||||
|
|
||||||
|
def testDetectsInInterfaces(self):
|
||||||
|
"""Tests that missing @NullMarked or @NullUnmarked are correctly flagged in interfaces."""
|
||||||
|
mock_input = MockInputApi()
|
||||||
|
mock_input.files = [
|
||||||
|
MockFile('path/OneMissing.java', ['interface OneMissing']),
|
||||||
|
MockFile('path/TwoMarked.java', [
|
||||||
|
'@NullMarked',
|
||||||
|
'interface TwoMarked {',
|
||||||
|
]),
|
||||||
|
]
|
||||||
|
results = PRESUBMIT._CheckAndroidNullAwayAnnotatedClasses(mock_input, MockOutputApi())
|
||||||
|
self.assertEqual(1, len(results))
|
||||||
|
self.assertEqual('warning', results[0].type)
|
||||||
|
self.assertEqual(1, len(results[0].items))
|
||||||
|
self.assertIn('OneMissing.java', results[0].items[0])
|
||||||
|
|
||||||
|
def testExcludesTests(self):
|
||||||
|
"""Tests that missing @NullMarked or @NullUnmarked are not flagged in tests."""
|
||||||
|
mock_input = MockInputApi()
|
||||||
|
mock_input.files = [
|
||||||
|
MockFile('path/OneTest.java', ['public class OneTest']),
|
||||||
|
]
|
||||||
|
results = PRESUBMIT._CheckAndroidNullAwayAnnotatedClasses(mock_input, MockOutputApi())
|
||||||
|
self.assertEqual(0, len(results))
|
||||||
|
|
||||||
|
def testExcludesTestSupport(self):
|
||||||
|
"""Tests that missing @NullMarked or @NullUnmarked are not flagged in test support classes."""
|
||||||
|
mock_input = MockInputApi()
|
||||||
|
mock_input.files = [
|
||||||
|
MockFile('path/test/Two.java', [
|
||||||
|
'public class Two'
|
||||||
|
]),
|
||||||
|
]
|
||||||
|
results = PRESUBMIT._CheckAndroidNullAwayAnnotatedClasses(mock_input, MockOutputApi())
|
||||||
|
self.assertEqual(0, len(results))
|
||||||
|
|
||||||
|
|
||||||
class AssertNoJsInIosTest(unittest.TestCase):
|
class AssertNoJsInIosTest(unittest.TestCase):
|
||||||
def testErrorJs(self):
|
def testErrorJs(self):
|
||||||
input_api = MockInputApi()
|
input_api = MockInputApi()
|
||||||
|
Reference in New Issue
Block a user