[clang-tidy] Reword the "code outside header guard" warning.

The check doesn't really know if the code it is warning about came before
or after the header guard, so phrase it more neutral instead of complaining
about code before the header guard. The location for the warning is still
not optimal, but I don't think fixing that is worth the effort, the
preprocessor doesn't give us a better location.

Differential Revision: https://reviews.llvm.org/D30191

llvm-svn: 295715

1 files changed, 89 insertions, 59 deletions

diff --git a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
index 1d7fb24098a..8ab207d9c53 100644
--- a/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
+++ b/clang-tools-extra/unittests/clang-tidy/LLVMModuleTest.cpp
@@ -12,11 +12,16 @@ namespace test {
 // FIXME: It seems this might be incompatible to dos path. Investigating.
 #if !defined(_WIN32)
 static std::string runHeaderGuardCheck(StringRef Code, const Twine &Filename,
-                                       unsigned ExpectedWarnings) {
+                                       Optional<StringRef> ExpectedWarning) {
   std::vector<ClangTidyError> Errors;
   std::string Result = test::runCheckOnCode<LLVMHeaderGuardCheck>(
       Code, &Errors, Filename, std::string("-xc++-header"));
-  return Errors.size() == ExpectedWarnings ? Result : "invalid error count";
+  if (Errors.size() != (size_t)ExpectedWarning.hasValue())
+    return "invalid error count";
+  if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
+    return "expected: '" + ExpectedWarning->str() + "', saw: '" +
+           Errors.back().Message.Message + "'";
+  return Result;
 }
 
 namespace {
@@ -27,24 +32,30 @@ struct WithEndifComment : public LLVMHeaderGuardCheck {
 };
 } // namespace
 
-static std::string runHeaderGuardCheckWithEndif(StringRef Code,
-                                                const Twine &Filename,
-                                                unsigned ExpectedWarnings) {
+static std::string
+runHeaderGuardCheckWithEndif(StringRef Code, const Twine &Filename,
+                             Optional<StringRef> ExpectedWarning) {
   std::vector<ClangTidyError> Errors;
   std::string Result = test::runCheckOnCode<WithEndifComment>(
       Code, &Errors, Filename, std::string("-xc++-header"));
-  return Errors.size() == ExpectedWarnings ? Result : "invalid error count";
+  if (Errors.size() != (size_t)ExpectedWarning.hasValue())
+    return "invalid error count";
+  if (ExpectedWarning && *ExpectedWarning != Errors.back().Message.Message)
+    return "expected: '" + ExpectedWarning->str() + "', saw: '" +
+           Errors.back().Message.Message + "'";
+  return Result;
 }
 
 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",
-                                /*ExpectedWarnings=*/1));
+            runHeaderGuardCheck(
+                "#ifndef FOO\n"
+                "#define FOO\n"
+                "#endif\n",
+                "include/llvm/ADT/foo.h",
+                StringRef("header guard does not follow preferred style")));
 
   // Allow trailing underscores.
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n"
@@ -53,8 +64,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
             runHeaderGuardCheck("#ifndef LLVM_ADT_FOO_H_\n"
                                 "#define LLVM_ADT_FOO_H_\n"
                                 "#endif\n",
-                                "include/llvm/ADT/foo.h",
-                                /*ExpectedWarnings=*/0));
+                                "include/llvm/ADT/foo.h", None));
 
   EXPECT_EQ("#ifndef LLVM_CLANG_C_BAR_H\n"
             "#define LLVM_CLANG_C_BAR_H\n"
@@ -62,7 +72,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
             "\n"
             "#endif\n",
             runHeaderGuardCheck("", "./include/clang-c/bar.h",
-                                /*ExpectedWarnings=*/1));
+                                StringRef("header is missing header guard")));
 
   EXPECT_EQ("#ifndef LLVM_CLANG_LIB_CODEGEN_C_H\n"
             "#define LLVM_CLANG_LIB_CODEGEN_C_H\n"
@@ -70,7 +80,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
             "\n"
             "#endif\n",
             runHeaderGuardCheck("", "tools/clang/lib/CodeGen/c.h",
-                                /*ExpectedWarnings=*/1));
+                                StringRef("header is missing header guard")));
 
   EXPECT_EQ("#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n"
             "#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_X_H\n"
@@ -78,17 +88,33 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
             "\n"
             "#endif\n",
             runHeaderGuardCheck("", "tools/clang/tools/extra/clang-tidy/x.h",
-                                /*ExpectedWarnings=*/1));
-
-  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", /*ExpectedWarnings=*/1));
+                                StringRef("header is missing header guard")));
+
+  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",
+                          StringRef("code/includes outside of area guarded by "
+                                    "header guard; consider moving it")));
+
+  EXPECT_EQ(
+      "#ifndef LLVM_CLANG_BAR_H\n"
+      "#define LLVM_CLANG_BAR_H\n"
+      "#endif\n"
+      "int foo;\n",
+      runHeaderGuardCheck("#ifndef LLVM_CLANG_BAR_H\n"
+                          "#define LLVM_CLANG_BAR_H\n"
+                          "#endif\n"
+                          "int foo;\n",
+                          "include/clang/bar.h",
+                          StringRef("code/includes outside of area guarded by "
+                                    "header guard; consider moving it")));
 
   EXPECT_EQ("#ifndef LLVM_CLANG_BAR_H\n"
             "#define LLVM_CLANG_BAR_H\n"
@@ -103,35 +129,40 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
                                 "#ifndef FOOLOLO\n"
                                 "#define FOOLOLO\n"
                                 "#endif\n",
-                                "include/clang/bar.h", /*ExpectedWarnings=*/1));
+                                "include/clang/bar.h",
+                                StringRef("header is missing header guard")));
 
   // Fix incorrect #endif comments even if we shouldn't add new ones.
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
             "#endif // LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheck("#ifndef FOO\n"
-                                "#define FOO\n"
-                                "#endif // FOO\n",
-                                "include/llvm/ADT/foo.h",
-                                /*ExpectedWarnings=*/1));
+            runHeaderGuardCheck(
+                "#ifndef FOO\n"
+                "#define FOO\n"
+                "#endif // FOO\n",
+                "include/llvm/ADT/foo.h",
+                StringRef("header guard does not follow preferred style")));
 
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
             "#endif // LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheckWithEndif("#ifndef FOO\n"
-                                         "#define FOO\n"
-                                         "#endif\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/1));
+            runHeaderGuardCheckWithEndif(
+                "#ifndef FOO\n"
+                "#define FOO\n"
+                "#endif\n",
+                "include/llvm/ADT/foo.h",
+                StringRef("header guard does not follow preferred style")));
 
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
             "#endif // LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n"
-                                         "#define LLVM_ADT_FOO_H\n"
-                                         "#endif // LLVM_H\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/1));
+            runHeaderGuardCheckWithEndif(
+                "#ifndef LLVM_ADT_FOO_H\n"
+                "#define LLVM_ADT_FOO_H\n"
+                "#endif // LLVM_H\n",
+                "include/llvm/ADT/foo.h",
+                StringRef("#endif for a header guard should reference the "
+                          "guard macro in a comment")));
 
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
@@ -139,8 +170,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
             runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n"
                                          "#define LLVM_ADT_FOO_H\n"
                                          "#endif /* LLVM_ADT_FOO_H */\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/0));
+                                         "include/llvm/ADT/foo.h", None));
 
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H_\n"
             "#define LLVM_ADT_FOO_H_\n"
@@ -148,28 +178,29 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
             runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H_\n"
                                          "#define LLVM_ADT_FOO_H_\n"
                                          "#endif // LLVM_ADT_FOO_H_\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/0));
+                                         "include/llvm/ADT/foo.h", None));
 
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
             "#endif // LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H_\n"
-                                         "#define LLVM_ADT_FOO_H_\n"
-                                         "#endif // LLVM\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/1));
+            runHeaderGuardCheckWithEndif(
+                "#ifndef LLVM_ADT_FOO_H_\n"
+                "#define LLVM_ADT_FOO_H_\n"
+                "#endif // LLVM\n",
+                "include/llvm/ADT/foo.h",
+                StringRef("header guard does not follow preferred style")));
 
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
             "#endif \\ \n"
             "// LLVM_ADT_FOO_H\n",
-            runHeaderGuardCheckWithEndif("#ifndef LLVM_ADT_FOO_H\n"
-                                         "#define LLVM_ADT_FOO_H\n"
-                                         "#endif \\ \n"
-                                         "// LLVM_ADT_FOO_H\n",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/1));
+            runHeaderGuardCheckWithEndif(
+                "#ifndef LLVM_ADT_FOO_H\n"
+                "#define LLVM_ADT_FOO_H\n"
+                "#endif \\ \n"
+                "// LLVM_ADT_FOO_H\n",
+                "include/llvm/ADT/foo.h",
+                StringRef("backslash and newline separated by space")));
 
   EXPECT_EQ("#ifndef LLVM_ADT_FOO_H\n"
             "#define LLVM_ADT_FOO_H\n"
@@ -179,8 +210,7 @@ TEST(LLVMHeaderGuardCheckTest, FixHeaderGuards) {
                                          "#define LLVM_ADT_FOO_H\n"
                                          "#endif  /* LLVM_ADT_FOO_H\\ \n"
                                          " FOO */",
-                                         "include/llvm/ADT/foo.h",
-                                         /*ExpectedWarnings=*/0));
+                                         "include/llvm/ADT/foo.h", None));
 }
 #endif