diff --git a/PRESUBMIT.py b/PRESUBMIT.py index b6bbf9ecaf2e1..24a5f29893330 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -5957,6 +5957,7 @@ def ChecksAndroidSpecificOnUpload(input_api, output_api): results.extend(_CheckNewImagesWarning(input_api, output_api)) results.extend(_CheckAndroidNoBannedImports(input_api, output_api)) results.extend(_CheckAndroidInfoBarDeprecation(input_api, output_api)) + results.extend(_CheckAndroidNullAwayAnnotatedClasses(input_api, output_api)) return results @@ -7578,6 +7579,48 @@ a subclass of it), or use "@Rule BaseRobolectricTestRule". 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): """Checks to make sure that JavaScript files are not used on iOS.""" diff --git a/PRESUBMIT_test.py b/PRESUBMIT_test.py index 13552626245ed..1739c3e115deb 100755 --- a/PRESUBMIT_test.py +++ b/PRESUBMIT_test.py @@ -5138,6 +5138,84 @@ class CheckAndroidTestAnnotations(unittest.TestCase): 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): def testErrorJs(self): input_api = MockInputApi()