[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

[Nathan Ridge via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG5b6f47595bab: [clangd] Sort results of incomingCalls request 
by container name (authored by nridge).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92009/new/

https://reviews.llvm.org/D92009

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -31,6 +31,7 @@
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAre;
 
@@ -69,27 +70,27 @@
 
   std::vector 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")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
 
   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) {
@@ -113,16 +114,16 @@
 
   std::vector 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")),
+FromRanges(Source.range("Caller1");
 }
 
 TEST(CallHierarchy, IncomingQualified) {
@@ -146,13 +147,13 @@
 
   std::vector 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")),
+FromRanges(Source.range("Caller1"))),
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller2");
 }
 
 TEST(CallHierarchy, IncomingMultiFile) {
@@ -212,27 +213,27 @@
   auto CheckCallHierarchy = [&](ParsedAST &AST, Position Pos, PathRef TUPath) {
 std::vector Items =
 prepareCallHierarchy(AST, Pos, TUPath);
-EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+ASSERT_THAT(Items, ElementsAre(WithName("callee")));
 auto IncomingLevel1 = incomingCalls(Ite

[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 307262.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92009/new/

https://reviews.llvm.org/D92009

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -31,6 +31,7 @@
 using ::testing::AllOf;
 using ::testing::ElementsAre;
 using ::testing::Field;
+using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::UnorderedElementsAre;
 
@@ -69,27 +70,27 @@
 
   std::vector 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")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
 
   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) {
@@ -113,16 +114,16 @@
 
   std::vector 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")),
+FromRanges(Source.range("Caller1");
 }
 
 TEST(CallHierarchy, IncomingQualified) {
@@ -146,13 +147,13 @@
 
   std::vector 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")),
+FromRanges(Source.range("Caller1"))),
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller2");
 }
 
 TEST(CallHierarchy, IncomingMultiFile) {
@@ -212,27 +213,27 @@
   auto CheckCallHierarchy = [&](ParsedAST &AST, Position Pos, PathRef TUPath) {
 std::vector 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")),
   

[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:79
   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");
+  EXPECT_THAT(IncomingLevel2,
+  ElementsAre(AllOf(From(WithName("caller2")),

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



Comment at: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp:92
   auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
   EXPECT_THAT(IncomingLevel4, ElementsAre());
 }

s/ElementsAre()/IsEmpty()/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92009/new/

https://reviews.llvm.org/D92009

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D92009: [clangd] Sort results of incomingCalls request by container name

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman, mgrang.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D92009

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp


Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -76,12 +76,12 @@
 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");
+  EXPECT_THAT(IncomingLevel2,
+  ElementsAre(AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
 
   auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
   EXPECT_THAT(IncomingLevel3,
@@ -121,8 +121,8 @@
 
   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")),
+FromRanges(Source.range("Caller1");
 }
 
 TEST(CallHierarchy, IncomingQualified) {
@@ -149,10 +149,10 @@
   EXPECT_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")),
+FromRanges(Source.range("Caller1"))),
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller2");
 }
 
 TEST(CallHierarchy, IncomingMultiFile) {
@@ -219,9 +219,9 @@
   FromRanges(Caller1C.range();
 
 auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
-EXPECT_THAT(IncomingLevel2,
-UnorderedElementsAre(
-AllOf(From(WithName("caller2")),
+EXPECT_THAT(
+IncomingLevel2,
+ElementsAre(AllOf(From(WithName("caller2")),
   FromRanges(Caller2C.range("A"), 
Caller2C.range("B"))),
 AllOf(From(WithName("caller3")),
   FromRanges(Caller3C.range("Caller1");
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -1716,6 +1716,11 @@
   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;
 }
 


Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -76,12 +76,12 @@
 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(Sou