[clang-tidy] Add a namespace checker.

[bkramer]

Summary

This warns on two constructs.

1. anonymous namespaces in header files.
2. using namespace directives everywhere.

[bkramer]

Remove test that should've been part of D4522.

[djasper]

clang-tidy/google/GoogleTidyModule.cpp

32–33

I think both the check name and the class Name should be more specific, e.g. UnnamedNamespaceInHeaderCheck. Or do you envision this to be a more generic check?

clang-tidy/google/NamespacesCheck.cpp

27

nit: I'd probably remove the empty lines inside the ast_matchers namespace but add an empty line before starting the tidy namespace.

clang-tidy/google/NamespacesCheck.h

19

IMO, "using namespace directives" should be detected in a separate check, there doesn't seem to be a strong relation to the empty namespaces in headers.

20

I actually don't think we should keep references to a different tool. The meaning of "build/namespaces" might change making this comment stale. This check is good on its own without a specific reference.

[alexfh]

clang-tidy/google/NamespacesCheck.h

19

nit: Add an empty line after the \brief line?

20

The main use of this is if we'll want to support categorized NOLINT comments: "// NOLINT(build/namespaces)", then it would be nice to have them in the same form.

[bkramer]

• Split into two checkers.
• Now depends on D4517.

[alexfh]

clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp

38 ↗

(On Diff #11495)

nit: I'd use early return instead.

41 ↗

(On Diff #11495)

The list of header file extensions may be a good candidate for being stored in ClangTidyOptions, so we can configure it when needed.

clang-tidy/google/UsingNamespaceDirectiveCheck.cpp

30 ↗

(On Diff #11495)

nit: Use early exit instead? (Especially, if we're going to implement fix-its)

31 ↗

(On Diff #11495)

Further down the line we may want to suggest the list of using declarations as a replacement. Could you add a TODO?

[bkramer]

clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp

41 ↗

(On Diff #11495)

Hmm, we don't have a way of accessing ClangTidyOptions from within a checker?

[alexfh]

clang-tidy/google/UnnamedNamespaceInHeaderCheck.cpp

41 ↗

(On Diff #11495)

Not yet. I meant, that we should do this in general, not necessarily now. Sorry for ambiguous language.

[bkramer]

Use early exits, add TODOs.

[alexfh]

Looks good. Thanks!

[bkramer]

Closed by commit rL213153 (authored by d0k).

clang-tidy/google/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyGoogleModule
   AvoidCStyleCastsCheck.cpp
   ExplicitConstructorCheck.cpp
   ExplicitMakePairCheck.cpp
   GoogleTidyModule.cpp
   NamedParameterCheck.cpp
+  NamespacesCheck.cpp
   OverloadedUnaryAndCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   )

clang-tidy/google/GoogleTidyModule.cpp

 //===--- GoogleTidyModule.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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidCStyleCastsCheck.h"
 #include "ExplicitConstructorCheck.h"
 #include "ExplicitMakePairCheck.h"
 #include "NamedParameterCheck.h"
+#include "NamespacesCheck.h"
 #include "OverloadedUnaryAndCheck.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 namespace tidy {
 
 class GoogleModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.addCheckFactory(
         "google-build-explicit-make-pair",
         new ClangTidyCheckFactory<build::ExplicitMakePairCheck>());
     CheckFactories.addCheckFactory(
+        "google-build-namespaces",
+        new ClangTidyCheckFactory<build::NamespacesCheck>());
+    CheckFactories.addCheckFactory(
         "google-explicit-constructor",
         new ClangTidyCheckFactory<ExplicitConstructorCheck>());
     CheckFactories.addCheckFactory(
         "google-runtime-operator",
         new ClangTidyCheckFactory<runtime::OverloadedUnaryAndCheck>());
     CheckFactories.addCheckFactory(
         "google-readability-casting",
         new ClangTidyCheckFactory<readability::AvoidCStyleCastsCheck>());

clang-tidy/google/NamespacesCheck.h

+//===--- NamespacesCheck.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_NAMESPACES_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMESPACES_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace build {
+
+/// \brief Finds anonymous namespaces in headers and using namespace directives.
+/// Corresponding cpplint.py check name: 'build/namespaces'.
+class NamespacesCheck : public ClangTidyCheck {
+public:
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace build
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_NAMESPACES_CHECK_H

clang-tidy/google/NamespacesCheck.cpp

+//===--- NamespacesCheck.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 "NamespacesCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/AST/ASTContext.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace ast_matchers {
+
+AST_MATCHER(NamespaceDecl, isAnonymousNamespace) {
+  return Node.isAnonymousNamespace();
+}
+
+const internal::VariadicDynCastAllOfMatcher<Decl, UsingDirectiveDecl>
+    usingDirectiveDecl;
+
+} // namespace ast_matchers
+namespace tidy {
+namespace build {
+
+void NamespacesCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
+  Finder->addMatcher(
+      namespaceDecl(isAnonymousNamespace()).bind("anonymousNamespace"), this);
+  Finder->addMatcher(usingDirectiveDecl().bind("usingNamespace"), this);
+}
+
+void NamespacesCheck::check(const MatchFinder::MatchResult &Result) {
+  SourceManager *SM = Result.SourceManager;
+  if (const auto *N =
+          Result.Nodes.getNodeAs<NamespaceDecl>("anonymousNamespace")) {
+    SourceLocation Loc = N->getLocStart();
+    if (Loc.isValid()) {
+      // Check for common suffixes only
+      StringRef FileName = SM->getPresumedLoc(Loc).getFilename();
+      if (FileName.endswith(".h") || FileName.endswith(".hh") ||
+          FileName.endswith(".hpp") || FileName.endswith(".hxx"))
+        diag(Loc, "do not use unnamed namespaces in header files.");
+    }
+  } else if (const auto *U =
+                 Result.Nodes.getNodeAs<UsingDirectiveDecl>("usingNamespace")) {
+    SourceLocation Loc = U->getLocStart();
+    if (!U->isImplicit() && Loc.isValid())
+      diag(Loc, "do not use namespace using-directives. Use using-declarations "
+                "instead.");
+  }
+}
+
+} // namespace build
+} // namespace tidy
+} // namespace clang

test/clang-tidy/Inputs/google-namespaces.h

+namespace {
+int x;
+}
+
+namespace spaaaace {
+class core;
+}

test/clang-tidy/google-namespaces.cpp

+// RUN: clang-tidy %s -checks='-*,google-build-namespaces' -header-filter='.*' -- | FileCheck %s -implicit-check-not="{{warning|error}}:"
+#include "Inputs/google-namespaces.h"
+// CHECK: warning: do not use unnamed namespaces in header files.
+
+using namespace spaaaace;
+// CHECK: :[[@LINE-1]]:1: warning: do not use namespace using-directives. Use using-declarations instead.
+
+using spaaaace::core; // no-warning