Add clang-tidy check google-global-names-in-headers.

[sbenza]

Summary

google-global-names-in-headers flags global namespace pollution in header files.
Right now it only triggers on using declarations and directives.

[alexfh]

Looks good modulo comments. Thanks!

clang-tidy/google/GlobalNamesInHeadersCheck.cpp

31 ↗

(On Diff #19756)

nit: const auto*?

36 ↗

(On Diff #19756)

nit: I'd put ellipsis in the end of the comment to avoid confusion.

43 ↗

(On Diff #19756)

nit: No braces needed in the LLVM style.

49 ↗

(On Diff #19756)

The message reads as if clang-tidy didn't find the aforementioned using declaration and wants you to insert one. Maybe "using declarations in the global namespace in headers are prohibited" or something similar? I'd even go with "don't use using declarations in ...", it's an error message, not a novel after all ;)

[sbenza]

Minor fixes.

clang-tidy/google/GlobalNamesInHeadersCheck.cpp

31 ↗

(On Diff #19756)

Done

36 ↗

(On Diff #19756)

Done.

43 ↗

(On Diff #19756)

Done.

49 ↗

(On Diff #19756)

Done.

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

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyGoogleModule
   AvoidCStyleCastsCheck.cpp
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp
+  GlobalNamesInHeadersCheck.cpp
   GoogleTidyModule.cpp
   IntegerTypesCheck.cpp
   MemsetZeroLengthCheck.cpp
   NamedParameterCheck.cpp
   OverloadedUnaryAndCheck.cpp
   StringReferenceMemberCheck.cpp
   TodoCommentCheck.cpp
   UnnamedNamespaceInHeaderCheck.cpp

clang-tools-extra/trunk/clang-tidy/google/GlobalNamesInHeadersCheck.h

+//===--- GlobalNamesInHeadersCheck.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_GOOGLE_GLOBAL_NAMES_IN_HEADERS_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBAL_NAMES_IN_HEADERS_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+// Flag global namespace pollution in header files.
+// Right now it only triggers on using declarations and directives.
+class GlobalNamesInHeadersCheck : public ClangTidyCheck {
+public:
+  GlobalNamesInHeadersCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif  // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_GLOBAL_NAMES_IN_HEADERS_CHECK_H
+

clang-tools-extra/trunk/clang-tidy/google/GlobalNamesInHeadersCheck.cpp

+//===--- GlobalNamesInHeadersCheck.cpp - clang-tidy -----------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GlobalNamesInHeadersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void
+GlobalNamesInHeadersCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  Finder->addMatcher(
+      decl(anyOf(usingDecl(), usingDirectiveDecl()),
+           hasDeclContext(translationUnitDecl())).bind("using_decl"),
+      this);
+}
+
+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(
+          Result.SourceManager->getExpansionLoc(D->getLocStart()))) {
+    // unless that file is a header.
+    StringRef Filename = Result.SourceManager->getFilename(
+        Result.SourceManager->getSpellingLoc(D->getLocStart()));
+
+    if (!Filename.endswith(".h"))
+      return;
+  }
+
+  diag(D->getLocStart(),
+       "using declarations in the global namespace in headers are prohibited");
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

clang-tools-extra/trunk/clang-tidy/google/GoogleTidyModule.cpp

 #include "../ClangTidyModuleRegistry.h"
 #include "../readability/BracesAroundStatementsCheck.h"
 #include "../readability/FunctionSize.h"
 #include "../readability/NamespaceCommentCheck.h"
 #include "../readability/RedundantSmartptrGet.h"
 #include "AvoidCStyleCastsCheck.h"
 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
+#include "GlobalNamesInHeadersCheck.h"
 #include "IntegerTypesCheck.h"
 #include "MemsetZeroLengthCheck.h"
 #include "NamedParameterCheck.h"
 #include "OverloadedUnaryAndCheck.h"
 #include "StringReferenceMemberCheck.h"
 #include "TodoCommentCheck.h"
 #include "UnnamedNamespaceInHeaderCheck.h"
 #include "UsingNamespaceDirectiveCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<readability::AvoidCStyleCastsCheck>(
         "google-readability-casting");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "google-readability-function");
     CheckFactories.registerCheck<readability::TodoCommentCheck>(
         "google-readability-todo");
     CheckFactories.registerCheck<readability::BracesAroundStatementsCheck>(
         "google-readability-braces-around-statements");
+    CheckFactories.registerCheck<readability::GlobalNamesInHeadersCheck>(
+        "google-global-names-in-headers");
     CheckFactories.registerCheck<readability::FunctionSizeCheck>(
         "google-readability-function-size");
     CheckFactories.registerCheck<readability::NamespaceCommentCheck>(
         "google-readability-namespace-comments");
     CheckFactories.registerCheck<readability::RedundantSmartptrGet>(
         "google-readability-redundant-smartptr-get");
   }
 

clang-tools-extra/trunk/unittests/clang-tidy/GoogleModuleTest.cpp

 #include "ClangTidyTest.h"
 #include "google/ExplicitConstructorCheck.h"
+#include "google/GlobalNamesInHeadersCheck.h"
 #include "gtest/gtest.h"
 
 namespace clang {
 namespace tidy {
 namespace test {
 
 #define EXPECT_NO_CHANGES(Check, Code)                                         \
   EXPECT_EQ(Code, runCheckOnCode<Check>(Code))
TEST(ExplicitConstructorCheckTest, RemoveExplicitWithMacros) {
   EXPECT_EQ(
       "#define A(T) class T##Bar { explicit T##Bar(const T##Bar &b) {} };\n"
       "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) {
+    static const char *const Header = "namespace std {\n"
+                                      "class string {};\n"
+                                      "}  // namespace std\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");
+    }
+    test::runCheckOnCode<readability::GlobalNamesInHeadersCheck>(
+        Header + Code, &Errors, Filename, Args);
+    if (Errors.empty())
+      return false;
+    assert(Errors.size() == 1);
+    assert(
+        Errors[0].Message.Message ==
+        "using declarations in the global namespace in headers are prohibited");
+    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"));
+}
+
+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"));
+}
+
 } // namespace test
 } // namespace tidy
 } // namespace clang