[clang-tidy] Fix a couple additional cases in misc-use-anonymous-namespace only

- Do not analyze header files, since we don't want to promote
  using anonymous namespaces there.

- Do not warn about const/constexpr variables, those are implicitly
  static in C++ and they don't need to be moved to an anonymous
  namespace. Warning about redundant static in general could be
  implemented as a standalone check, moving away some of the
  functionality from this check.

This check has been introduced in the current release, thus
no mention of this change is needed in the Release Notes.

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

diff --git a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
index fdf7828..b26632d 100644 (file)
--- a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
@@ -21,6 +21,15 @@ AST_POLYMORPHIC_MATCHER(isStatic, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
   return Node.getStorageClass() == SC_Static;
 }
 
+AST_POLYMORPHIC_MATCHER_P(isInHeaderFile,
+                          AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                          VarDecl),
+                          utils::FileExtensionsSet, HeaderFileExtensions) {
+  return utils::isExpansionLocInHeaderFile(
+      Node.getBeginLoc(), Finder->getASTContext().getSourceManager(),
+      HeaderFileExtensions);
+}
+
 AST_MATCHER(FunctionDecl, isMemberFunction) {
   return llvm::isa<CXXMethodDecl>(&Node);
 }
@@ -31,15 +40,36 @@ AST_MATCHER(Decl, isInAnonymousNamespace) {
 }
 } // namespace
 
+UseAnonymousNamespaceCheck::UseAnonymousNamespaceCheck(
+    StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+          "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {
+  if (!utils::parseFileExtensions(RawStringHeaderFileExtensions,
+                                  HeaderFileExtensions,
+                                  utils::defaultFileExtensionDelimiters())) {
+    this->configurationDiag("Invalid header file extension: '%0'")
+        << RawStringHeaderFileExtensions;
+  }
+}
+
+void UseAnonymousNamespaceCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "HeaderFileExtensions", RawStringHeaderFileExtensions);
+}
+
 void UseAnonymousNamespaceCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       functionDecl(isStatic(),
-                   unless(anyOf(isInAnonymousNamespace(), isMemberFunction())))
+                   unless(anyOf(isInHeaderFile(HeaderFileExtensions),
+                                isInAnonymousNamespace(), isMemberFunction())))
           .bind("x"),
       this);
   Finder->addMatcher(
-      varDecl(isStatic(), unless(anyOf(isInAnonymousNamespace(),
-                                       isStaticLocal(), isStaticDataMember())))
+      varDecl(isStatic(),
+              unless(anyOf(isInHeaderFile(HeaderFileExtensions),
+                           isInAnonymousNamespace(), isStaticLocal(),
+                           isStaticDataMember(), hasType(isConstQualified()))))
           .bind("x"),
       this);
 }

diff --git a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
index 19d76bf..af4ac60 100644 (file)
--- a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
+++ b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.h
@@ -10,6 +10,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_USEANONYMOUSNAMESPACECHECK_H
 
 #include "../ClangTidyCheck.h"
+#include "../utils/FileExtensionsUtils.h"
 
 namespace clang {
 namespace tidy {
@@ -18,17 +19,29 @@ namespace misc {
 /// Warns when using 'static' functions or variables at global scope, and
 /// suggests moving them to an anonymous namespace.
 ///
+/// The check supports these options:
+///   - `HeaderFileExtensions`: a semicolon-separated list of filename
+///     extensions of header files (The filename extension should not contain
+///     "." prefix). ";h;hh;hpp;hxx" by default.
+///
+///     For extension-less header files, using an empty string or leaving an
+///     empty string between ";" if there are other filename extensions.
+///
 /// For the user-facing documentation see:
 /// http://clang.llvm.org/extra/clang-tidy/checks/modernize/use-anonymous-namespace.html
 class UseAnonymousNamespaceCheck : public ClangTidyCheck {
 public:
-  UseAnonymousNamespaceCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UseAnonymousNamespaceCheck(StringRef Name, ClangTidyContext *Context);
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus;
   }
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const StringRef RawStringHeaderFileExtensions;
+  utils::FileExtensionsSet HeaderFileExtensions;
 };
 
 } // namespace misc

diff --git a/clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst
index c116bfb..6b25670 100644 (file)
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/use-anonymous-namespace.rst
@@ -11,6 +11,14 @@ Standard. ``static`` was proposed for deprecation, but later un-deprecated to
 keep C compatibility [1]. ``static`` is an overloaded term with different meanings in
 different contexts, so it can create confusion.
 
+The following uses of ``static`` will *not* be diagnosed:
+
+* Functions or variables in header files, since anonymous namespaces in headers
+  is considered an antipattern. Allowed header file extensions can be configured
+  via the `HeaderFileExtensions` option (see below).
+* ``const`` or ``constexpr`` variables, since they already have implicit internal
+  linkage in C++.
+
 Examples:
 
 .. code-block:: c++
@@ -25,4 +33,14 @@ Examples:
     int x;
   } // namespace
 
+Options
+-------
+
+.. option:: HeaderFileExtensions
+
+   A semicolon-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an
+   empty string between ";" if there are other filename extensions.
+
 [1] `Undeprecating static <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1012>`_

diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-anonymous-namespace.h b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-anonymous-namespace.h
new file mode 100644 (file)
index 0000000..59fdef1
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/Inputs/use-anonymous-namespace.h
@@ -0,0 +1,3 @@
+// Should not warn here, do not require anonymous namespaces in headers
+static int gv{123};
+static void gf(){}

diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp
index 966310e..14f8436 100644 (file)
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/use-anonymous-namespace.cpp
@@ -1,4 +1,5 @@
-// RUN: %check_clang_tidy %s misc-use-anonymous-namespace %t
+// RUN: %check_clang_tidy %s misc-use-anonymous-namespace %t -- -header-filter=.* -- -I%S/Inputs
+#include "use-anonymous-namespace.h"
 
 static void f1();
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function 'f1' declared 'static', move to anonymous namespace instead [misc-use-anonymous-namespace]
@@ -41,3 +42,7 @@ void foo()
 {
   static int x;
 }
+
+// OK
+static const int v8{123};
+static constexpr int v9{123};