[clang-tidy] Add a check to detect static definitions in anonymous namespace.

[hokein]

Summary

Fixes PR26595

[alexfh]

Sorry for the delay. A few minor issues.

clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp

47

I'd expand the message a bit: "; static is redundant here".

docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst

9

nit: in _the_ current _translation_ unit.

test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp

2

Please add a couple of tests for different cases with macros.

10

This check is not helpful, it will match static int c = 1; as well, since it's not anchored to the line start. Should be:
CHECK-FIXES: {{^}}int c = 1;. Same in other checks.

[hokein]

Address comments.

[hokein]

Remove dump code.

[hokein]

doc update.

[alexfh]

clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp

53

DiagnosticBuilder handles hella different stuff. I wonder whether this code works without ->getName()?

docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst

10

Still reads a bit strange. A bit more wordsmithing:

In this case, `static` is redundant, because anonymous namespace limits the visibility of definitions to a single translation unit.

test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp

33

Please add a CHECK-FIXES for this case as well. Also move the macro definition closer to its usage and add a CHECK-FIXES to ensure it doesn't get changed.

Same below.

[hokein]

Address comments.

clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp

53

Using Def works!

[alexfh]

clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp

49

nit: auto?

docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst

8

Double backquotes should be used for inline code snippets (yeah, I know, this is confusing: different flavors of markdown and RST are completely inconsistent in this regard).

test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp

33

You missed the "Also move the macro definition closer to its usage and add a CHECK-FIXES to ensure it doesn't get changed." part.

Same below.

[hokein]

Update

[hokein]

test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp

35

Oops. I misunderstood your comment. Done now.

[alexfh]

docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst

10

This is still not done.

test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp

35

Sorry for being unclear again: please add

// CHECK-FIXES: #define DEFINE_STATIC_VAR(x) static int x = 2

to verify that the check doesn't unintentionally remove static from the macro definition. Same for the macro above.

[hokein]

• fix inline code snippet in rst.
• Add check-message for macro in test code.

[alexfh]

Awesome. Thank you!
LG

clang-tidy/readability/CMakeLists.txt

add_clang_library(clangTidyReadabilityModule
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
   ReadabilityTidyModule.cpp
   RedundantControlFlowCheck.cpp
   RedundantStringCStrCheck.cpp
   RedundantSmartptrGetCheck.cpp
   RedundantStringInitCheck.cpp
   SimplifyBooleanExprCheck.cpp
+  StaticDefinitionInAnonymousNamespaceCheck.cpp
   UniqueptrDeleteReleaseCheck.cpp
 
   LINK_LIBS
   clangAST
   clangASTMatchers
   clangBasic
   clangLex
   clangTidy
   clangTooling
   )

clang-tidy/readability/ReadabilityTidyModule.cpp

 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
 #include "NamedParameterCheck.h"
 #include "RedundantControlFlowCheck.h"
 #include "RedundantSmartptrGetCheck.h"
 #include "RedundantStringCStrCheck.h"
 #include "RedundantStringInitCheck.h"
 #include "SimplifyBooleanExprCheck.h"
+#include "StaticDefinitionInAnonymousNamespaceCheck.h"
 #include "UniqueptrDeleteReleaseCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace readability {
 
 class ReadabilityModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<BracesAroundStatementsCheck>(
         "readability-braces-around-statements");
     CheckFactories.registerCheck<ContainerSizeEmptyCheck>(
         "readability-container-size-empty");
     CheckFactories.registerCheck<ElseAfterReturnCheck>(
         "readability-else-after-return");
     CheckFactories.registerCheck<FunctionSizeCheck>(
         "readability-function-size");
     CheckFactories.registerCheck<IdentifierNamingCheck>(
         "readability-identifier-naming");
     CheckFactories.registerCheck<ImplicitBoolCastCheck>(
         "readability-implicit-bool-cast");
     CheckFactories.registerCheck<InconsistentDeclarationParameterNameCheck>(
         "readability-inconsistent-declaration-parameter-name");
+    CheckFactories.registerCheck<StaticDefinitionInAnonymousNamespaceCheck>(
+        "readability-static-definition-in-anonymous-namespace");
     CheckFactories.registerCheck<readability::NamedParameterCheck>(
         "readability-named-parameter");
     CheckFactories.registerCheck<RedundantControlFlowCheck>(
         "readability-redundant-control-flow");
     CheckFactories.registerCheck<RedundantSmartptrGetCheck>(
         "readability-redundant-smartptr-get");
     CheckFactories.registerCheck<RedundantStringCStrCheck>(
         "readability-redundant-string-cstr");

clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.h

+//===--- StaticDefinitionInAnonymousNamespaceCheck.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_READABILITY_STATIC_DEFINITION_IN_ANONYMOUS_NAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STATIC_DEFINITION_IN_ANONYMOUS_NAMESPACE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Finds static function and variable definitions in anonymous namespace.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.html
+class StaticDefinitionInAnonymousNamespaceCheck : public ClangTidyCheck {
+public:
+  StaticDefinitionInAnonymousNamespaceCheck(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_READABILITY_STATIC_DEFINITION_IN_ANONYMOUS_NAMESPACE_H

clang-tidy/readability/StaticDefinitionInAnonymousNamespaceCheck.cpp

+//===--- StaticDefinitionInAnonymousNamespaceCheck.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 "StaticDefinitionInAnonymousNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+AST_POLYMORPHIC_MATCHER(isStatic, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
+                                                                  VarDecl)) {
+  return Node.getStorageClass() == SC_Static;
+}
+} // namespace
+
+void StaticDefinitionInAnonymousNamespaceCheck::registerMatchers(
+    MatchFinder *Finder) {
+  Finder->addMatcher(namedDecl(anyOf(functionDecl(isDefinition(), isStatic()),
+                                     varDecl(isDefinition(), isStatic())),
+                               hasParent(namespaceDecl(isAnonymous())))
+                         .bind("static-def"),
+                     this);
+}
+
+void StaticDefinitionInAnonymousNamespaceCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *Def = Result.Nodes.getNodeAs<NamedDecl>("static-def");
+  // Skips all static definitions defined in Macro.
+  if (Def->getLocation().isMacroID())
+    return;
+
+  // Skips all static definitions in function scope.
+  const DeclContext *DC = Def->getDeclContext();
+  if (DC->getDeclKind() != Decl::Namespace)
+    return;
+
+  DiagnosticBuilder Diag =
+      diag(Def->getLocation(), "%0 is a static definition in "
+                               "anonymous namespace; static is redundant here")
+      << Def;
+  Token Tok;
+  SourceLocation Loc = Def->getSourceRange().getBegin();
+  while (Loc < Def->getSourceRange().getEnd() &&
+         !Lexer::getRawToken(Loc, Tok, *Result.SourceManager,
+                             Result.Context->getLangOpts(), true)) {
+    SourceRange TokenRange(Tok.getLocation(), Tok.getEndLoc());
+    StringRef SourceText = Lexer::getSourceText(
+        CharSourceRange::getTokenRange(TokenRange),
+        *Result.SourceManager, Result.Context->getLangOpts());
+    if (SourceText == "static") {
+      Diag << FixItHint::CreateRemoval(TokenRange);
+      break;
+    }
+    Loc = Tok.getEndLoc();
+  }
+}
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang

docs/clang-tidy/checks/list.rst

.. toctree::
    readability-implicit-bool-cast
    readability-inconsistent-declaration-parameter-name
    readability-named-parameter
    readability-redundant-control-flow
    readability-redundant-smartptr-get
    readability-redundant-string-cstr
    readability-redundant-string-init
    readability-simplify-boolean-expr
+   readability-static-definition-in-anonymous-namespace
    readability-uniqueptr-delete-release

docs/clang-tidy/checks/readability-static-definition-in-anonymous-namespace.rst

+.. title:: clang-tidy - readability-static-definition-in-anonymous-namespace
+
+readability-static-definition-in-anonymous-namespace
+====================================================
+
+Finds static function and variable definitions in anonymous namespace.
+
+In this case, `static` is redundant, because anonymous namespace limits the
+visibility of definitions to a single translation unit.
+
+.. code:: c++
+  namespace {
+    static int a = 1; // Warning.
+    static const b = 1; // Warning.
+  }
+
+The check will apply a fix by removing the redundant `static` qualifier.

test/clang-tidy/readability-static-definition-in-anonymous-namespace.cpp

+// RUN: %check_clang_tidy %s readability-static-definition-in-anonymous-namespace %t
+
+#define DEFINE_STATIC static
+#define DEFINE_STATIC_VAR(x) static int x = 2
+
+namespace {
+
+int a = 1;
+const int b = 1;
+static int c = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'c' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]
+// CHECK-FIXES: {{^}}int c = 1;
+static const int d = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'd' is a static definition in anonymous namespace
+// CHECK-FIXES: {{^}}const int d = 1;
+const static int e = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: 'e' is a static definition in anonymous namespace
+// CHECK-FIXES: {{^}}const int e = 1;
+
+void f() {
+  int a = 1;
+  static int b = 1;
+}
+
+static int g() {
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: 'g' is a static definition in anonymous namespace
+// CHECK-FIXES: {{^}}int g() {
+  return 1;
+}
+
+DEFINE_STATIC int h = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: 'h' is a static definition in anonymous namespace
+// CHECK-FIXES: {{^}}DEFINE_STATIC int h = 1;
+
+DEFINE_STATIC_VAR(i);
+// CHECK-FIXES: {{^}}DEFINE_STATIC_VAR(i);
+
+} // namespace
+
+namespace N {
+
+int a = 1;
+const int b = 1;
+static int c = 1;
+static const int d = 1;
+const static int e = 1;
+
+} // namespace N