Make CheckForIncludeGuards() presubmit fire warning for incorrect #endif

Check headers to make sure the "#endif // GUARD" line matches the
"#if GUARD" line.

Change-Id: I9d11dc8cf42f4058bd3bb1cd9e27a3269251a301
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5577500
Reviewed-by: Daniel Cheng <[email protected]>
Commit-Queue: Lei Zhang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1306964}

PRESUBMIT.py

@@ -6191,6 +6191,7 @@ def replace_special_with_underscore(string):
         guard_name = None
         guard_line_number = None
         seen_guard_end = False
+        bypass_checks_at_end_of_file = False
 
         file_with_path = input_api.os_path.normpath(f.LocalPath())
         base_file_name = input_api.os_path.splitext(
@@ -6228,7 +6229,7 @@ def replace_special_with_underscore(string):
         for line_number, line in enumerate(f.NewContents()):
             if ('no-include-guard-because-multiply-included' in line
                     or 'no-include-guard-because-pch-file' in line):
-                guard_name = 'DUMMY'  # To not trigger check outside the loop.
+                bypass_checks_at_end_of_file = True
                 break
 
             if guard_name is None:
@@ -6273,6 +6274,9 @@ def replace_special_with_underscore(string):
                                 % (guard_name), [f.LocalPath()]))
                         break  # Nothing else to check and enough to warn once.
 
+        if bypass_checks_at_end_of_file:
+            continue
+
         if guard_name is None:
             errors.append(
                 output_api.PresubmitPromptWarning(
@@ -6282,6 +6286,12 @@ def replace_special_with_underscore(string):
                     '"no-include-guard-because-multiply-included" or\n'
                     '"no-include-guard-because-pch-file" in the header.'
                     % (f.LocalPath(), expected_guard)))
+        elif not seen_guard_end:
+            errors.append(
+                output_api.PresubmitPromptWarning(
+                    'Incorrect or missing include guard #endif in %s\n'
+                    'Recommended #endif comment: // %s'
+                    % (f.LocalPath(), expected_guard)))
 
     return errors
 

PRESUBMIT_test.py

@@ -1015,10 +1015,17 @@ def testIncludeGuardChecks(self):
           '#error "Failed to include content/common/foo_messages.h"',
           '#endif',
         ]),
+        MockAffectedFile('chrome/renderer/thing/qux.h', [
+          '// Comment',
+          '#ifndef CHROME_RENDERER_THING_QUX_H_',
+          '#define CHROME_RENDERER_THING_QUX_H_',
+          'struct Boaty;',
+          '#endif',
+        ]),
       ]
     msgs = PRESUBMIT.CheckForIncludeGuards(
         mock_input_api, mock_output_api)
-    expected_fail_count = 8
+    expected_fail_count = 10
     self.assertEqual(expected_fail_count, len(msgs),
                      'Expected %d items, found %d: %s'
                      % (expected_fail_count, len(msgs), msgs))
@@ -1029,37 +1036,46 @@ def testIncludeGuardChecks(self):
 
     self.assertIn('content/browser/test1.h', msgs[1].message)
     self.assertIn('Recommended name: CONTENT_BROWSER_TEST1_H_',
-                     msgs[1].message)
+                  msgs[1].message)
 
     self.assertEqual(msgs[2].items, ['content/browser/test2.h:3'])
     self.assertEqual(msgs[2].message,
                      'Missing "#define CONTENT_BROWSER_TEST2_H_" for '
                      'include guard')
 
-    self.assertEqual(msgs[3].items, ['content/browser/spleling.h:1'])
-    self.assertEqual(msgs[3].message,
+    self.assertIn('content/browser/internal.h', msgs[3].message)
+    self.assertIn('Recommended #endif comment: // CONTENT_BROWSER_INTERNAL_H_',
+                  msgs[3].message)
+
+    self.assertEqual(msgs[4].items, ['content/browser/spleling.h:1'])
+    self.assertEqual(msgs[4].message,
                      'Header using the wrong include guard name '
                      'CONTENT_BROWSER_SPLLEING_H_')
 
-    self.assertIn('content/browser/foo+bar.h', msgs[4].message)
+    self.assertIn('content/browser/foo+bar.h', msgs[5].message)
     self.assertIn('Recommended name: CONTENT_BROWSER_FOO_BAR_H_',
-                  msgs[4].message)
+                  msgs[5].message)
 
-    self.assertEqual(msgs[5].items, ['content/NotInBlink.h:1'])
-    self.assertEqual(msgs[5].message,
+    self.assertEqual(msgs[6].items, ['content/NotInBlink.h:1'])
+    self.assertEqual(msgs[6].message,
                      'Header using the wrong include guard name '
                      'NotInBlink_h')
 
-    self.assertEqual(msgs[6].items, ['third_party/blink/InBlink.h:1'])
-    self.assertEqual(msgs[6].message,
+    self.assertEqual(msgs[7].items, ['third_party/blink/InBlink.h:1'])
+    self.assertEqual(msgs[7].message,
                      'Header using the wrong include guard name '
                      'InBlink_h')
 
-    self.assertEqual(msgs[7].items, ['third_party/blink/AlsoInBlink.h:1'])
-    self.assertEqual(msgs[7].message,
+    self.assertEqual(msgs[8].items, ['third_party/blink/AlsoInBlink.h:1'])
+    self.assertEqual(msgs[8].message,
                      'Header using the wrong include guard name '
                      'WrongInBlink_h')
 
+    self.assertIn('chrome/renderer/thing/qux.h', msgs[9].message)
+    self.assertIn('Recommended #endif comment: // CHROME_RENDERER_THING_QUX_H_',
+                  msgs[9].message)
+
+
 class AccessibilityRelnotesFieldTest(unittest.TestCase):
   def testRelnotesPresent(self):
     mock_input_api = MockInputApi()