[clangd] Sort results of incomingCalls request by container name

[kadircet]

Thanks!

clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp

80–85

let's also change these from EXPECT_ to ASSERT_. As we are dereferencing elements in the following calls. Same in other places too.

93

s/ElementsAre()/IsEmpty()/

[nridge]

Address review comments

clang-tools-extra/clangd/XRefs.cpp

incomingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   // get complete CallHierarchyIncomingCall objects.
   Index->lookup(ContainerLookup, [&](const Symbol &Caller) {
     auto It = CallsIn.find(Caller.ID);
     assert(It != CallsIn.end());
     if (auto CHI = symbolToCallHierarchyItem(Caller, Item.uri.file()))
       Results.push_back(
           CallHierarchyIncomingCall{std::move(*CHI), std::move(It->second)});
   });
+  // Sort results by name of container.
+  llvm::sort(Results, [](const CallHierarchyIncomingCall &A,
+                         const CallHierarchyIncomingCall &B) {
+    return A.from.name < B.from.name;
+  });
   return Results;
 }
 
 llvm::DenseSet<const Decl *> getNonLocalDeclRefs(ParsedAST &AST,
                                                  const FunctionDecl *FD) {
   if (!FD->hasBody())
     return {};
   llvm::DenseSet<const Decl *> DeclRefs;

clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp

 
 namespace clang {
 namespace clangd {
 namespace {
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAre;
 
 // Helpers for matching call hierarchy data structures.
 MATCHER_P(WithName, N, "") { return arg.name == N; }
 MATCHER_P(WithSelectionRange, R, "") { return arg.selectionRange == R; }
 
 template <class ItemMatcher>
Annotations Source(R"cpp(
     }
   )cpp");
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
   auto Index = TU.index();
 
   std::vector<CallHierarchyItem> Items =
       prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
               ElementsAre(AllOf(From(WithName("caller1")),
                                 FromRanges(Source.range("Callee")))));
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
-                                  AllOf(From(WithName("caller2")),
-                                        FromRanges(Source.range("Caller1A"),
-                                                   Source.range("Caller1B"))),
-                                  AllOf(From(WithName("caller3")),
-                                        FromRanges(Source.range("Caller1C")))));
+  ASSERT_THAT(IncomingLevel2,
+              ElementsAre(AllOf(From(WithName("caller2")),
 
   auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel3,
+  ASSERT_THAT(IncomingLevel3,
               ElementsAre(AllOf(From(WithName("caller3")),
                                 FromRanges(Source.range("Caller2")))));
 
   auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
-  EXPECT_THAT(IncomingLevel4, ElementsAre());
+  EXPECT_THAT(IncomingLevel4, IsEmpty());
 }
 
 TEST(CallHierarchy, MainFileOnlyRef) {
   // In addition to testing that we store refs to main-file only symbols,
   // this tests that anonymous namespaces do not interfere with the
   // symbol re-identification process in callHierarchyItemToSymbo().
   Annotations Source(R"cpp(
     void call^ee(int);
     namespace {
       void caller1() {
         $Callee[[callee]](42);
       }
     }
     void caller2() {
       $Caller1[[caller1]]();
     }
   )cpp");
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
   auto Index = TU.index();
 
   std::vector<CallHierarchyItem> Items =
       prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  ASSERT_THAT(Items, ElementsAre(WithName("callee")));
   auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-  EXPECT_THAT(IncomingLevel1,
+  ASSERT_THAT(IncomingLevel1,
               ElementsAre(AllOf(From(WithName("caller1")),
                                 FromRanges(Source.range("Callee")))));
 
   auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
   EXPECT_THAT(IncomingLevel2,
-              UnorderedElementsAre(AllOf(From(WithName("caller2")),
-                                         FromRanges(Source.range("Caller1")))));
+              ElementsAre(AllOf(From(WithName("caller2")),
 }
 
 TEST(CallHierarchy, IncomingQualified) {
   Annotations Source(R"cpp(
     namespace ns {
     struct Waldo {
       void find();
     };
     void Waldo::find() {}
     void caller1(Waldo &W) {
       W.$Caller1[[f^ind]]();
     }
     void caller2(Waldo &W) {
       W.$Caller2[[find]]();
     }
     }
   )cpp");
   TestTU TU = TestTU::withCode(Source.code());
   auto AST = TU.build();
   auto Index = TU.index();
 
   std::vector<CallHierarchyItem> Items =
       prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
-  EXPECT_THAT(Items, ElementsAre(WithName("Waldo::find")));
+  ASSERT_THAT(Items, ElementsAre(WithName("Waldo::find")));
   auto Incoming = incomingCalls(Items[0], Index.get());
   EXPECT_THAT(Incoming,
-              UnorderedElementsAre(AllOf(From(WithName("caller1")),
-                                         FromRanges(Source.range("Caller1"))),
-                                   AllOf(From(WithName("caller2")),
-                                         FromRanges(Source.range("Caller2")))));
+              ElementsAre(AllOf(From(WithName("caller1")),
 }
 
 TEST(CallHierarchy, IncomingMultiFile) {
   // The test uses a .hh suffix for header files to get clang
   // to parse them in C++ mode. .h files are parsed in C mode
   // by default, which causes problems because e.g. symbol
   // USRs are different in C mode (do not include function signatures).
 
TEST(CallHierarchy, IncomingMultiFile) {
   Workspace.addMainFile("caller2.cc", Caller2C.code());
   Workspace.addMainFile("caller3.cc", Caller3C.code());
 
   auto Index = Workspace.index();
 
   auto CheckCallHierarchy = [&](ParsedAST &AST, Position Pos, PathRef TUPath) {
     std::vector<CallHierarchyItem> Items =
         prepareCallHierarchy(AST, Pos, TUPath);
-    EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+    ASSERT_THAT(Items, ElementsAre(WithName("callee")));
     auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
-    EXPECT_THAT(IncomingLevel1,
+    ASSERT_THAT(IncomingLevel1,
                 ElementsAre(AllOf(From(WithName("caller1")),
                                   FromRanges(Caller1C.range()))));
 
     auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-    EXPECT_THAT(IncomingLevel2,
-                UnorderedElementsAre(
-                    AllOf(From(WithName("caller2")),
+    ASSERT_THAT(
+        IncomingLevel2,
+        ElementsAre(AllOf(From(WithName("caller2")),
                           FromRanges(Caller2C.range("A"), Caller2C.range("B"))),
                     AllOf(From(WithName("caller3")),
                           FromRanges(Caller3C.range("Caller1")))));
 
     auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
-    EXPECT_THAT(IncomingLevel3,
+    ASSERT_THAT(IncomingLevel3,
                 ElementsAre(AllOf(From(WithName("caller3")),
                                   FromRanges(Caller3C.range("Caller2")))));
 
     auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
-    EXPECT_THAT(IncomingLevel4, ElementsAre());
+    EXPECT_THAT(IncomingLevel4, IsEmpty());
   };
 
   // Check that invoking from a call site works.
   auto AST = Workspace.openFile("caller1.cc");
   ASSERT_TRUE(bool(AST));
   CheckCallHierarchy(*AST, Caller1C.point(), testPath("caller1.cc"));
 
   // Check that invoking from the declaration site works.