[clang-tidy] Add new check llvm-prefer-static-over-anonymous-namespace (#142839)

Finds function and variable declarations inside anonymous namespace and
suggests replacing them with ``static`` declarations.

The check will enforce that
[restrict-visibility](https://llvm.org/docs/CodingStandards.html#restrict-visibility)
rule in LLVM Coding Standards is followed correctly (by adding `static`
to functions instead of putting them in anonimous namespace).

The check has additional levels of "strictness" represented by Options.
By default, the check works in the most relaxed way by giving warning
only for methods and functions defined in anonymous namespaces. Also, It
finds `static` functions that are placed inside anonymous namespace -
there is no point in keeping them inside.

Author: vbvictor

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`.