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 @@ -136,8 +136,10 @@ return true; } -// Rewrites body of FD to fully-qualify all of the decls inside. -llvm::Expected qualifyAllDecls(const FunctionDecl *FD) { +// Rewrites body of FD by re-spelling all of the names to make sure they are +// still valid in context of Target. +llvm::Expected qualifyAllDecls(const FunctionDecl *FD, + const FunctionDecl *Target) { // There are three types of spellings that needs to be qualified in a function // body: // - Types: Foo -> ns::Foo @@ -148,16 +150,16 @@ // 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. + auto *TargetContext = Target->getLexicalDeclContext(); const SourceManager &SM = FD->getASTContext().getSourceManager(); + tooling::Replacements Replacements; bool HadErrors = false; findExplicitReferences(FD->getBody(), [&](ReferenceLoc Ref) { @@ -191,10 +193,18 @@ // namespace a { class Bar { public: static int x; } } // void foo() { Bar::x; } // ~~~~~ -> we need to qualify Bar not x. - if (!ND->getDeclContext()->isNamespace()) + if (!ND->getLexicalDeclContext()->isNamespace()) return; - std::string Qualifier = printNamespaceScope(*ND->getDeclContext()); + std::string Qualifier; + // FIXME: Also take using directives and namespace aliases inside function + // body into account. + if (auto *NNS = getQualification(FD->getASTContext(), TargetContext, + Target->getBeginLoc(), ND)) { + 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; @@ -464,7 +474,7 @@ if (!ParamReplacements) return ParamReplacements.takeError(); - auto QualifiedBody = qualifyAllDecls(Source); + auto QualifiedBody = qualifyAllDecls(Source, Target); 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 @@ -1714,6 +1714,138 @@ 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"); +} + +TEST_F(DefineInlineTest, QualifyWithUsingDirectives) { + // FIXME: The last reference to cux() in body of foo should not be qualified, + // since there is a using directive inside the function body. + EXPECT_EQ(apply(R"cpp( + namespace a { + void bar(); + namespace b { struct Foo{}; void aux(); } + namespace c { void cux(); } + } + using namespace a; + using X = b::Foo; + void foo(); + + using namespace b; + using namespace c; + void ^foo() { + cux(); + bar(); + X x; + aux(); + using namespace c; + cux(); + })cpp"), R"cpp( + namespace a { + void bar(); + namespace b { struct Foo{}; void aux(); } + namespace c { void cux(); } + } + using namespace a; + using X = b::Foo; + void foo(){ + c::cux(); + bar(); + X x; + b::aux(); + using namespace c; + c::cux(); + } + + using namespace b; + using namespace c; + )cpp"); +} } // namespace } // namespace clangd } // namespace clang