diff --git a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp --- a/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp @@ -136,7 +136,6 @@ // Contains function signature, body and template parameters if applicable. // No need to qualify parameters, as they are looked up in the context // containing the function/method. -// FIXME: Qualify function name depending on the target context. llvm::Expected getFunctionSourceCode(const FunctionDecl *FD, llvm::StringRef TargetNamespace) { auto &SM = FD->getASTContext().getSourceManager(); @@ -149,16 +148,17 @@ llvm::Error Errors = llvm::Error::success(); tooling::Replacements QualifierInsertions; - // Finds the first unqualified name in function return type and qualifies it - // to be valid in TargetContext. + // Finds the first unqualified name in function return type and name, then + // qualifies those to be valid in TargetContext. findExplicitReferences(FD, [&](ReferenceLoc Ref) { // It is enough to qualify the first qualifier, so skip references with a // qualifier. Also we can't do much if there are no targets or name is // inside a macro body. if (Ref.Qualifier || Ref.Targets.empty() || Ref.NameLoc.isMacroID()) return; - // Qualify return type - if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin()) + // Only qualify return type and function name. + if (Ref.NameLoc != FD->getReturnTypeSourceRange().getBegin() && + Ref.NameLoc != FD->getLocation()) return; for (const NamedDecl *ND : Ref.Targets) { @@ -293,9 +293,7 @@ auto FuncDef = getFunctionSourceCode(Source, InsertionPoint->EnclosingNamespace); if (!FuncDef) - return llvm::createStringError( - llvm::inconvertibleErrorCode(), - "Couldn't get full source for function definition."); + return FuncDef.takeError(); SourceManagerForFile SMFF(*CCFile, Contents); const tooling::Replacement InsertFunctionDef( 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 @@ -2004,7 +2004,7 @@ Bar foo() ; }; })cpp", - "a::Foo::Bar foo() { return {}; }\n"}, + "a::Foo::Bar a::Foo::foo() { return {}; }\n"}, {R"cpp( class Foo; Foo fo^o() { return; })cpp", @@ -2022,6 +2022,58 @@ } } +TEST_F(DefineOutlineTest, QualifyFunctionName) { + FileName = "Test.hpp"; + struct { + llvm::StringRef TestHeader; + llvm::StringRef TestSource; + llvm::StringRef ExpectedHeader; + llvm::StringRef ExpectedSource; + } Cases[] = { + { + R"cpp( + namespace a { + namespace b { + class Foo { + void fo^o() {} + }; + } + })cpp", + "", + R"cpp( + namespace a { + namespace b { + class Foo { + void foo() ; + }; + } + })cpp", + "void a::b::Foo::foo() {}\n", + }, + { + "namespace a { namespace b { void f^oo() {} } }", + "namespace a{}", + "namespace a { namespace b { void foo() ; } }", + "namespace a{void b::foo() {} }", + }, + { + "namespace a { namespace b { void f^oo() {} } }", + "using namespace a;", + "namespace a { namespace b { void foo() ; } }", + // FIXME: Take using namespace directives in the source file into + // account. This can be spelled as b::foo instead. + "using namespace a;void a::b::foo() {} ", + }, + }; + llvm::StringMap EditedFiles; + for (auto &Case : Cases) { + ExtraFiles["Test.cpp"] = Case.TestSource; + EXPECT_EQ(apply(Case.TestHeader, &EditedFiles), Case.ExpectedHeader); + EXPECT_THAT(EditedFiles, testing::ElementsAre(FileWithContents( + testPath("Test.cpp"), Case.ExpectedSource))) + << Case.TestHeader; + } +} } // namespace } // namespace clangd } // namespace clang