maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[clangd] Be more explicit on testing the optional DefLoc in LocatedSymbol. 

And also fix a bug where we may return a meaningless location.

Differential Revision: https://reviews.llvm.org/D84919
This commit is contained in:
Haojian Wu 2020-07-31 14:32:18 +02:00
parent 0d25d3b7e3
commit 638f0cf565
2 changed files with 51 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
clang-tools-extra/clangd/XRefs.cpp
View File

@ -405,15 +405,17 @@ locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
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 @@ locateSymbolTextually(const SpelledWord &Word, ParsedAST &AST,
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;

74
clang-tools-extra/clangd/unittests/XRefsTests.cpp
View File

@ -41,6 +41,7 @@ using ::testing::ElementsAre;
using ::testing::Eq;
using ::testing::IsEmpty;
using ::testing::Matcher;
using ::testing::UnorderedElementsAre;
using ::testing::UnorderedElementsAreArray;
MATCHER_P2(FileRange, File, Range, "") {
@ -264,19 +265,23 @@ MATCHER_P3(Sym, Name, Decl, DefOrNone, "") {
<< 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<LocatedSymbol> 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 @@ TEST(LocateSymbol, TextualSmoke) {
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 @@ TEST(LocateSymbol, Ambiguous) {
// 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 @@ TEST(LocateSymbol, TextualDependent) {
// 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 @@ int [[bar_not_preamble]];
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(GoToInclude, All) {
// 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 @@ TEST(GoToInclude, All) {
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 @@ TEST(GoToInclude, All) {
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 @@ TEST(LocateSymbol, WithPreamble) {
// 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 @@ TEST(LocateSymbol, WithPreamble) {
// 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 @@ TEST(LocateSymbol, WithPreamble) {
// 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 @@ TEST(LocateSymbol, NearbyTokenSmoke) {
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) {