diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -405,15 +405,17 @@ log("locateSymbolNamedTextuallyAt: {0}", MaybeDeclLoc.takeError()); return; } - Location DeclLoc = *MaybeDeclLoc; - Location DefLoc; + LocatedSymbol Located; + Located.PreferredDeclaration = *MaybeDeclLoc; + Located.Name = (Sym.Name + Sym.TemplateSpecializationArgs).str(); if (Sym.Definition) { auto MaybeDefLoc = indexToLSPLocation(Sym.Definition, MainFilePath); if (!MaybeDefLoc) { log("locateSymbolNamedTextuallyAt: {0}", MaybeDefLoc.takeError()); return; } - DefLoc = *MaybeDefLoc; + Located.PreferredDeclaration = *MaybeDefLoc; + Located.Definition = *MaybeDefLoc; } if (ScoredResults.size() >= 3) { @@ -424,11 +426,6 @@ return; } - LocatedSymbol Located; - Located.Name = (Sym.Name + Sym.TemplateSpecializationArgs).str(); - Located.PreferredDeclaration = bool(Sym.Definition) ? DefLoc : DeclLoc; - Located.Definition = DefLoc; - SymbolQualitySignals Quality; Quality.merge(Sym); SymbolRelevanceSignals Relevance; diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -41,6 +41,7 @@ using ::testing::Eq; using ::testing::IsEmpty; using ::testing::Matcher; +using ::testing::UnorderedElementsAre; using ::testing::UnorderedElementsAreArray; MATCHER_P2(FileRange, File, Range, "") { @@ -264,19 +265,23 @@ << llvm::to_string(arg.PreferredDeclaration); return false; } + if (!Def && !arg.Definition) + return true; if (Def && !arg.Definition) { *result_listener << "Has no definition"; return false; } - if (Def && arg.Definition->range != *Def) { + if (!Def && arg.Definition) { + *result_listener << "Definition is " << llvm::to_string(arg.Definition); + return false; + } + if (arg.Definition->range != *Def) { *result_listener << "Definition is " << llvm::to_string(arg.Definition); return false; } return true; } -::testing::Matcher Sym(std::string Name, Range Decl) { - return Sym(Name, Decl, llvm::None); -} + MATCHER_P(Sym, Name, "") { return arg.Name == Name; } MATCHER_P(RangeIs, R, "") { return arg.range == R; } @@ -771,7 +776,7 @@ auto AST = TU.build(); auto Index = TU.index(); EXPECT_THAT(locateSymbolAt(AST, T.point(), Index.get()), - ElementsAre(Sym("MyClass", T.range()))); + ElementsAre(Sym("MyClass", T.range(), T.range()))); } TEST(LocateSymbol, Textual) { @@ -891,18 +896,20 @@ // FIXME: Target the constructor as well. EXPECT_THAT(locateSymbolAt(AST, T.point("9")), ElementsAre(Sym("Foo"))); EXPECT_THAT(locateSymbolAt(AST, T.point("10")), - ElementsAre(Sym("Foo", T.range("ConstructorLoc")))); + ElementsAre(Sym("Foo", T.range("ConstructorLoc"), llvm::None))); EXPECT_THAT(locateSymbolAt(AST, T.point("11")), - ElementsAre(Sym("Foo", T.range("ConstructorLoc")))); + ElementsAre(Sym("Foo", T.range("ConstructorLoc"), llvm::None))); // These assertions are unordered because the order comes from // CXXRecordDecl::lookupDependentName() which doesn't appear to provide // an order guarantee. EXPECT_THAT(locateSymbolAt(AST, T.point("12")), - UnorderedElementsAre(Sym("bar", T.range("NonstaticOverload1")), - Sym("bar", T.range("NonstaticOverload2")))); - EXPECT_THAT(locateSymbolAt(AST, T.point("13")), - UnorderedElementsAre(Sym("baz", T.range("StaticOverload1")), - Sym("baz", T.range("StaticOverload2")))); + UnorderedElementsAre( + Sym("bar", T.range("NonstaticOverload1"), llvm::None), + Sym("bar", T.range("NonstaticOverload2"), llvm::None))); + EXPECT_THAT( + locateSymbolAt(AST, T.point("13")), + UnorderedElementsAre(Sym("baz", T.range("StaticOverload1"), llvm::None), + Sym("baz", T.range("StaticOverload2"), llvm::None))); } TEST(LocateSymbol, TextualDependent) { @@ -932,9 +939,10 @@ // interaction between locateASTReferent() and // locateSymbolNamedTextuallyAt(). auto Results = locateSymbolAt(AST, Source.point(), Index.get()); - EXPECT_THAT(Results, UnorderedElementsAre( - Sym("uniqueMethodName", Header.range("FooLoc")), - Sym("uniqueMethodName", Header.range("BarLoc")))); + EXPECT_THAT(Results, + UnorderedElementsAre( + Sym("uniqueMethodName", Header.range("FooLoc"), llvm::None), + Sym("uniqueMethodName", Header.range("BarLoc"), llvm::None))); } TEST(LocateSymbol, TemplateTypedefs) { @@ -992,20 +1000,23 @@ auto Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p1")); EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo", SourceAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo", SourceAnnotations.range(), + SourceAnnotations.range()))); // Go to a definition in header_in_preamble.h. Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p2")); EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error"; EXPECT_THAT( *Locations, - ElementsAre(Sym("bar_preamble", HeaderInPreambleAnnotations.range()))); + ElementsAre(Sym("bar_preamble", HeaderInPreambleAnnotations.range(), + HeaderInPreambleAnnotations.range()))); // Go to a definition in header_not_in_preamble.h. Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("p3")); EXPECT_TRUE(bool(Locations)) << "findDefinitions returned an error"; EXPECT_THAT(*Locations, ElementsAre(Sym("bar_not_preamble", + HeaderNotInPreambleAnnotations.range(), HeaderNotInPreambleAnnotations.range()))); } @@ -1039,21 +1050,25 @@ // Test include in preamble. auto Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point()); ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(), + HeaderAnnotations.range()))); // Test include in preamble, last char. Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("2")); ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(), + HeaderAnnotations.range()))); Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("3")); ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(), + HeaderAnnotations.range()))); // Test include outside of preamble. Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("6")); ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(), + HeaderAnnotations.range()))); // Test a few positions that do not result in Locations. Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("4")); @@ -1062,11 +1077,13 @@ Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("5")); ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(), + HeaderAnnotations.range()))); Locations = runLocateSymbolAt(Server, FooCpp, SourceAnnotations.point("7")); ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(), + HeaderAnnotations.range()))); // Objective C #import directive. Annotations ObjC(R"objc( @@ -1078,7 +1095,8 @@ Server.addDocument(FooM, ObjC.code()); Locations = runLocateSymbolAt(Server, FooM, ObjC.point()); ASSERT_TRUE(bool(Locations)) << "locateSymbolAt returned an error"; - EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range()))); + EXPECT_THAT(*Locations, ElementsAre(Sym("foo.h", HeaderAnnotations.range(), + HeaderAnnotations.range()))); } TEST(LocateSymbol, WithPreamble) { @@ -1103,7 +1121,7 @@ // LocateSymbol goes to a #include file: the result comes from the preamble. EXPECT_THAT( cantFail(runLocateSymbolAt(Server, FooCpp, FooWithHeader.point())), - ElementsAre(Sym("foo.h", FooHeader.range()))); + ElementsAre(Sym("foo.h", FooHeader.range(), FooHeader.range()))); // Only preamble is built, and no AST is built in this request. Server.addDocument(FooCpp, FooWithoutHeader.code(), "null", @@ -1112,7 +1130,7 @@ // stale one. EXPECT_THAT( cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())), - ElementsAre(Sym("foo", FooWithoutHeader.range()))); + ElementsAre(Sym("foo", FooWithoutHeader.range(), llvm::None))); // Reset test environment. runAddDocument(Server, FooCpp, FooWithHeader.code()); @@ -1122,7 +1140,7 @@ // Use the AST being built in above request. EXPECT_THAT( cantFail(runLocateSymbolAt(Server, FooCpp, FooWithoutHeader.point())), - ElementsAre(Sym("foo", FooWithoutHeader.range()))); + ElementsAre(Sym("foo", FooWithoutHeader.range(), llvm::None))); } TEST(LocateSymbol, NearbyTokenSmoke) { @@ -1133,7 +1151,7 @@ auto AST = TestTU::withCode(T.code()).build(); // We don't pass an index, so can't hit index-based fallback. EXPECT_THAT(locateSymbolAt(AST, T.point()), - ElementsAre(Sym("err", T.range()))); + ElementsAre(Sym("err", T.range(), T.range()))); } TEST(LocateSymbol, NearbyIdentifier) {