[IDE] Report ambiguous cursor info results

Author: ahoppen

lib/IDE/CursorInfo.cpp

@@ -121,7 +121,7 @@ class NodeFinderExprResult : public NodeFinderResult {
 /// Walks the AST, looking for a node at \c LocToResolve. While walking the
 /// AST, also gathers information about shorthand shadows.
 class NodeFinder : ASTWalker {
-  SourceFile &SrcFile;
+  DeclContext &DC;
   SourceLoc LocToResolve;
 
   /// As we are walking the tree, this variable is updated to the last seen
@@ -139,11 +139,10 @@ class NodeFinder : ASTWalker {
   llvm::DenseMap<ValueDecl *, ValueDecl *> ShorthandShadowedDecls;
 
 public:
-  NodeFinder(SourceFile &SrcFile, SourceLoc LocToResolve)
-      : SrcFile(SrcFile), LocToResolve(LocToResolve),
-        DeclContextStack({&SrcFile}) {}
+  NodeFinder(DeclContext &DC, SourceLoc LocToResolve)
+      : DC(DC), LocToResolve(LocToResolve), DeclContextStack({&DC}) {}
 
-  void resolve() { SrcFile.walk(*this); }
+  void resolve() { DC.walkContext(*this); }
 
   std::unique_ptr<NodeFinderResult> takeResult() { return std::move(Result); }
 
@@ -161,9 +160,7 @@ class NodeFinder : ASTWalker {
   }
 
 private:
-  SourceManager &getSourceMgr() const {
-    return SrcFile.getASTContext().SourceMgr;
-  }
+  SourceManager &getSourceMgr() const { return DC.getASTContext().SourceMgr; }
 
   /// The decl context that is currently being walked.
   DeclContext *getCurrentDeclContext() { return DeclContextStack.back(); }
@@ -232,7 +229,8 @@ class NodeFinder : ASTWalker {
     switch (E->getKind()) {
     case ExprKind::DeclRef:
     case ExprKind::UnresolvedDot:
-    case ExprKind::UnresolvedDeclRef: {
+    case ExprKind::UnresolvedDeclRef:
+    case ExprKind::OverloadedDeclRef: {
       assert(Result == nullptr);
       Result =
           std::make_unique<NodeFinderExprResult>(E, getCurrentDeclContext());
@@ -280,13 +278,33 @@ class CursorInfoTypeCheckSolutionCallback : public TypeCheckCompletionCallback {
   };
 
 private:
-  /// The expression for which we want to provide cursor info results.
-  Expr *ResolveExpr;
+  /// The location to resolve and the \c DeclContext to resolve it in.
+  /// Note that we cannot store the expression to resolve directly because an
+  /// \c UnresolvedDeclRefExpr might be replaced by an \c OverloadedDeclRefExpr
+  /// and thus the constraint system solution doesn't know about the
+  /// \c UnresolvedDeclRefExpr. Instead, we find the expression to resolve in
+  /// the source file again after expression pre-check has run.
+  DeclContext &DC;
+  SourceLoc ResolveLoc;
 
   SmallVector<CursorInfoDeclReference, 1> Results;
 
+  Expr *getExprToResolve() {
+    NodeFinder Finder(DC, ResolveLoc);
+    Finder.resolve();
+    auto Result = Finder.takeResult();
+    if (!Result || Result->getKind() != NodeFinderResultKind::Expr) {
+      return nullptr;
+    }
+    return cast<NodeFinderExprResult>(Result.get())->getExpr();
+  }
+
   void sawSolutionImpl(const Solution &S) override {
     auto &CS = S.getConstraintSystem();
+    auto ResolveExpr = getExprToResolve();
+    if (!ResolveExpr) {
+      return;
+    }
 
     auto Locator = CS.getConstraintLocator(ResolveExpr);
     auto CalleeLocator = S.getCalleeLocator(Locator);
@@ -310,8 +328,8 @@ class CursorInfoTypeCheckSolutionCallback : public TypeCheckCompletionCallback {
   }
 
 public:
-  CursorInfoTypeCheckSolutionCallback(Expr *ResolveExpr)
-      : ResolveExpr(ResolveExpr) {}
+  CursorInfoTypeCheckSolutionCallback(DeclContext &DC, SourceLoc ResolveLoc)
+      : DC(DC), ResolveLoc(ResolveLoc) {}
 
   ArrayRef<CursorInfoDeclReference> getResults() const { return Results; }
 };
@@ -332,7 +350,7 @@ class CursorInfoDoneParsingCallback : public IDEInspectionCallbacks {
   getDeclResult(NodeFinderDeclResult *DeclResult, SourceFile *SrcFile,
                 NodeFinder &Finder) const {
     typeCheckDeclAndParentClosures(DeclResult->getDecl());
-    return new ResolvedValueRefCursorInfo(
+    auto CursorInfo = new ResolvedValueRefCursorInfo(
         SrcFile, RequestedLoc, DeclResult->getDecl(),
         /*CtorTyRef=*/nullptr,
         /*ExtTyRef=*/nullptr, /*IsRef=*/false, /*Ty=*/Type(),
@@ -352,7 +370,7 @@ class CursorInfoDoneParsingCallback : public IDEInspectionCallbacks {
     DeclContext *DC = ExprResult->getDeclContext();
 
     // Type check the statemnt containing E and listen for solutions.
-    CursorInfoTypeCheckSolutionCallback Callback(E);
+    CursorInfoTypeCheckSolutionCallback Callback(*DC, RequestedLoc);
     llvm::SaveAndRestore<TypeCheckCompletionCallback *> CompletionCollector(
         DC->getASTContext().SolutionCallback, &Callback);
     typeCheckASTNodeAtLoc(TypeCheckASTNodeAtLocContext::declContext(DC),

lib/Sema/CSSolver.cpp

@@ -1430,7 +1430,10 @@ ConstraintSystem::solve(SyntacticElementTarget &target,
     case SolutionResult::Ambiguous:
       // If salvaging produced an ambiguous result, it has already been
       // diagnosed.
-      if (stage == 1) {
+      // If we have found an ambiguous solution in the first stage, salvaging
+      // won't produce more solutions, so we can inform the solution callback
+      // about the current ambiguous solutions straight away.
+      if (stage == 1 || Context.SolutionCallback) {
         reportSolutionsToSolutionCallback(solution);
         solution.markAsDiagnosed();
         return None;
@@ -1449,8 +1452,10 @@ ConstraintSystem::solve(SyntacticElementTarget &target,
       LLVM_FALLTHROUGH;
 
     case SolutionResult::UndiagnosedError:
-      if (shouldSuppressDiagnostics()) {
-        reportSolutionsToSolutionCallback(solution);
+      /// If we have a SolutionCallback, we are inspecting constraint system
+      /// solutions directly and thus also want to receive ambiguous solutions.
+      /// Hence always run the second (salvaging) stage.
+      if (shouldSuppressDiagnostics() && !Context.SolutionCallback) {
         solution.markAsDiagnosed();
         return None;
       }

test/SourceKit/CursorInfo/cursor_ambiguous.swift

@@ -0,0 +1,17 @@
+func testAmbiguousFunctionReference() {
+  func foo(a: Int) {}
+  func foo(a: String) {}
+
+  // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):7 %s -- %s | %FileCheck %s
+  _ = foo
+
+  // RUN: %sourcekitd-test -req=cursor -pos=%(line + 1):7 %s -- %s | %FileCheck %s
+  _ = foo(a: UInt(1))
+}
+
+// CHECK: source.lang.swift.ref.function.free (2:8-2:19)
+// CHECK: <Declaration>func foo(a: <Type usr="s:Si">Int</Type>)</Declaration>
+// CHECK: SECONDARY SYMBOLS BEGIN
+// CHECK: source.lang.swift.ref.function.free (3:8-3:22)
+// CHECK: <Declaration>func foo(a: <Type usr="s:SS">String</Type>)</Declaration>
+// CHECK: SECONDARY SYMBOLS END

test/SourceKit/Refactoring/basic.swift

@@ -170,7 +170,7 @@ func hasCallToAsyncAlternative(c: ConvertAsync) {
 // RUN: %sourcekitd-test -req=cursor -pos=117:16  -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
 // RUN: %sourcekitd-test -req=cursor -pos=117:17  -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-GLOBAL
 
-// RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME-EXTRACT
+// RUN: %sourcekitd-test -req=cursor -pos=35:10 -end-pos=35:16 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-RENAME
 
 // RUN: %sourcekitd-test -req=cursor -pos=54:10 -end-pos=54:22 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-LOCAL
 // RUN: %sourcekitd-test -req=cursor -pos=54:12 -end-pos=54:22 -cursor-action %s -- %s | %FileCheck %s -check-prefix=CHECK-SELF-RENAME1
@@ -231,6 +231,8 @@ func hasCallToAsyncAlternative(c: ConvertAsync) {
 // CHECK-LOCAL-NOT: Global Rename
 // CHECK-LOCAL: ACTIONS END
 
+// CHECK-RENAME: Global Rename
+
 // CHECK-RENAME-EXTRACT: Global Rename
 // CHECK-RENAME-EXTRACT: Extract Method
 

tools/SourceKit/lib/SwiftLang/SwiftSourceDocInfo.cpp

@@ -1915,15 +1915,15 @@ static void deliverCursorInfoResults(
     // TODO: Implement delivery of other result types as more cursor info kinds
     // are migrated to be completion-like.
     CursorInfoData Data;
-    for (auto ResolvedCursorInfo : Results.getResult().ResolvedCursorInfos) {
+    for (auto ResolvedCursorInfo : Results->ResolvedCursorInfos) {
       if (auto Result = dyn_cast_or_null<ResolvedValueRefCursorInfo>(
               ResolvedCursorInfo)) {
         std::string Diagnostic; // Unused
         addCursorInfoForDecl(Data, Result, AddRefactorings, AddSymbolGraph,
                              Lang, Invoc, Diagnostic, /*PreviousSnaps=*/{});
       }
-      Data.DidReuseAST = Results->DidReuseAST;
     }
+    Data.DidReuseAST = Results->DidReuseAST;
     if (!Data.Symbols.empty()) {
       Receiver(RequestResult<CursorInfoData>::fromResult(Data));
     }
@@ -2008,7 +2008,8 @@ void SwiftLangSupport::getCursorInfo(
   std::unique_ptr<llvm::MemoryBuffer> UnresolvedInputFile =
       getASTManager()->getMemoryBuffer(RealInputFilePath, fileSystem,
                                        InputFileError);
-  if (UnresolvedInputFile) {
+  // The solver-based implementation doesn't support range based cursor info.
+  if (UnresolvedInputFile && Length == 0) {
     auto SolverBasedReceiver = [&](const RequestResult<CursorInfoData> &Res) {
       SolverBasedProducedResult = true;
       Receiver(Res);