[clang-tidy] Add new check llvm-prefer-static-over-anonymous-namespace

clang-tools-extra/clang-tidy/llvm/CMakeLists.txt

@@ -9,6 +9,7 @@ add_clang_library(clangTidyLLVMModule STATIC
   LLVMTidyModule.cpp
   PreferIsaOrDynCastInConditionalsCheck.cpp
   PreferRegisterOverUnsignedCheck.cpp
+  PreferStaticOverAnonymousNamespaceCheck.cpp
   TwineLocalCheck.cpp
 
   LINK_LIBS

clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp

@@ -16,6 +16,7 @@
 #include "IncludeOrderCheck.h"
 #include "PreferIsaOrDynCastInConditionalsCheck.h"
 #include "PreferRegisterOverUnsignedCheck.h"
+#include "PreferStaticOverAnonymousNamespaceCheck.h"
 #include "TwineLocalCheck.h"
 
 namespace clang::tidy {
@@ -34,6 +35,8 @@ class LLVMModule : public ClangTidyModule {
         "llvm-prefer-isa-or-dyn-cast-in-conditionals");
     CheckFactories.registerCheck<PreferRegisterOverUnsignedCheck>(
         "llvm-prefer-register-over-unsigned");
+    CheckFactories.registerCheck<PreferStaticOverAnonymousNamespaceCheck>(
+        "llvm-prefer-static-over-anonymous-namespace");
     CheckFactories.registerCheck<readability::QualifiedAutoCheck>(
         "llvm-qualified-auto");
     CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");

clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.cpp

@@ -0,0 +1,100 @@
+//===--- PreferStaticOverAnonymousNamespaceCheck.cpp - clang-tidy ---------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "PreferStaticOverAnonymousNamespaceCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::llvm_check {
+
+namespace {
+
+AST_MATCHER(NamedDecl, isInMacro) {
+  return Node.getBeginLoc().isMacroID() || Node.getEndLoc().isMacroID();
+}
+
+AST_MATCHER(VarDecl, isLocalVariable) { return Node.isLocalVarDecl(); }
+
+} // namespace
+
+PreferStaticOverAnonymousNamespaceCheck::
+    PreferStaticOverAnonymousNamespaceCheck(StringRef Name,
+                                            ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      AllowVariableDeclarations(Options.get("AllowVariableDeclarations", true)),
+      AllowMemberFunctionsInClass(
+          Options.get("AllowMemberFunctionsInClass", true)) {}
+
+void PreferStaticOverAnonymousNamespaceCheck::storeOptions(
+    ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "AllowVariableDeclarations", AllowVariableDeclarations);
+  Options.store(Opts, "AllowMemberFunctionsInClass",
+                AllowMemberFunctionsInClass);
+}
+
+void PreferStaticOverAnonymousNamespaceCheck::registerMatchers(
+    MatchFinder *Finder) {
+  const auto IsDefinitionInAnonymousNamespace =
+      allOf(unless(isExpansionInSystemHeader()), isInAnonymousNamespace(),
+            unless(isInMacro()), isDefinition());
+
+  if (AllowMemberFunctionsInClass) {
+    Finder->addMatcher(
+        functionDecl(IsDefinitionInAnonymousNamespace,
+                     unless(anyOf(hasParent(cxxRecordDecl()),
+                                  hasParent(functionTemplateDecl(
+                                      hasParent(cxxRecordDecl()))))))
+            .bind("function"),
+        this);
+  } else {
+    Finder->addMatcher(
+        functionDecl(IsDefinitionInAnonymousNamespace).bind("function"), this);
+  }
+
+  if (!AllowVariableDeclarations)
+    Finder->addMatcher(varDecl(IsDefinitionInAnonymousNamespace,
+                               unless(isLocalVariable()), unless(parmVarDecl()))
+                           .bind("var"),
+                       this);
+}
+
+void PreferStaticOverAnonymousNamespaceCheck::check(
+    const MatchFinder::MatchResult &Result) {
+
+  if (const auto *Func = Result.Nodes.getNodeAs<FunctionDecl>("function")) {
+    if (Func->isCXXClassMember())
+      diag(Func->getLocation(),
+           "place definition of method %0 outside of an anonymous namespace")
+          << Func;
+    else if (Func->isStatic())
+      diag(Func->getLocation(),
+           "place static function %0 outside of an anonymous namespace")
+          << Func;
+    else
+      diag(Func->getLocation(),
+           "function %0 is declared in an anonymous namespace; "
+           "prefer using 'static' for restricting visibility")
+          << Func;
+    return;
+  }
+
+  if (const auto *Var = Result.Nodes.getNodeAs<VarDecl>("var")) {
+    if (Var->getStorageClass() == SC_Static)
+      diag(Var->getLocation(),
+           "place static variable %0 outside of an anonymous namespace")
+          << Var;
+    else
+      diag(Var->getLocation(),
+           "variable %0 is declared in an anonymous namespace; "
+           "prefer using 'static' for restricting visibility")
+          << Var;
+  }
+}
+
+} // namespace clang::tidy::llvm_check

clang-tools-extra/clang-tidy/llvm/PreferStaticOverAnonymousNamespaceCheck.h

@@ -0,0 +1,42 @@
+//===--- PreferStaticOverAnonymousNamespaceCheck.h - clang-tidy -*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERSTATICOVERANONYMOUSNAMESPACECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERSTATICOVERANONYMOUSNAMESPACECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::llvm_check {
+
+/// Finds function and variable declarations inside anonymous namespace and
+/// suggests replacing them with ``static`` declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/llvm/prefer-static-over-anonymous-namespace.html
+class PreferStaticOverAnonymousNamespaceCheck : public ClangTidyCheck {
+public:
+  PreferStaticOverAnonymousNamespaceCheck(StringRef Name,
+                                          ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.CPlusPlus;
+  }
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+
+private:
+  const bool AllowVariableDeclarations;
+  const bool AllowMemberFunctionsInClass;
+};
+
+} // namespace clang::tidy::llvm_check
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PREFERSTATICOVERANONYMOUSNAMESPACECHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

@@ -142,6 +142,12 @@ New checks
   Finds unscoped (non-class) ``enum`` declarations and suggests using
   ``enum class`` instead.
 
+- New :doc:`llvm-prefer-static-over-anonymous-namespace
+  <clang-tidy/checks/llvm/prefer-static-over-anonymous-namespace>` check.
+
+  Finds function and variable declarations inside anonymous namespace and
+  suggests replacing them with ``static`` declarations.
+
 - New :doc:`modernize-use-scoped-lock
   <clang-tidy/checks/modernize/use-scoped-lock>` check.
 

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -250,6 +250,7 @@ Clang-Tidy Checks
    :doc:`llvm-namespace-comment <llvm/namespace-comment>`,
    :doc:`llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm/prefer-isa-or-dyn-cast-in-conditionals>`, "Yes"
    :doc:`llvm-prefer-register-over-unsigned <llvm/prefer-register-over-unsigned>`, "Yes"
+   :doc:`llvm-prefer-static-over-anonymous-namespace <llvm/prefer-static-over-anonymous-namespace>`,
    :doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
    :doc:`llvmlibc-callee-namespace <llvmlibc/callee-namespace>`,
    :doc:`llvmlibc-implementation-in-namespace <llvmlibc/implementation-in-namespace>`,

clang-tools-extra/docs/clang-tidy/checks/llvm/prefer-static-over-anonymous-namespace.rst

@@ -0,0 +1,73 @@
+.. title:: clang-tidy - llvm-prefer-static-over-anonymous-namespace
+
+llvm-prefer-static-over-anonymous-namespace
+===========================================
+
+Finds function and variable declarations inside anonymous namespace and
+suggests replacing them with ``static`` declarations.
+
+The `LLVM Coding Standards <https://llvm.org/docs/CodingStandards.html#restrict-visibility>`_
+recommend keeping anonymous namespaces as small as possible and only use them
+for class declarations. For functions and variables the ``static`` specifier
+should be preferred for restricting visibility.
+
+For example non-compliant code:
+
+.. code-block:: c++
+
+  namespace {
+
+  class StringSort {
+  public:
+    StringSort(...)
+    bool operator<(const char *RHS) const;
+  };
+
+  // warning: place method definition outside of an anonymous namespace
+  bool StringSort::operator<(const char *RHS) const {}
+
+  // warning: prefer using 'static' for restricting visibility
+  void runHelper() {}
+
+  // warning: prefer using 'static' for restricting visibility
+  int myVariable = 42;
+
+  }
+
+Should become:
+
+.. code-block:: c++
+
+  // Small anonymous namespace for class declaration
+  namespace {
+
+  class StringSort {
+  public:
+    StringSort(...)
+    bool operator<(const char *RHS) const;
+  };
+
+  }
+
+  // placed method definition outside of the anonymous namespace
+  bool StringSort::operator<(const char *RHS) const {}
+
+  // used 'static' instead of an anonymous namespace
+  static void runHelper() {}
+
+  // used 'static' instead of an anonymous namespace
+  static int myVariable = 42;
+
+
+Options
+-------
+
+.. option:: AllowVariableDeclarations
+
+  When `true`, allow variable declarations to be in anonymous namespace.
+  Default value is `true`.
+
+.. option:: AllowMemberFunctionsInClass
+
+  When `true`, only methods defined in anonymous namespace outside of the
+  corresponding class will be warned. Default value is `true`.