[clang-tidy] Add a generic header guard checker + LLVM implementation.

The implementation is split into a generic part and a LLVM-specific part.
Other codebases can implement it with their own style. The specific features
supported are:

- Verification (and fixing) of header guards against a style based on the file path
- Automatic insertion of header guards for headers that are missing them
- A warning when the header guard doesn't enable our fancy header guard optimization
(e.g. when there's an include preceeding the guard)
- Automatic insertion of a comment with the guard name after #endif.

For the LLVM style we disable #endif comments for now, they're not very common
in the codebase. We also only flag headers in the include directories, there
doesn't seem to be a common style outside.

Differential Revision: http://reviews.llvm.org/D4867

llvm-svn: 215548

clang-tools-extra/clang-tidy/CMakeLists.txt

@@ -27,3 +27,4 @@ add_subdirectory(tool)
 add_subdirectory(llvm)
 add_subdirectory(google)
 add_subdirectory(misc)
+add_subdirectory(utils)

clang-tools-extra/clang-tidy/Makefile

@@ -11,6 +11,6 @@ CLANG_LEVEL := ../../..
 LIBRARYNAME := clangTidy
 include $(CLANG_LEVEL)/../../Makefile.config
 
-DIRS = llvm google misc tool
+DIRS = llvm google misc tool utils
 
 include $(CLANG_LEVEL)/Makefile

clang-tools-extra/clang-tidy/llvm/CMakeLists.txt

@@ -1,6 +1,7 @@
 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyLLVMModule
+  HeaderGuardCheck.cpp
   IncludeOrderCheck.cpp
   LLVMTidyModule.cpp
   NamespaceCommentCheck.cpp
@@ -12,4 +13,5 @@ add_clang_library(clangTidyLLVMModule
   clangBasic
   clangLex
   clangTidy
+  clangTidyUtils
   )

clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.cpp

@@ -0,0 +1,53 @@
+//===--- HeaderGuardCheck.cpp - clang-tidy --------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "HeaderGuardCheck.h"
+
+namespace clang {
+namespace tidy {
+
+bool LLVMHeaderGuardCheck::shouldFixHeaderGuard(StringRef Filename) {
+  return Filename.endswith(".h");
+}
+
+std::string LLVMHeaderGuardCheck::getHeaderGuard(StringRef Filename,
+                                                 StringRef OldGuard) {
+  std::string Guard = tooling::getAbsolutePath(Filename);
+
+  // Sanitize the path. There are some rules for compatibility with the historic
+  // style in include/llvm and include/clang which we want to preserve.
+
+  // We don't want _INCLUDE_ in our guards.
+  size_t PosInclude = Guard.rfind("include/");
+  if (PosInclude != StringRef::npos)
+    Guard = Guard.substr(PosInclude + std::strlen("include/"));
+
+  // For clang we drop the _TOOLS_.
+  size_t PosToolsClang = Guard.rfind("tools/clang/");
+  if (PosToolsClang != StringRef::npos)
+    Guard = Guard.substr(PosToolsClang + std::strlen("tools/"));
+
+  // The remainder is LLVM_FULL_PATH_TO_HEADER_H
+  size_t PosLLVM = Guard.rfind("llvm/");
+  if (PosLLVM != StringRef::npos)
+    Guard = Guard.substr(PosLLVM);
+
+  std::replace(Guard.begin(), Guard.end(), '/', '_');
+  std::replace(Guard.begin(), Guard.end(), '.', '_');
+  std::replace(Guard.begin(), Guard.end(), '-', '_');
+
+  // The prevalent style in clang is LLVM_CLANG_FOO_BAR_H
+  if (StringRef(Guard).startswith("clang"))
+    Guard = "LLVM_" + Guard;
+
+  return StringRef(Guard).upper();
+}
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/llvm/HeaderGuardCheck.h

@@ -0,0 +1,29 @@
+//===--- HeaderGuardCheck.h - clang-tidy ------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H
+
+#include "../utils/HeaderGuard.h"
+
+namespace clang {
+namespace tidy {
+
+/// Finds and fixes header guards that do not adhere to LLVM style.
+class LLVMHeaderGuardCheck : public HeaderGuardCheck {
+public:
+  bool shouldSuggestEndifComment(StringRef Filename) override { return false; }
+  bool shouldFixHeaderGuard(StringRef Filename) override;
+  std::string getHeaderGuard(StringRef Filename, StringRef OldGuard) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_HEADER_GUARD_CHECK_H

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

@@ -10,6 +10,7 @@
 #include "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
+#include "HeaderGuardCheck.h"
 #include "IncludeOrderCheck.h"
 #include "NamespaceCommentCheck.h"
 #include "TwineLocalCheck.h"
@@ -20,6 +21,8 @@ namespace tidy {
 class LLVMModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+    CheckFactories.addCheckFactory(
+        "llvm-header-guard", new ClangTidyCheckFactory<LLVMHeaderGuardCheck>());
     CheckFactories.addCheckFactory(
         "llvm-include-order", new ClangTidyCheckFactory<IncludeOrderCheck>());
     CheckFactories.addCheckFactory(

clang-tools-extra/clang-tidy/utils/CMakeLists.txt

@@ -0,0 +1,12 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyUtils
+  HeaderGuard.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  )

clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp

@@ -0,0 +1,257 @@
+//===--- HeaderGuard.cpp - clang-tidy -------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "HeaderGuard.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Path.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief canonicalize a path by removing ./ and ../ components.
+// FIXME: Consider moving this to llvm::sys::path.
+static std::string cleanPath(StringRef Path) {
+  SmallString<256> NewPath;
+  for (auto I = llvm::sys::path::begin(Path), E = llvm::sys::path::end(Path);
+       I != E; ++I) {
+    if (*I == ".")
+      continue;
+    if (*I == "..") {
+      // Drop the last component.
+      NewPath.resize(llvm::sys::path::parent_path(NewPath).size());
+    } else {
+      if (!NewPath.empty())
+        NewPath += '/';
+      NewPath += *I;
+    }
+  }
+  return NewPath.str();
+}
+
+namespace {
+class HeaderGuardPPCallbacks : public PPCallbacks {
+public:
+  explicit HeaderGuardPPCallbacks(Preprocessor *PP, HeaderGuardCheck *Check)
+      : PP(PP), Check(Check) {}
+
+  void FileChanged(SourceLocation Loc, FileChangeReason Reason,
+                   SrcMgr::CharacteristicKind FileType,
+                   FileID PrevFID) override {
+    // Record all files we enter. We'll need them to diagnose headers without
+    // guards.
+    SourceManager &SM = PP->getSourceManager();
+    if (Reason == EnterFile && FileType == SrcMgr::C_User) {
+      if (const FileEntry *FE = SM.getFileEntryForID(SM.getFileID(Loc))) {
+        std::string FileName = cleanPath(FE->getName());
+        Files[FileName] = FE;
+      }
+    }
+  }
+
+  void Ifndef(SourceLocation Loc, const Token &MacroNameTok,
+              const MacroDirective *MD) override {
+    if (MD)
+      return;
+
+    // Record #ifndefs that succeeded. We also need the Location of the Name.
+    Ifndefs[MacroNameTok.getIdentifierInfo()] =
+        std::make_pair(Loc, MacroNameTok.getLocation());
+  }
+
+  void MacroDefined(const Token &MacroNameTok,
+                    const MacroDirective *MD) override {
+    // Record all defined macros. We store the whole token to get info on the
+    // name later.
+    Macros.emplace_back(MacroNameTok, MD);
+  }
+
+  void Endif(SourceLocation Loc, SourceLocation IfLoc) override {
+    // Record all #endif and the corresponding #ifs (including #ifndefs).
+    EndIfs[IfLoc] = Loc;
+  }
+
+  void EndOfMainFile() override {
+    // Now that we have all this information from the preprocessor, use it!
+    SourceManager &SM = PP->getSourceManager();
+
+    for (const auto &MacroEntry : Macros) {
+      const MacroInfo *MI = MacroEntry.second->getMacroInfo();
+
+      // We use clang's header guard detection. This has the advantage of also
+      // emitting a warning for cases where a pseudo header guard is found but
+      // preceeded by something blocking the header guard optimization.
+      if (!MI->isUsedForHeaderGuard())
+        continue;
+
+      const FileEntry *FE =
+          SM.getFileEntryForID(SM.getFileID(MI->getDefinitionLoc()));
+      std::string FileName = cleanPath(FE->getName());
+      Files.erase(FileName);
+
+      // See if we should check and fix this header guard.
+      if (!Check->shouldFixHeaderGuard(FileName))
+        continue;
+
+      // Look up Locations for this guard.
+      SourceLocation Ifndef =
+          Ifndefs[MacroEntry.first.getIdentifierInfo()].second;
+      SourceLocation Define = MacroEntry.first.getLocation();
+      SourceLocation EndIf =
+          EndIfs[Ifndefs[MacroEntry.first.getIdentifierInfo()].first];
+
+      // If the macro Name is not equal to what we can compute, correct it in
+      // the
+      // #ifndef and #define.
+      StringRef CurHeaderGuard =
+          MacroEntry.first.getIdentifierInfo()->getName();
+      std::string NewGuard =
+          checkHeaderGuardDefinition(Ifndef, Define, FileName, CurHeaderGuard);
+
+      // Now look at the #endif. We want a comment with the header guard. Fix it
+      // at the slightest deviation.
+      if (Check->shouldSuggestEndifComment(FileName))
+        checkEndifComment(EndIf, NewGuard);
+    }
+
+    // Emit warnings for headers that are missing guards.
+    checkGuardlessHeaders();
+
+    // Clear all state.
+    Macros.clear();
+    Files.clear();
+    Ifndefs.clear();
+    EndIfs.clear();
+  }
+
+  /// \brief Look for header guards that don't match the preferred style. Emit
+  /// fix-its and return the suggested header guard (or the original if no
+  /// change was made.
+  std::string checkHeaderGuardDefinition(SourceLocation Ifndef,
+                                         SourceLocation Define,
+                                         StringRef FileName,
+                                         StringRef CurHeaderGuard) {
+    std::string CPPVar = Check->getHeaderGuard(FileName, CurHeaderGuard);
+    std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
+    if (Ifndef.isValid() && CurHeaderGuard != CPPVar &&
+        CurHeaderGuard != CPPVarUnder) {
+      Check->diag(Ifndef, "header guard does not follow preferred style")
+          << FixItHint::CreateReplacement(
+                 CharSourceRange::getTokenRange(
+                     Ifndef, Ifndef.getLocWithOffset(CurHeaderGuard.size())),
+                 CPPVar)
+          << FixItHint::CreateReplacement(
+                 CharSourceRange::getTokenRange(
+                     Define, Define.getLocWithOffset(CurHeaderGuard.size())),
+                 CPPVar);
+      return CPPVar;
+    }
+    return CurHeaderGuard;
+  }
+
+  /// \brief Checks the comment after the #endif of a header guard and fixes it
+  /// if it doesn't match \c HeaderGuard.
+  void checkEndifComment(SourceLocation EndIf, StringRef HeaderGuard) {
+    const char *EndIfData = PP->getSourceManager().getCharacterData(EndIf);
+    size_t EndIfLen = std::strcspn(EndIfData, "\r\n");
+
+    StringRef EndIfStr(EndIfData, EndIfLen);
+    if (EndIf.isValid() && !EndIfStr.endswith("// " + HeaderGuard.str())) {
+      std::string Correct = "endif  // " + HeaderGuard.str();
+      Check->diag(EndIf, "#endif for a header guard should reference the "
+                         "guard macro in a comment")
+          << FixItHint::CreateReplacement(
+              CharSourceRange::getCharRange(EndIf,
+                                            EndIf.getLocWithOffset(EndIfLen)),
+              Correct);
+    }
+  }
+
+  /// \brief Looks for files that were visited but didn't have a header guard.
+  /// Emits a warning with fixits suggesting adding one.
+  void checkGuardlessHeaders() {
+    // Look for header files that didn't have a header guard. Emit a warning and
+    // fix-its to add the guard.
+    // TODO: Insert the guard after top comments.
+    for (const auto &FE : Files) {
+      StringRef FileName = FE.getKey();
+      if (!Check->shouldSuggestToAddHeaderGuard(FileName))
+        continue;
+
+      SourceManager &SM = PP->getSourceManager();
+      FileID FID = SM.translateFile(FE.getValue());
+      SourceLocation StartLoc = SM.getLocForStartOfFile(FID);
+      if (StartLoc.isInvalid())
+        continue;
+
+      std::string CPPVar = Check->getHeaderGuard(FileName);
+      std::string CPPVarUnder = CPPVar + '_'; // Allow a trailing underscore.
+      // If there is a header guard macro but it's not in the topmost position
+      // emit a plain warning without fix-its. This often happens when the guard
+      // macro is preceeded by includes.
+      // FIXME: Can we move it into the right spot?
+      bool SeenMacro = false;
+      for (const auto &MacroEntry : Macros) {
+        StringRef Name = MacroEntry.first.getIdentifierInfo()->getName();
+        SourceLocation DefineLoc = MacroEntry.first.getLocation();
+        if ((Name == CPPVar || Name == CPPVarUnder) &&
+            SM.isWrittenInSameFile(StartLoc, DefineLoc)) {
+          Check->diag(
+              DefineLoc,
+              "Header guard after code/includes. Consider moving it up.");
+          SeenMacro = true;
+          break;
+        }
+      }
+
+      if (SeenMacro)
+        continue;
+
+      Check->diag(StartLoc, "header is missing header guard")
+          << FixItHint::CreateInsertion(
+                 StartLoc, "#ifndef " + CPPVar + "\n#define " + CPPVar + "\n\n")
+          << FixItHint::CreateInsertion(
+                 SM.getLocForEndOfFile(FID),
+                 Check->shouldSuggestEndifComment(FileName)
+                     ? "\n#endif  // " + CPPVar + "\n"
+                     : "\n#endif\n");
+    }
+  }
+
+private:
+  std::vector<std::pair<Token, const MacroDirective *>> Macros;
+  llvm::StringMap<const FileEntry *> Files;
+  std::map<const IdentifierInfo *, std::pair<SourceLocation, SourceLocation>>
+      Ifndefs;
+  std::map<SourceLocation, SourceLocation> EndIfs;
+
+  Preprocessor *PP;
+  HeaderGuardCheck *Check;
+};
+} // namespace
+
+void HeaderGuardCheck::registerPPCallbacks(CompilerInstance &Compiler) {
+  Compiler.getPreprocessor().addPPCallbacks(
+      new HeaderGuardPPCallbacks(&Compiler.getPreprocessor(), this));
+}
+
+bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) {
+  return FileName.endswith(".h");
+}
+
+bool HeaderGuardCheck::shouldFixHeaderGuard(StringRef FileName) { return true; }
+
+bool HeaderGuardCheck::shouldSuggestToAddHeaderGuard(StringRef FileName) {
+  return FileName.endswith(".h");
+}
+
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/utils/HeaderGuard.h

@@ -0,0 +1,41 @@
+//===--- HeaderGuard.h - clang-tidy -----------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Finds and fixes header guards.
+class HeaderGuardCheck : public ClangTidyCheck {
+public:
+  void registerPPCallbacks(CompilerInstance &Compiler) override;
+
+  /// \brief Returns true if the checker should suggest inserting a trailing
+  /// comment on the #endif of the header guard. It will use the same name as
+  /// returned by getHeaderGuard.
+  virtual bool shouldSuggestEndifComment(StringRef Filename);
+  /// \brief Returns true if the checker should suggest changing an existing
+  /// header guard to the string returned by getHeaderGuard.
+  virtual bool shouldFixHeaderGuard(StringRef Filename);
+  /// \brief Returns true if the checker should add a header guard to the file
+  /// if it has none.
+  virtual bool shouldSuggestToAddHeaderGuard(StringRef Filename);
+  /// \brief Get the canonical header guard for a file.
+  virtual std::string getHeaderGuard(StringRef Filename,
+                                     StringRef OldGuard = StringRef()) = 0;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_UTILS_HEADER_GUARD_H

clang-tools-extra/clang-tidy/utils/Makefile

@@ -0,0 +1,12 @@
+##===- clang-tidy/google/Makefile --------------------------*- Makefile -*-===##
+#
+#                     The LLVM Compiler Infrastructure
+#
+# This file is distributed under the University of Illinois Open Source
+# License. See LICENSE.TXT for details.
+#
+##===----------------------------------------------------------------------===##
+CLANG_LEVEL := ../../../..
+LIBRARYNAME := clangTidyUtils
+
+include $(CLANG_LEVEL)/Makefile

clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp

@@ -1,4 +1,5 @@
 #include "ClangTidyTest.h"
+#include "llvm/HeaderGuardCheck.h"
 #include "llvm/IncludeOrderCheck.h"
 #include "llvm/NamespaceCommentCheck.h"
 #include "gtest/gtest.h"
@@ -85,6 +86,46 @@ TEST(NamespaceCommentCheckTest, FixWrongComments) {
                                                   "} // namespace asdf"));
 }
 
+static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename) {
+  return test::runCheckOnCode<LLVMHeaderGuardCheck>(
+      Code, /*Errors=*/nullptr, Filename, std::string("-xc++-header"));
+}
+
+TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
+  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n#define LLVM_ADT_FOO_H\n#endif\n",
+            runHeaderGuardCheck("#ifndef FOO\n#define FOO\n#endif\n",
+                                "include/llvm/ADT/foo.h"));
+
+  // Allow trailing underscores.
+  EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
+            runHeaderGuardCheck(
+                "#ifndef LLVM_ADT_FOO_H_\n#define LLVM_ADT_FOO_H_\n#endif\n",
+                "include/llvm/ADT/foo.h"));
+
+  EXPECT_EQ("#ifndef LLVM_CLANG_C_BAR_H\n#define LLVM_CLANG_C_BAR_H\n\n\n#endif\n",
+            runHeaderGuardCheck("", "./include/clang-c/bar.h"));
+
+  EXPECT_EQ("#ifndef LLVM_CLANG_LIB_CODEGEN_C_H\n#define "
+            "LLVM_CLANG_LIB_CODEGEN_C_H\n\n\n#endif\n",
+            runHeaderGuardCheck("", "tools/clang/lib/CodeGen/c.h"));
+
+  EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n#define "
+            "LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n\n\n#endif\n",
+            runHeaderGuardCheck("", "tools/clang/tools/extra/clang-tidy/x.h"));
+
+  EXPECT_EQ(
+      "int foo;\n#ifndef LLVM_CLANG_BAR_H\n#define LLVM_CLANG_BAR_H\n#endif\n",
+      runHeaderGuardCheck("int foo;\n#ifndef LLVM_CLANG_BAR_H\n"
+                          "#define LLVM_CLANG_BAR_H\n#endif\n",
+                          "include/clang/bar.h"));
+
+  EXPECT_EQ("#ifndef LLVM_CLANG_BAR_H\n#define LLVM_CLANG_BAR_H\n\n"
+            "int foo;\n#ifndef FOOLOLO\n#define FOOLOLO\n#endif\n\n#endif\n",
+            runHeaderGuardCheck(
+                "int foo;\n#ifndef FOOLOLO\n#define FOOLOLO\n#endif\n",
+                "include/clang/bar.h"));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang