[clang-tidy] Exclude class/struct scope variables from cppcoreguidelines-avoid-non-const-global-variables

[PiotrZSL]

Summary

Ignore static variables declared within the scope of class/struct.
Those variables should be covered by I.3 rule.

Fixes: #47384

[carlosgalvezp]

LGTM!

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidNonConstGlobalVariablesCheck.cpp

 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang::tidy::cppcoreguidelines {
 
-namespace {
-AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); }
-} // namespace
-
 void AvoidNonConstGlobalVariablesCheck::registerMatchers(MatchFinder *Finder) {
+  auto GlobalContext =
+      varDecl(hasGlobalStorage(),
+              hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())));
+
   auto GlobalVariable = varDecl(
-      hasGlobalStorage(),
+      GlobalContext,
       unless(anyOf(
-          isLocalVarDecl(), isConstexpr(), hasType(isConstQualified()),
+          isConstexpr(), hasType(isConstQualified()),
           hasType(referenceType())))); // References can't be changed, only the
                                        // data they reference can be changed.
 
   auto GlobalReferenceToNonConst =
-      varDecl(hasGlobalStorage(), hasType(referenceType()),
+      varDecl(GlobalContext, hasType(referenceType()),
               unless(hasType(references(qualType(isConstQualified())))));
 
-  auto GlobalPointerToNonConst =
-      varDecl(hasGlobalStorage(),
-              hasType(pointerType(pointee(unless(isConstQualified())))));
+  auto GlobalPointerToNonConst = varDecl(
+      GlobalContext, hasType(pointerType(pointee(unless(isConstQualified())))));
 
   Finder->addMatcher(GlobalVariable.bind("non-const_variable"), this);
   Finder->addMatcher(GlobalReferenceToNonConst.bind("indirection_to_non-const"),
                      this);
   Finder->addMatcher(GlobalPointerToNonConst.bind("indirection_to_non-const"),
                      this);
 }
 

clang-tools-extra/docs/ReleaseNotes.rst

 
 Changes in existing checks
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 - Fixed bug in :doc:`bugprone-reserved-identifier
   <clang-tidy/checks/bugprone/reserved-identifier>`, so that it does not warn
   on macros starting with underscore and lowercase letter.
 
+- Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
+  <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
+  to ignore ``static`` variables declared within the scope of
+  ``class``/``struct``.
+
 - Improved :doc:`llvm-namespace-comment
   <clang-tidy/checks/llvm/namespace-comment>` check to provide fixes for
   ``inline`` namespaces in the same format as :program:`clang-format`.
 
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-non-const-global-variables.cpp

 // CHECKING AGAINST FALSE POSITIVES INSIDE FUNCTION SCOPE /////////////////////
 int main() {
   for (int i = 0; i < 3; ++i) {
     static int staticNonConstLoopVariable = 42;
     int nonConstLoopVariable = 42;
     nonConstInt = nonConstLoopVariable + i + staticNonConstLoopVariable;
   }
 }
+
+// CHECKING AGAINST FALSE POSITIVES INSIDE STRUCT SCOPE /////////////////////
+struct StructWithStatic {
+  static DummyStruct nonConstDummyStructInstance;
+  static int value;
+  static int* valuePtr;
+  static int& valueRef;
+};
+
+DummyStruct StructWithStatic::nonConstDummyStructInstance;
+int StructWithStatic::value = 0;
+int* StructWithStatic::valuePtr = &StructWithStatic::value;
+int& StructWithStatic::valueRef = StructWithStatic::value;
+