[clang-tidy] support unscoped enumerations in readability-static-accessed-through-instance

[HerrCai0907]

Summary

fixed 60810
unscoped enumerations in class can also be checked by readability-static-accessed-through-instance
add matcher for enumConstantDecl to match format

struct {
    enum { E1 };
};

The filter of member expression and the fix hint should be same as other condition.

[PiotrZSL]

Add test with scoped enums, to validate that it works correctly.
In theory this change should suport them also.

[PiotrZSL]

And update commit message to be more descriptive...

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

264

Please keep alphabetical order (by check name) in this section.

[HerrCai0907]

In D147315#4236403, @PiotrZSL wrote:

Add test with scoped enums, to validate that it works correctly.
In theory this change should suport them also.

scoped enum not support to visit by member. EnumName:: is needed for scoped enum. And ins.EnumName is invalid.

[PiotrZSL]

Ok, you right.
Fix all open issues, and update commit message to be more detailed.
Except that, LGTM.

[HerrCai0907]

update description

[PiotrZSL]

Well, description update didn't update, you can update it here in UI, just edit revision, if you use "arc", there is a switch to apply description update.

[carlosgalvezp]

Would it make sense to add a scoped enum to the test, to ensure it doesn't warn/fix?

[HerrCai0907]

In D147315#4238596, @carlosgalvezp wrote:

Would it make sense to add a scoped enum to the test, to ensure it doesn't warn/fix?

It is not possible to visit scoped enum by instance. I think adding an invalid expression to test it is meaningless.

clang-tools-extra/clang-tidy/readability/StaticAccessedThroughInstanceCheck.cpp

void StaticAccessedThroughInstanceCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "NameSpecifierNestingThreshold",
                 NameSpecifierNestingThreshold);
 }
 
 void StaticAccessedThroughInstanceCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
       memberExpr(hasDeclaration(anyOf(cxxMethodDecl(isStaticStorageClass()),
-                                      varDecl(hasStaticStorageDuration()))))
+                                      varDecl(hasStaticStorageDuration()),
+                                      enumConstantDecl())))
           .bind("memberExpression"),
       this);
 }
 
 void StaticAccessedThroughInstanceCheck::check(
     const MatchFinder::MatchResult &Result) {
   const auto *MemberExpression =
       Result.Nodes.getNodeAs<MemberExpr>("memberExpression");
 
   if (MemberExpression->getBeginLoc().isMacroID())
     return;
 
   const Expr *BaseExpr = MemberExpression->getBase();
 
   // Do not warn for overloaded -> operators.
   if (isa<CXXOperatorCallExpr>(BaseExpr))
     return;
 
   QualType BaseType =
       BaseExpr->getType()->isPointerType()
           ? BaseExpr->getType()->getPointeeType().getUnqualifiedType()
           : BaseExpr->getType().getUnqualifiedType();
 
   const ASTContext *AstContext = Result.Context;
-  PrintingPolicy PrintingPolicyWithSupressedTag(AstContext->getLangOpts());
-  PrintingPolicyWithSupressedTag.SuppressTagKeyword = true;
-  PrintingPolicyWithSupressedTag.SuppressUnwrittenScope = true;
+  PrintingPolicy PrintingPolicyWithSuppressedTag(AstContext->getLangOpts());
+  PrintingPolicyWithSuppressedTag.SuppressTagKeyword = true;
+  PrintingPolicyWithSuppressedTag.SuppressUnwrittenScope = true;
 
-  PrintingPolicyWithSupressedTag.PrintCanonicalTypes =
+  PrintingPolicyWithSuppressedTag.PrintCanonicalTypes =
       !BaseExpr->getType()->isTypedefNameType();
 
   std::string BaseTypeName =
-      BaseType.getAsString(PrintingPolicyWithSupressedTag);
+      BaseType.getAsString(PrintingPolicyWithSuppressedTag);
 
   // Do not warn for CUDA built-in variables.
   if (StringRef(BaseTypeName).startswith("__cuda_builtin_"))
     return;
 
   SourceLocation MemberExprStartLoc = MemberExpression->getBeginLoc();
   auto Diag =
       diag(MemberExprStartLoc, "static member accessed through instance");

clang-tools-extra/docs/ReleaseNotes.rst

- Fixed an issue in :doc:`readability-identifier-naming
   <clang-tidy/checks/readability/identifier-naming>` when specifying an empty
   string for ``Prefix`` or ``Suffix`` options could result in the style not
   being used.
 
 - Fixed an issue in :doc:`google-readability-avoid-underscore-in-googletest-name
   <clang-tidy/checks/google/readability-avoid-underscore-in-googletest-name>` when using
   ``DISABLED_`` in the test suite name.
 
+- Improved :doc:`readability-static-accessed-through-instance
+  <clang-tidy/checks/readability/static-accessed-through-instance>` check to 
+  support unscoped enumerations through instances.
+
 Removed checks
 ^^^^^^^^^^^^^^
 
 Improvements to include-fixer
 -----------------------------
 
 The improvements are...
 

clang-tools-extra/docs/clang-tidy/checks/readability/static-accessed-through-instance.rst

 
 The following code:
 
 .. code-block:: c++
 
   struct C {
     static void foo();
     static int x;
+    enum { E1 };
+    enum E { E2 };
   };
 
   C *c1 = new C();
   c1->foo();
   c1->x;
+  c1->E1;
+  c1->E2;
 
 is changed to:
 
 .. code-block:: c++
 
   C *c1 = new C();
   C::foo();
   C::x;
+  C::E1;
+  C::E2;
 

clang-tools-extra/test/clang-tidy/checkers/readability/static-accessed-through-instance.cpp

 // RUN: %check_clang_tidy %s readability-static-accessed-through-instance %t -- -- -isystem %S/Inputs/static-accessed-through-instance
 #include <__clang_cuda_builtin_vars.h>
 
+enum OutEnum {
+  E0,
+};
+
 struct C {
   static void foo();
   static int x;
   int nsx;
+  enum {
+    Anonymous,
+  };
+  enum E {
+    E1,
+  };
+  using enum OutEnum;
   void mf() {
     (void)&x;    // OK, x is accessed inside the struct.
     (void)&C::x; // OK, x is accessed using a qualified-id.
     foo();       // OK, foo() is accessed inside the struct.
   }
   void ns() const;
 };
 

 void static_through_instance() {
   C *c1 = new C();
   c1->foo(); // 1
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
   // CHECK-FIXES: {{^}}  C::foo(); // 1{{$}}
   c1->x; // 2
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
   // CHECK-FIXES: {{^}}  C::x; // 2{{$}}
+  c1->Anonymous; // 3
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  C::Anonymous; // 3{{$}}
+  c1->E1; // 4
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  C::E1; // 4{{$}}
+  c1->E0; // 5
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
+  // CHECK-FIXES: {{^}}  C::E0; // 5{{$}}
+
   c1->nsx; // OK, nsx is a non-static member.
 
   const C *c2 = new C();
   c2->foo(); // 2
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: static member
   // CHECK-FIXES: {{^}}  C::foo(); // 2{{$}}
 
   C::foo(); // OK, foo() is accessed using a qualified-id.