[clangd] Strip invalid fromRanges for outgoing calls

`CallHierarchyOutgoingCall::fromRanges` are interpreted as ranges
in the same file as the item for which 'outgoingCalls' was called.

It's possible for outgoing calls to be in a different file than
that item if the item is just a declaration (e.g. in a header file).
Now, such calls are dropped instead of being returned to the client.

Author: HampusAdolfsson

clang-tools-extra/clangd/XRefs.cpp

@@ -2380,7 +2380,7 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
   // Initially store the ranges in a map keyed by SymbolID of the callee.
   // This allows us to group different calls to the same function
   // into the same CallHierarchyOutgoingCall.
-  llvm::DenseMap<SymbolID, std::vector<Range>> CallsOut;
+  llvm::DenseMap<SymbolID, std::vector<Location>> CallsOut;
   // We can populate the ranges based on a refs request only. As we do so, we
   // also accumulate the callee IDs into a lookup request.
   LookupRequest CallsOutLookup;
@@ -2390,8 +2390,8 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
       elog("outgoingCalls failed to convert location: {0}", Loc.takeError());
       return;
     }
-    auto It = CallsOut.try_emplace(R.Symbol, std::vector<Range>{}).first;
-    It->second.push_back(Loc->range);
+    auto It = CallsOut.try_emplace(R.Symbol, std::vector<Location>{}).first;
+    It->second.push_back(*Loc);
 
     CallsOutLookup.IDs.insert(R.Symbol);
   });
@@ -2411,9 +2411,22 @@ outgoingCalls(const CallHierarchyItem &Item, const SymbolIndex *Index) {
 
     auto It = CallsOut.find(Callee.ID);
     assert(It != CallsOut.end());
-    if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file()))
+    if (auto CHI = symbolToCallHierarchyItem(Callee, Item.uri.file())) {
+      std::vector<Range> FromRanges;
+      for (const Location &L : It->second) {
+        if (L.uri != Item.uri) {
+          // Call location not in same file as the item that outgoingCalls was
+          // requested for. This can happen when Item is a declaration separate
+          // from the implementation. There's not much we can do, since the
+          // protocol only allows returning ranges interpreted as being in
+          // Item's file.
+          continue;
+        }
+        FromRanges.push_back(L.range);
+      }
       Results.push_back(
-          CallHierarchyOutgoingCall{std::move(*CHI), std::move(It->second)});
+          CallHierarchyOutgoingCall{std::move(*CHI), std::move(FromRanges)});
+  }
   });
   // Sort results by name of the callee.
   llvm::sort(Results, [](const CallHierarchyOutgoingCall &A,

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

@@ -383,7 +383,8 @@ TEST(CallHierarchy, MultiFileCpp) {
     EXPECT_THAT(IncomingLevel4, IsEmpty());
   };
 
-  auto CheckOutgoingCalls = [&](ParsedAST &AST, Position Pos, PathRef TUPath) {
+  auto CheckOutgoingCalls = [&](ParsedAST &AST, Position Pos, PathRef TUPath,
+                                bool IsDeclaration) {
     std::vector<CallHierarchyItem> Items =
         prepareCallHierarchy(AST, Pos, TUPath);
     ASSERT_THAT(Items, ElementsAre(withName("caller3")));
@@ -392,9 +393,11 @@ TEST(CallHierarchy, MultiFileCpp) {
         OutgoingLevel1,
         ElementsAre(
             AllOf(to(AllOf(withName("caller1"), withDetail("nsa::caller1"))),
-                  oFromRanges(Caller3C.range("Caller1"))),
+                  IsDeclaration ? oFromRanges()
+                                : oFromRanges(Caller3C.range("Caller1"))),
             AllOf(to(AllOf(withName("caller2"), withDetail("nsb::caller2"))),
-                  oFromRanges(Caller3C.range("Caller2")))));
+                  IsDeclaration ? oFromRanges()
+                                : oFromRanges(Caller3C.range("Caller2")))));
 
     auto OutgoingLevel2 = outgoingCalls(OutgoingLevel1[1].to, Index.get());
     ASSERT_THAT(OutgoingLevel2,
@@ -423,15 +426,15 @@ TEST(CallHierarchy, MultiFileCpp) {
   CheckIncomingCalls(*AST, CalleeH.point(), testPath("callee.hh"));
   AST = Workspace.openFile("caller3.hh");
   ASSERT_TRUE(bool(AST));
-  CheckOutgoingCalls(*AST, Caller3H.point(), testPath("caller3.hh"));
+  CheckOutgoingCalls(*AST, Caller3H.point(), testPath("caller3.hh"), true);
 
   // Check that invoking from the definition site works.
   AST = Workspace.openFile("callee.cc");
   ASSERT_TRUE(bool(AST));
   CheckIncomingCalls(*AST, CalleeC.point(), testPath("callee.cc"));
   AST = Workspace.openFile("caller3.cc");
   ASSERT_TRUE(bool(AST));
-  CheckOutgoingCalls(*AST, Caller3C.point(), testPath("caller3.cc"));
+  CheckOutgoingCalls(*AST, Caller3C.point(), testPath("caller3.cc"), false);
 }
 
 TEST(CallHierarchy, IncomingMultiFileObjC) {