[clang-tidy] Add 'included from' details to warning message.

[sbenza]

Summary

Add 'included from' details to warning message to google-global-names-in-headers.
It should make it clearer on those cases where a non-header is being mistakenly #included.

[alexfh]

clang-tidy/google/GlobalNamesInHeadersCheck.cpp

49

Static functions are preferred in LLVM. http://llvm.org/docs/CodingStandards.html#anonymous-namespaces

"make anonymous namespaces as small as possible, and only use them for class declarations. "

96

It's more common to present this kind of additional information as notes. As a bonus, the location of the note can point to the #include directive, which will make the whole diagnostic more useful.

Also, we might want to figure out why clang-tidy doesn't print "in file included from ..." itself, as clang does (but until then we can have this hand-rolled implementation).

[alexfh]

Ping ;)
Do you have time to finish this?

clang-tidy/google/GlobalNamesInHeadersCheck.cpp

 void
 GlobalNamesInHeadersCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
   Finder->addMatcher(
       decl(anyOf(usingDecl(), usingDirectiveDecl()),
            hasDeclContext(translationUnitDecl())).bind("using_decl"),
       this);
 }
 
+namespace {
+std::string getIncludeFromMessage(SourceLocation DeclLoc,
+                                  const SourceManager &SM) {
+  FileID FID = SM.getFileID(DeclLoc);
+  bool Invalid;
+  const auto &Entry = SM.getSLocEntry(FID, &Invalid);
+  if (Invalid)
+    return "";
+  SourceLocation IncludeLoc = Entry.getFile().getIncludeLoc();
+  if (!IncludeLoc.isValid())
+    return "";
+  StringRef Filename = SM.getFilename(IncludeLoc);
+  if (Filename.empty())
+    return "";
+  return (Twine("; in file included from '") + Filename + "'").str();
+}
+}  // namespace
+
 void GlobalNamesInHeadersCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *D = Result.Nodes.getNodeAs<Decl>("using_decl");
   // If it comes from a macro, we'll assume it is fine.
   if (D->getLocStart().isMacroID())
     return;
 
   // Ignore if it comes from the "main" file ...
   if (Result.SourceManager->isInMainFile(
if (UsingDirective->getNominatedNamespace()->isAnonymousNamespace()) {
       // Anynoumous namespaces inject a using directive into the AST to import
       // the names into the containing namespace.
       // We should not have them in headers, but there is another warning for
       // that.
       return;
     }
   }
 
+  std::string IncludeMessage =
+      getIncludeFromMessage(D->getLocStart(), *Result.SourceManager);
+
   diag(D->getLocStart(),
-       "using declarations in the global namespace in headers are prohibited");
+       "using declarations in the global namespace in headers are prohibited%0")
+      << IncludeMessage;
 }
 
 } // namespace readability
 } // namespace google
 } // namespace tidy
 } // namespace clang

unittests/clang-tidy/GoogleModuleTest.cpp

EXPECT_EQ(
       "A(Foo);",
       runCheckOnCode<ExplicitConstructorCheck>(
           "#define A(T) class T##Bar { explicit T##Bar(const T##Bar &b) {} };\n"
           "A(Foo);"));
 }
 
 class GlobalNamesInHeadersCheckTest : public ::testing::Test {
 protected:
-  bool runCheckOnCode(const std::string &Code, const std::string &Filename) {
+  bool runCheckOnCode(const std::string &Code, const std::string &Filename,
+                      bool IsIncluded = false) {
     static const char *const Header = "namespace std {\n"
                                       "class string {};\n"
                                       "}  // namespace std\n"
+                                      "#include \"header.h\"\n"
                                       "\n"
                                       "#define SOME_MACRO(x) using x\n";
     std::vector<ClangTidyError> Errors;
     std::vector<std::string> Args;
     if (!StringRef(Filename).endswith(".cpp")) {
       Args.emplace_back("-xc++-header");
     }
+    std::map<StringRef, StringRef> Paths;
+    Paths["header.h"] = IsIncluded ? StringRef(Code) : StringRef("\n");
+    ClangTidyOptions Options;
+    Options.HeaderFilterRegex = ".*";
     test::runCheckOnCode<readability::GlobalNamesInHeadersCheck>(
-        Header + Code, &Errors, Filename, Args);
+        Header + (IsIncluded ? "" : Code), &Errors,
+        IsIncluded ? "main.cpp" : Filename, Args, Options, Paths);
     if (Errors.empty())
       return false;
     assert(Errors.size() == 1);
-    assert(
-        Errors[0].Message.Message ==
-        "using declarations in the global namespace in headers are prohibited");
+    std::string Expected =
+        "using declarations in the global namespace in headers are prohibited";
+    if (IsIncluded)
+      Expected += "; in file included from 'main.cpp'";
+    assert(Errors[0].Message.Message == Expected);
     return true;
   }
 };
 
 TEST_F(GlobalNamesInHeadersCheckTest, UsingDeclarations) {
   EXPECT_TRUE(runCheckOnCode("using std::string;", "foo.h"));
   EXPECT_FALSE(runCheckOnCode("using std::string;", "foo.cpp"));
   EXPECT_FALSE(runCheckOnCode("namespace my_namespace {\n"
                               "using std::string;\n"
                               "}  // my_namespace\n",
                               "foo.h"));
   EXPECT_FALSE(runCheckOnCode("SOME_MACRO(std::string);", "foo.h"));
+  EXPECT_TRUE(runCheckOnCode("using std::string;", "foo.h", true));
 }
 
 TEST_F(GlobalNamesInHeadersCheckTest, UsingDirectives) {
   EXPECT_TRUE(runCheckOnCode("using namespace std;", "foo.h"));
   EXPECT_FALSE(runCheckOnCode("using namespace std;", "foo.cpp"));
   EXPECT_FALSE(runCheckOnCode("namespace my_namespace {\n"
                               "using namespace std;\n"
                               "}  // my_namespace\n",
                               "foo.h"));
   EXPECT_FALSE(runCheckOnCode("SOME_MACRO(namespace std);", "foo.h"));
+  EXPECT_TRUE(runCheckOnCode("using namespace std;", "foo.h", true));
 }
 
 TEST_F(GlobalNamesInHeadersCheckTest, RegressionAnonymousNamespace) {
   EXPECT_FALSE(runCheckOnCode("namespace {}", "foo.h"));
 }
 
 } // namespace test
 } // namespace tidy
 } // namespace clang