[change-namespace] don't fix using shadow decls in classes.

[ioeric]

Summary

Using shadow declarations in classes always refers to base class, which does not
need to be fixed/qualified since it can be inferred from inheritance.

[bkramer]

change-namespace/ChangeNamespace.cpp

331

Maybe add to this comment that you're matching non-alias using declarations.

[ioeric]

• address review comment.

change-namespace/ChangeNamespace.cpp

void ChangeNamespaceTool::registerMatchers(ast_matchers::MatchFinder *Finder) {
   // be moved back to the old namespace.
   auto DeclMatcher = namedDecl(
       hasAncestor(namespaceDecl()),
       unless(anyOf(
           isImplicit(), hasAncestor(namespaceDecl(isAnonymous())),
           hasAncestor(cxxRecordDecl()),
           allOf(IsInMovedNs, unless(cxxRecordDecl(unless(isDefinition())))))));
 
+  // Using shadow declarations in classes always refers to base class, which
+  // does not need to be qualified since it can be inferred from inheritance.
+  auto UsingShadowDeclInClass =
+      usingDecl(hasAnyUsingShadowDecl(decl()), hasParent(cxxRecordDecl()));
+
   // Match TypeLocs on the declaration. Carefully match only the outermost
   // TypeLoc and template specialization arguments (which are not outermost)
   // that are directly linked to types matching `DeclMatcher`. Nested name
   // specifier locs are handled separately below.
   Finder->addMatcher(
       typeLoc(IsInMovedNs,
               loc(qualType(hasDeclaration(DeclMatcher.bind("from_decl")))),
               unless(anyOf(hasParent(typeLoc(loc(qualType(
                                allOf(hasDeclaration(DeclMatcher),
                                      unless(templateSpecializationType())))))),
-                           hasParent(nestedNameSpecifierLoc()))),
+                           hasParent(nestedNameSpecifierLoc()),
+                           hasAncestor(isImplicit()),
+                           hasAncestor(UsingShadowDeclInClass))),
               hasAncestor(decl().bind("dc")))
           .bind("type"),
       this);
 
   // Types in `UsingShadowDecl` is not matched by `typeLoc` above, so we need to
   // special case it.
-  Finder->addMatcher(usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl()))
+  // Since using declarations inside classes must have the base class in the
+  // nested name specifier, we leave it to the nested name specifier matcher.
+  Finder->addMatcher(usingDecl(IsInMovedNs, hasAnyUsingShadowDecl(decl()),
+                               unless(UsingShadowDeclInClass))
                          .bind("using_with_shadow"),
                      this);
 
   // Handle types in nested name specifier. Specifiers that are in a TypeLoc
   // matched above are not matched, e.g. "A::" in "A::A" is not matched since
   // "A::A" would have already been fixed.
-  Finder->addMatcher(nestedNameSpecifierLoc(
-                         hasAncestor(decl(IsInMovedNs).bind("dc")),
-                         loc(nestedNameSpecifier(specifiesType(
-                             hasDeclaration(DeclMatcher.bind("from_decl"))))),
-                         unless(hasAncestor(typeLoc(loc(qualType(hasDeclaration(
-                             decl(equalsBoundNode("from_decl")))))))))
-                         .bind("nested_specifier_loc"),
-                     this);
+  Finder->addMatcher(
+      nestedNameSpecifierLoc(
+          loc(nestedNameSpecifier(
+              specifiesType(hasDeclaration(DeclMatcher.bind("from_decl"))))),
+          unless(anyOf(hasAncestor(isImplicit()),
+                       hasAncestor(UsingShadowDeclInClass),
+                       hasAncestor(typeLoc(loc(qualType(hasDeclaration(
+                           decl(equalsBoundNode("from_decl"))))))))))
 
   // Matches base class initializers in constructors. TypeLocs of base class
   // initializers do not need to be fixed. For example,
   //    class X : public a::b::Y {
   //      public:
   //        X() : Y::Y() {} // Y::Y do not need namespace specifier.
   //    };
   Finder->addMatcher(

unittests/change-namespace/ChangeNamespaceTests.cpp

std::string Expected = "class GLOB {};\n"
                          "using YO = na::nd::SAME;\n"
                          "typedef na::nd::SAME IDENTICAL;\n"
                          "void f(na::nd::SAME Same) {}\n"
                          "} // namespace y\n"
                          "} // namespace x\n";
   EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
 }
 
+TEST_F(ChangeNamespaceTest, DontFixUsingShadowDeclInClasses) {
+  std::string Code = "namespace na {\n"
+                     "class A {};\n"
+                     "class Base { public: Base() {} void m() {} };\n"
+                     "namespace nb {\n"
+                     "class D : public Base {\n"
+                     "public:\n"
+                     "  using AA = A; using B = Base;\n"
+                     "  using Base::m; using Base::Base;\n"
+                     "};"
+                     "} // namespace nb\n"
+                     "} // namespace na\n";
+
+  std::string Expected = "namespace na {\n"
+                         "class A {};\n"
+                         "class Base { public: Base() {} void m() {} };\n"
+                         "\n"
+                         "} // namespace na\n"
+                         "namespace x {\n"
+                         "namespace y {\n"
+                         "class D : public na::Base {\n"
+                         "public:\n"
+                         "  using AA = na::A; using B = na::Base;\n"
+                         "  using Base::m; using Base::Base;\n"
+                         "};"
+                         "} // namespace y\n"
+                         "} // namespace x\n";
+  EXPECT_EQ(format(Expected), runChangeNamespaceOnCode(Code));
+}
+
 TEST_F(ChangeNamespaceTest, TypeInNestedNameSpecifier) {
   std::string Code =
       "namespace na {\n"
       "class C_A {\n"
       "public:\n"
       "  class Nested {\n"
       "    public:\n"
       "      static int NestedX;\n"