Add automatic header guard checks/fixes (#1260)

My intent is to add CARBON_ bas a prefix, and this makes that easier by creating a tool for auto-fixing guards in general.

string_literal is a manual fix -- it had no guard and I didn't automate that (technically I think I could, especially by enforcing the file header/footer, but it didn't feel quite worth it to me).

.pre-commit-config.yaml

@@ -79,10 +79,17 @@ repos:
         pass_filenames: false
         additional_dependencies: ['clang-format==13.0.1']
 
-  # This may rename files, so it's deliberately between formatters and linters.
   - repo: local
     hooks:
+      - id: check-header-guards
+        # This should run after clang-format, which may reformat a guard.
+        name: Check header guards
+        entry: scripts/check_header_guards.py
+        language: python
+        files: '^.*\.h$'
       - id: check-sha-filenames
+        # This may rename files, so it's deliberately between formatters and
+        # linters.
         name: Check fuzzer SHA filenames
         entry: scripts/check_sha_filenames.py
         language: python

explorer/ast/impl_binding.h

@@ -2,8 +2,8 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#ifndef EXPLORER_AST_GENERIC_BINDING_H_
-#define EXPLORER_AST_GENERIC_BINDING_H_
+#ifndef EXPLORER_AST_IMPL_BINDING_H_
+#define EXPLORER_AST_IMPL_BINDING_H_
 
 #include <map>
 
@@ -75,4 +75,4 @@ class ImplBinding : public AstNode {
 
 }  // namespace Carbon
 
-#endif  // EXPLORER_AST_GENERIC_BINDING_H_
+#endif  // EXPLORER_AST_IMPL_BINDING_H_

explorer/interpreter/impl_scope.h

@@ -2,8 +2,8 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#ifndef EXPLORER_AST_IMPL_SCOPE_H_
-#define EXPLORER_AST_IMPL_SCOPE_H_
+#ifndef EXPLORER_INTERPRETER_IMPL_SCOPE_H_
+#define EXPLORER_INTERPRETER_IMPL_SCOPE_H_
 
 #include "explorer/ast/declaration.h"
 
@@ -110,4 +110,4 @@ class ImplScope {
 
 }  // namespace Carbon
 
-#endif  // EXPLORER_AST_IMPL_SCOPE_H_
+#endif  // EXPLORER_INTERPRETER_IMPL_SCOPE_H_

explorer/interpreter/resolve_names.h

@@ -17,4 +17,4 @@ auto ResolveNames(AST& ast) -> ErrorOr<Success>;
 
 }  // namespace Carbon
 
-#endif  // EXPLORER_INTERPRETER_RESOLVE_CONTROL_FLOW_H_
+#endif  // EXPLORER_INTERPRETER_RESOLVE_NAMES_H_

explorer/interpreter/stack.h

@@ -72,4 +72,4 @@ struct Stack {
 
 }  // namespace Carbon
 
-#endif  // EXPLORER_INTERPRETER_CONS_LIST_H_
+#endif  // EXPLORER_INTERPRETER_STACK_H_

explorer/syntax/parse_and_lex_context.h

@@ -2,8 +2,8 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#ifndef EXPLORER_SYNTAX_DRIVER_H_
-#define EXPLORER_SYNTAX_DRIVER_H_
+#ifndef EXPLORER_SYNTAX_PARSE_AND_LEX_CONTEXT_H_
+#define EXPLORER_SYNTAX_PARSE_AND_LEX_CONTEXT_H_
 
 #include <variant>
 
@@ -62,4 +62,4 @@ class ParseAndLexContext {
 // Declares yylex for the parser's sake.
 YY_DECL;
 
-#endif  // EXECUTABLE_SYNTAX_DRIVER_H_
+#endif  // EXPLORER_SYNTAX_PARSE_AND_LEX_CONTEXT_H_

scripts/check_header_guards.py

@@ -0,0 +1,112 @@
+#!/usr/bin/env python3
+
+"""Checks for missing or incorrect header guards."""
+
+__copyright__ = """
+Part of the Carbon Language project, under the Apache License v2.0 with LLVM
+Exceptions. See /LICENSE for license information.
+SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+"""
+
+from pathlib import Path
+import re
+import sys
+from typing import Iterable, List, NamedTuple, Optional
+
+
+class Guard(NamedTuple):
+    """A guard line in a file."""
+
+    line: int
+    guard: str
+
+
+def find_guard(
+    lines: List[str], pattern: str, from_end: bool
+) -> Optional[Guard]:
+    """Searches the lines for something matching the pattern."""
+    lines_range: Iterable[str] = lines
+    if from_end:
+        lines_range = reversed(lines)
+    for index, line in enumerate(lines_range):
+        m = re.match(pattern, line)
+        if m:
+            if from_end:
+                index = len(lines) - index - 1
+            return Guard(index, m[1])
+    return None
+
+
+def maybe_replace(
+    lines: List[str], old_guard: Guard, guard_prefix: str, guard: str
+) -> None:
+    """Replaces a header guard in the file if needed."""
+    if guard != old_guard.guard:
+        line = lines[old_guard.line].rstrip("\n")
+        print(f"- Replacing line {old_guard.line}: `{line}`", file=sys.stderr)
+        lines[old_guard.line] = f"{guard_prefix} {guard}\n"
+
+
+def check_path(path: Path) -> bool:
+    """Checks the path for header guard issues."""
+    if path.suffix != ".h":
+        print(f"Not a header: {path}", file=sys.stderr)
+        return True
+
+    with path.open() as f:
+        lines = f.readlines()
+
+    guard = str(path).upper().replace("/", "_").replace(".", "_") + "_"
+    ifndef = find_guard(lines, "#ifndef ([A-Z_]+_H_)", False)
+    define = find_guard(lines, "#define ([A-Z_]+_H_)", False)
+    endif = find_guard(lines, "#endif(?:  // ([A-Z_]+_H_))?", True)
+    if ifndef is None or define is None or endif is None:
+        print(f"Incomplete header guard in {path}:", file=sys.stderr)
+        if ifndef is None:
+            print(f"- Missing `#ifndef {guard}`", file=sys.stderr)
+        if define is None:
+            print(f"- Missing `#define {guard}`", file=sys.stderr)
+        if endif is None:
+            print(f"- Missing `#endif  // {guard}`", file=sys.stderr)
+        return True
+
+    if ifndef.line + 1 != define.line:
+        print(
+            f"Non-consecutive header guard in {path}: "
+            f"#ifndef on line {ifndef.line + 1}, "
+            f"#define on line {define.line + 1}.",
+            file=sys.stderr,
+        )
+        return True
+
+    if endif.line != len(lines) - 1:
+        print(
+            f"Misordered header guard in {path}: #endif on line {endif.line}, "
+            f"should be on last line ({len(lines) - 1}).",
+            file=sys.stderr,
+        )
+        return True
+
+    if guard != ifndef.guard or guard != define.guard or guard != endif.guard:
+        print(f"Fixing header guard in {path} to {guard}:", file=sys.stderr)
+        maybe_replace(lines, ifndef, "#ifndef", guard)
+        maybe_replace(lines, define, "#define", guard)
+        maybe_replace(lines, endif, "#endif  //", guard)
+        with path.open("w") as f:
+            f.writelines(lines)
+        return True
+
+    return False
+
+
+def main() -> None:
+    has_errors = False
+    for arg in sys.argv[1:]:
+        if check_path(Path(arg)):
+            has_errors = True
+    if has_errors:
+        exit(1)
+
+
+if __name__ == "__main__":
+    main()

toolchain/lexer/string_literal.h

@@ -2,6 +2,9 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
+#ifndef TOOLCHAIN_LEXER_STRING_LITERAL_H_
+#define TOOLCHAIN_LEXER_STRING_LITERAL_H_
+
 #include <string>
 
 #include "llvm/ADT/Optional.h"
@@ -58,3 +61,5 @@ class LexedStringLiteral {
 };
 
 }  // namespace Carbon
+
+#endif  // TOOLCHAIN_LEXER_STRING_LITERAL_H_

toolchain/lexer/test_helpers.h

@@ -53,4 +53,4 @@ class SingleTokenDiagnosticTranslator
 
 }  // namespace Carbon::Testing
 
-#endif  // TOOLCHAIN_LEXER_TOKENIZED_BUFFER_TEST_HELPERS_H_
+#endif  // TOOLCHAIN_LEXER_TEST_HELPERS_H_

toolchain/source/source_buffer.h

@@ -2,8 +2,8 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
-#ifndef TOOLCHAIN_SOURCE_SOURCEBUFFER_H_
-#define TOOLCHAIN_SOURCE_SOURCEBUFFER_H_
+#ifndef TOOLCHAIN_SOURCE_SOURCE_BUFFER_H_
+#define TOOLCHAIN_SOURCE_SOURCE_BUFFER_H_
 
 #include <string>
 #include <utility>
@@ -74,4 +74,4 @@ class SourceBuffer {
 
 }  // namespace Carbon
 
-#endif  // TOOLCHAIN_SOURCE_SOURCEBUFFER_H_
+#endif  // TOOLCHAIN_SOURCE_SOURCE_BUFFER_H_