[include-cleaner] Make handling of enum constants similar to members

[kadircet]

Summary

We were treating enum constants more like regular decls, which results
in ignoring type aliases/exports.
This patch brings the handling to be closer to member-like decls, with
one caveat. When we encounter reference to an enum constant we still
report an explicit reference to the particular enum constant, as
otherwise we might not see any references to the enum itself.

[sammccall]

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

135

try to write a test case that relies on this :-)

clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

320

test using enum

[kadircet]

• Add test for using enums
• Drop implicit references from qualified names to their containers, as we should already have explicit references from the spelling of the qualifier.

[kadircet]

• Change to c++20

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

bool TraverseCXXOperatorCallExpr(CXXOperatorCallExpr *S) {
     }
     for (auto *Arg : S->arguments())
       if (!TraverseStmt(Arg))
         return false;
     return true;
   }
 
   bool VisitDeclRefExpr(DeclRefExpr *DRE) {
-    // Static class members are handled here, as they don't produce MemberExprs.
-    if (DRE->getFoundDecl()->isCXXClassMember()) {
-      if (auto *Qual = DRE->getQualifier())
-        report(DRE->getLocation(), Qual->getAsRecordDecl(), RefType::Implicit);
-    } else {
-      report(DRE->getLocation(), DRE->getFoundDecl());
-    }
+    auto *FD = DRE->getFoundDecl();
+    // For refs to non-meber-like decls, use the found decl.
+    // For member-like decls, we should have a reference from the qualifier to
+    // the container decl instead, which is preferred as it'll handle
+    // aliases/exports properly.
+    if (!FD->isCXXClassMember() && !llvm::isa<EnumConstantDecl>(FD)) {
+      report(DRE->getLocation(), FD);
+      return true;
+    }
+    // If the ref is without a qualifier, and is a member, ignore it. As it is
+    // available in current context due to some other construct (e.g. base
+    // specifiers, using decls) that has to spell the name explicitly.
+    // If it's an enum constant, it must be due to prior decl. Report references
+    // to it instead.
+    if (llvm::isa<EnumConstantDecl>(FD) && !DRE->hasQualifier())
+      report(DRE->getLocation(), FD);
     return true;
   }
 
   bool VisitMemberExpr(MemberExpr *E) {
     // Reporting a usage of the member decl would cause issues (e.g. force
     // including the base class for inherited members). Instead, we report a
     // usage of the base type of the MemberExpr, so that e.g. code
     // `returnFoo().bar` can keep #include "foo.h" (rather than inserting

clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

std::vector<Decl::Kind> testWalk(llvm::StringRef TargetCode,
                                  llvm::StringRef ReferencingCode) {
   llvm::Annotations Target(TargetCode);
   llvm::Annotations Referencing(ReferencingCode);
 
   TestInputs Inputs(Referencing.code());
   Inputs.ExtraFiles["target.h"] = Target.code().str();
   Inputs.ExtraArgs.push_back("-include");
   Inputs.ExtraArgs.push_back("target.h");
-  Inputs.ExtraArgs.push_back("-std=c++17");
+  Inputs.ExtraArgs.push_back("-std=c++20");
   TestAST AST(Inputs);
   const auto &SM = AST.sourceManager();
 
   // We're only going to record references from the nominated point,
   // to the target file.
   FileID ReferencingFile = SM.getMainFileID();
   SourceLocation ReferencingLoc =
       SM.getComposedLoc(ReferencingFile, Referencing.point());
if (!DiagBuf.empty())
     ADD_FAILURE() << DiagBuf << "\nfrom code:\n" << ReferencingCode;
   return TargetDecls;
 }
 
 TEST(WalkAST, DeclRef) {
   testWalk("int $explicit^x;", "int y = ^x;");
   testWalk("int $explicit^foo();", "int y = ^foo();");
   testWalk("namespace ns { int $explicit^x; }", "int y = ns::^x;");
-  testWalk("struct $implicit^S { static int x; };", "int y = S::^x;");
+  testWalk("struct S { static int x; };", "int y = S::^x;");
   // Canonical declaration only.
   testWalk("extern int $explicit^x; int x;", "int y = ^x;");
   // Return type of `foo` isn't used.
   testWalk("struct S{}; S $explicit^foo();", "auto bar() { return ^foo(); }");
 }
 
 TEST(WalkAST, TagType) {
   testWalk("struct $explicit^S {};", "^S *y;");
TEST(WalkAST, Alias) {
   // happened through an exported type or not. So err towards attributing use to
   // the using-decl, specializations on the exported type should be rare and
   // they're not permitted on type-aliases.
   testWalk(R"cpp(
     template<typename> struct Foo {};
     namespace ns { using ::$explicit^Foo; }
     template<> struct ns::Foo<int> {};)cpp",
            "ns::^Foo<int> x;");
+  testWalk(R"cpp(
+    namespace ns { enum class foo { bar }; }
+    using ns::foo;)cpp",
+           "auto x = foo::^bar;");
+  testWalk(R"cpp(
+    namespace ns { enum foo { bar }; }
+    using ns::foo::$explicit^bar;)cpp",
+           "auto x = ^bar;");
 }
 
 TEST(WalkAST, Using) {
   // We should report unused overloads as ambiguous.
   testWalk(R"cpp(
     namespace ns {
       void $explicit^x(); void $ambiguous^x(int); void $ambiguous^x(char);
     })cpp",
            "using ns::^x; void foo() { x(); }");
namespace ns {
            "using ns::^Y;");
   testWalk(R"cpp(
     namespace ns {
       template<class T>
       class Y {};
     }
     using ns::$explicit^Y;)cpp",
            "^Y<int> x;");
+  testWalk("namespace ns { enum E {A}; } using enum ns::$explicit^E;",
+           "auto x = ^A;");
 }
 
 TEST(WalkAST, Namespaces) {
   testWalk("namespace ns { void x(); }", "using namespace ^ns;");
 }
 
 TEST(WalkAST, TemplateNames) {
   testWalk("template<typename> struct $explicit^S {};", "^S<int> s;");
testWalk(R"cpp(
       struct Base { struct $implicit^a {}; };
       struct Derived : public Base {};
       struct SoDerived : public Derived {};
       )cpp",
            "void fun() { SoDerived::Derived::^a x; }");
 }
 
 TEST(WalkAST, MemberExprs) {
-  testWalk("struct $implicit^S { static int f; };", "void foo() { S::^f; }");
-  testWalk("struct B { static int f; }; struct $implicit^S : B {};",
+  testWalk("struct S { static int f; };", "void foo() { S::^f; }");
+  testWalk("struct B { static int f; }; struct S : B {};",
            "void foo() { S::^f; }");
-  testWalk("struct B { static void f(); }; struct $implicit^S : B {};",
+  testWalk("struct B { static void f(); }; struct S : B {};",
            "void foo() { S::^f; }");
   testWalk("struct B { static void f(); }; ",
            "struct S : B { void foo() { ^f(); } };");
   testWalk("struct $implicit^S { void foo(); };", "void foo() { S{}.^foo(); }");
   testWalk(
       "struct S { void foo(); }; struct $implicit^X : S { using S::foo; };",
       "void foo() { X{}.^foo(); }");
   testWalk("struct Base { int a; }; struct $implicit^Derived : public Base {};",