diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp @@ -143,8 +143,48 @@ return true; } -// Rewrites body of FD to fully-qualify all of the decls inside. -llvm::Expected qualifyAllDecls(const FunctionDecl *FD) { +// Gets the nested name specifiers necessary for spelling a name from +// SourceContext in DestContext. Drops any shared nested name specifiers between +// CurContext and SourceContext, doesn't take using declarations and directives +// into account. +NestedNameSpecifier *getQualification(ASTContext &Context, + const DeclContext *DestContext, + const DeclContext *SourceContext) { + // Goes over all parents of SourceContext until we find a comman ancestor + // between Dest and Source. Any qualifier including and above common ancestor + // is redundant, therefore we stop at lowest common ancestor. + std::vector SourceParents; + for (const DeclContext *Context = SourceContext; Context; + Context = Context->getLookupParent()) { + // Stop once we reach a common ancestor. + if (Context->Encloses(DestContext)) + break; + // Inline namespace names are not spelled while qualifying a name, so skip + // those. + if (Context->isInlineNamespace()) + continue; + + auto *NSD = llvm::dyn_cast(Context); + assert(NSD && "Non-namespace decl context found."); + // Again, ananoymous namespaces are not spelled while qualifying a name. + if (NSD->isAnonymousNamespace()) + continue; + + SourceParents.push_back(NSD); + } + + // Go over name-specifiers in reverse order to create necessary qualification, + // since we stored inner-most parent first. + NestedNameSpecifier *Result = nullptr; + for (const auto *Parent : llvm::reverse(SourceParents)) + Result = NestedNameSpecifier::Create(Context, Result, Parent); + return Result; +} + +// Rewrites body of FD by re-spelling all of the names to make sure they are +// still valid in TargetContext. +llvm::Expected qualifyAllDecls(const FunctionDecl *FD, + const DeclContext *TargetContext) { // There are three types of spellings that needs to be qualified in a function // body: // - Types: Foo -> ns::Foo @@ -155,14 +195,13 @@ // using ns3 = ns2 -> using ns3 = ns1::ns2 // // Go over all references inside a function body to generate replacements that - // will fully qualify those. So that body can be moved into an arbitrary file. + // will qualify those. So that body can be moved into an arbitrary file. // We perform the qualification by qualyfying the first type/decl in a // (un)qualified name. e.g: // namespace a { namespace b { class Bar{}; void foo(); } } // b::Bar x; -> a::b::Bar x; // foo(); -> a::b::foo(); - // FIXME: Instead of fully qualyfying we should try deducing visible scopes at - // target location and generate minimal edits. + // FIXME: Take using declarations and directives into account. const SourceManager &SM = FD->getASTContext().getSourceManager(); tooling::Replacements Replacements; @@ -198,7 +237,13 @@ if (!ND->getDeclContext()->isNamespace()) return; - std::string Qualifier = printNamespaceScope(*ND->getDeclContext()); + std::string Qualifier; + if (auto *NNS = getQualification(FD->getASTContext(), TargetContext, + ND->getLexicalDeclContext())) { + llvm::raw_string_ostream OS(Qualifier); + NNS->print(OS, FD->getASTContext().getPrintingPolicy(), true); + } + if (auto Err = Replacements.add( tooling::Replacement(SM, Ref.NameLoc, 0, Qualifier))) { HadErrors = true; @@ -527,7 +572,8 @@ if (!ParamReplacements) return ParamReplacements.takeError(); - auto QualifiedBody = qualifyAllDecls(Source); + auto QualifiedBody = + qualifyAllDecls(Source, Target->getLexicalDeclContext()); if (!QualifiedBody) return QualifiedBody.takeError(); diff --git a/clang-tools-extra/clangd/unittests/TweakTests.cpp b/clang-tools-extra/clangd/unittests/TweakTests.cpp --- a/clang-tools-extra/clangd/unittests/TweakTests.cpp +++ b/clang-tools-extra/clangd/unittests/TweakTests.cpp @@ -1402,6 +1402,95 @@ template <> inline void foo(){})cpp"))); } +TEST_F(DefineInlineTest, DropCommonNamespecifiers) { + EXPECT_EQ(apply(R"cpp( + namespace a { namespace b { void aux(); } } + namespace ns1 { + void foo(); + namespace qq { void test(); } + namespace ns2 { + void bar(); + namespace ns3 { void baz(); } + } + } + + using namespace a; + using namespace a::b; + using namespace ns1::qq; + void ns1::ns2::ns3::b^az() { + foo(); + bar(); + baz(); + ns1::ns2::ns3::baz(); + aux(); + test(); + })cpp"), + R"cpp( + namespace a { namespace b { void aux(); } } + namespace ns1 { + void foo(); + namespace qq { void test(); } + namespace ns2 { + void bar(); + namespace ns3 { void baz(){ + foo(); + bar(); + baz(); + ns1::ns2::ns3::baz(); + a::b::aux(); + qq::test(); + } } + } + } + + using namespace a; + using namespace a::b; + using namespace ns1::qq; + )cpp"); + + EXPECT_EQ(apply(R"cpp( + namespace ns1 { + namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; } + namespace ns2 { void baz(); } + } + + using namespace ns1::qq; + void ns1::ns2::b^az() { Foo f; B b; })cpp"), + R"cpp( + namespace ns1 { + namespace qq { struct Foo { struct Bar {}; }; using B = Foo::Bar; } + namespace ns2 { void baz(){ qq::Foo f; qq::B b; } } + } + + using namespace ns1::qq; + )cpp"); + + EXPECT_EQ(apply(R"cpp( + namespace ns1 { + namespace qq { + template struct Foo { template struct Bar {}; }; + template + using B = typename Foo::template Bar; + } + namespace ns2 { void baz(); } + } + + using namespace ns1::qq; + void ns1::ns2::b^az() { B b; })cpp"), + R"cpp( + namespace ns1 { + namespace qq { + template struct Foo { template struct Bar {}; }; + template + using B = typename Foo::template Bar; + } + namespace ns2 { void baz(){ qq::B b; } } + } + + using namespace ns1::qq; + )cpp"); +} + } // namespace } // namespace clangd } // namespace clang