Merge pull request #63712 from ahoppen/ahoppen/completion-misc

ahoppen · web-flow · commit 0ed6deea9bd8 · 2023-02-28T16:02:58.000+01:00
[CodeCompletion] Collection of minor changes to prepare migrating remaining completion kinds to solver-based
diff --git a/include/swift/AST/ASTNode.h b/include/swift/AST/ASTNode.h
@@ -107,6 +107,19 @@ namespace llvm {
       return LHS.getOpaqueValue() == RHS.getOpaqueValue();
     }
   };
+
+  // A ASTNode is "pointer like".
+  template <>
+  struct PointerLikeTypeTraits<ASTNode> {
+  public:
+    static inline void *getAsVoidPointer(ASTNode N) {
+      return (void *)N.getOpaqueValue();
+    }
+    static inline ASTNode getFromVoidPointer(void *P) {
+      return ASTNode::getFromOpaqueValue(P);
+    }
+    enum { NumLowBitsAvailable = swift::TypeAlignInBits };
+  };
 }
 
 #endif // LLVM_SWIFT_AST_AST_NODE_H
diff --git a/include/swift/IDE/ArgumentCompletion.h b/include/swift/IDE/ArgumentCompletion.h
@@ -25,6 +25,8 @@ class ArgumentTypeCheckCompletionCallback : public TypeCheckCompletionCallback {
   struct Result {
     /// The type associated with the code completion expression itself.
     Type ExpectedType;
+    /// The expected return type of the function call.
+    Type ExpectedCallType;
     /// True if this is a subscript rather than a function call.
     bool IsSubscript;
     /// The FuncDecl or SubscriptDecl associated with the call.
@@ -70,6 +72,11 @@ class ArgumentTypeCheckCompletionCallback : public TypeCheckCompletionCallback {
 
   void sawSolutionImpl(const constraints::Solution &solution) override;
 
+  /// Populates \p ShadowedDecls with all \c FuncD in \p Results that are
+  /// defined in protocol extensions but redeclared on a nominal type and thus
+  /// cannot be accessed
+  void computeShadowedDecls(SmallPtrSetImpl<ValueDecl *> &ShadowedDecls);
+
 public:
   ArgumentTypeCheckCompletionCallback(CodeCompletionExpr *CompletionExpr,
                                       DeclContext *DC)
diff --git a/lib/IDE/ArgumentCompletion.cpp b/lib/IDE/ArgumentCompletion.cpp
@@ -24,7 +24,7 @@ using namespace swift::constraints;
 bool ArgumentTypeCheckCompletionCallback::addPossibleParams(
     const ArgumentTypeCheckCompletionCallback::Result &Res,
     SmallVectorImpl<PossibleParamInfo> &Params, SmallVectorImpl<Type> &Types) {
-  if (!Res.ParamIdx) {
+  if (!Res.ParamIdx || !Res.FuncTy) {
     // We don't really know much here. Suggest global results without a specific
     // expected type.
     return true;
@@ -78,6 +78,44 @@ bool ArgumentTypeCheckCompletionCallback::addPossibleParams(
   return ShowGlobalCompletions;
 }
 
+/// Applies heuristic to determine whether the result type of \p E is
+/// unconstrained, that is if the constraint system is satisfiable for any
+/// result type of \p E.
+static bool isExpressionResultTypeUnconstrained(const Solution &S, Expr *E) {
+  ConstraintSystem &CS = S.getConstraintSystem();
+  if (auto ParentExpr = CS.getParentExpr(E)) {
+    if (auto Assign = dyn_cast<AssignExpr>(ParentExpr)) {
+      if (isa<DiscardAssignmentExpr>(Assign->getDest())) {
+        // _ = <expr> is unconstrained
+        return true;
+      }
+    } else if (isa<RebindSelfInConstructorExpr>(ParentExpr)) {
+      // super.init() is unconstrained (it always produces the correct result
+      // by definition)
+      return true;
+    }
+  }
+  auto targetIt = S.solutionApplicationTargets.find(E);
+  if (targetIt == S.solutionApplicationTargets.end()) {
+    return false;
+  }
+  auto target = targetIt->second;
+  assert(target.kind == SolutionApplicationTarget::Kind::expression);
+  switch (target.getExprContextualTypePurpose()) {
+  case CTP_Unused:
+    // If we aren't using the contextual type, its unconstrained by definition.
+    return true;
+  case CTP_Initialization: {
+    // let x = <expr> is unconstrained
+    auto contextualType = target.getExprContextualType();
+    return !contextualType || contextualType->is<UnresolvedType>();
+  }
+  default:
+    // Assume that it's constrained by default.
+    return false;
+  }
+}
+
 void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
   Type ExpectedTy = getTypeForCompletion(S, CompletionExpr);
 
@@ -111,10 +149,25 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
   }
   auto ArgIdx = ArgInfo->completionIdx;
 
+  Type ExpectedCallType;
+  if (!isExpressionResultTypeUnconstrained(S, ParentCall)) {
+    ExpectedCallType = getTypeForCompletion(S, ParentCall);
+  }
+
   auto *CallLocator = CS.getConstraintLocator(ParentCall);
   auto *CalleeLocator = S.getCalleeLocator(CallLocator);
 
   auto Info = getSelectedOverloadInfo(S, CalleeLocator);
+  if (Info.Value && Info.Value->shouldHideFromEditor()) {
+    return;
+  }
+  // Disallow invalid initializer references
+  for (auto Fix : S.Fixes) {
+    if (Fix->getLocator() == CalleeLocator &&
+        Fix->getKind() == FixKind::AllowInvalidInitRef) {
+      return;
+    }
+  }
 
   // Find the parameter the completion was bound to (if any), as well as which
   // parameters are already bound (so we don't suggest them even when the args
@@ -179,10 +232,39 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
   if (Info.ValueTy) {
     FuncTy = Info.ValueTy->lookThroughAllOptionalTypes()->getAs<AnyFunctionType>();
   }
-  Results.push_back({ExpectedTy, isa<SubscriptExpr>(ParentCall), Info.Value,
-                     FuncTy, ArgIdx, ParamIdx, std::move(ClaimedParams),
-                     IsNoninitialVariadic, Info.BaseTy, HasLabel, IsAsync,
-                     SolutionSpecificVarTypes});
+  Results.push_back({ExpectedTy, ExpectedCallType,
+                     isa<SubscriptExpr>(ParentCall), Info.Value, FuncTy, ArgIdx,
+                     ParamIdx, std::move(ClaimedParams), IsNoninitialVariadic,
+                     Info.BaseTy, HasLabel, IsAsync, SolutionSpecificVarTypes});
+}
+
+void ArgumentTypeCheckCompletionCallback::computeShadowedDecls(
+    SmallPtrSetImpl<ValueDecl *> &ShadowedDecls) {
+  for (size_t i = 0; i < Results.size(); ++i) {
+    auto &ResultA = Results[i];
+    for (size_t j = i + 1; j < Results.size(); ++j) {
+      auto &ResultB = Results[j];
+      if (!ResultA.FuncD || !ResultB.FuncD || !ResultA.FuncTy || !ResultB.FuncTy) {
+        continue;
+      }
+      if (ResultA.FuncD->getName() != ResultB.FuncD->getName()) {
+        continue;
+      }
+      if (!ResultA.FuncTy->isEqual(ResultB.FuncTy)) {
+        continue;
+      }
+      ProtocolDecl *inProtocolExtensionA =
+          ResultA.FuncD->getDeclContext()->getExtendedProtocolDecl();
+      ProtocolDecl *inProtocolExtensionB =
+          ResultB.FuncD->getDeclContext()->getExtendedProtocolDecl();
+
+      if (inProtocolExtensionA && !inProtocolExtensionB) {
+        ShadowedDecls.insert(ResultA.FuncD);
+      } else if (!inProtocolExtensionA && inProtocolExtensionB) {
+        ShadowedDecls.insert(ResultB.FuncD);
+      }
+    }
+  }
 }
 
 void ArgumentTypeCheckCompletionCallback::deliverResults(
@@ -193,12 +275,23 @@ void ArgumentTypeCheckCompletionCallback::deliverResults(
   CompletionLookup Lookup(CompletionCtx.getResultSink(), Ctx, DC,
                           &CompletionCtx);
 
+  SmallPtrSet<ValueDecl *, 4> ShadowedDecls;
+  computeShadowedDecls(ShadowedDecls);
+
   // Perform global completion as a fallback if we don't have any results.
   bool shouldPerformGlobalCompletion = Results.empty();
+  SmallVector<Type, 4> ExpectedCallTypes;
+  for (auto &Result : Results) {
+    ExpectedCallTypes.push_back(Result.ExpectedCallType);
+  }
+
   SmallVector<Type, 8> ExpectedTypes;
 
   if (IncludeSignature && !Results.empty()) {
     Lookup.setHaveLParen(true);
+    Lookup.setExpectedTypes(ExpectedCallTypes,
+                            /*isImplicitSingleExpressionReturn=*/false);
+
     for (auto &Result : Results) {
       auto SemanticContext = SemanticContextKind::None;
       NominalTypeDecl *BaseNominal = nullptr;
@@ -229,13 +322,17 @@ void ArgumentTypeCheckCompletionCallback::deliverResults(
       }
       if (Result.FuncTy) {
         if (auto FuncTy = Result.FuncTy) {
-          if (Result.IsSubscript) {
-            assert(SemanticContext != SemanticContextKind::None);
-            auto *SD = dyn_cast_or_null<SubscriptDecl>(Result.FuncD);
-            Lookup.addSubscriptCallPattern(FuncTy, SD, SemanticContext);
-          } else {
-            auto *FD = dyn_cast_or_null<AbstractFunctionDecl>(Result.FuncD);
-            Lookup.addFunctionCallPattern(FuncTy, FD, SemanticContext);
+          if (ShadowedDecls.count(Result.FuncD) == 0) {
+            // Don't show call pattern completions if the function is
+            // overridden.
+            if (Result.IsSubscript) {
+              assert(SemanticContext != SemanticContextKind::None);
+              auto *SD = dyn_cast_or_null<SubscriptDecl>(Result.FuncD);
+              Lookup.addSubscriptCallPattern(FuncTy, SD, SemanticContext);
+            } else {
+              auto *FD = dyn_cast_or_null<AbstractFunctionDecl>(Result.FuncD);
+              Lookup.addFunctionCallPattern(FuncTy, FD, SemanticContext);
+            }
           }
         }
       }
diff --git a/lib/IDE/CodeCompletion.cpp b/lib/IDE/CodeCompletion.cpp
@@ -1441,8 +1441,12 @@ bool CodeCompletionCallbacksImpl::trySolverCompletion(bool MaybeFuncBody) {
     // switch case where there control expression is invalid). Having normal
     // typechecking still resolve even these cases would be beneficial for
     // tooling in general though.
-    if (!Lookup.gotCallback())
+    if (!Lookup.gotCallback()) {
+      if (Context.TypeCheckerOpts.DebugConstraintSolver) {
+        llvm::errs() << "--- Fallback typecheck for code completion ---\n";
+      }
       Lookup.fallbackTypeCheck(CurDeclContext);
+    }
   };
 
   switch (Kind) {
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -9179,12 +9179,17 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
 
       // Only try and favor monomorphic unary initializers.
       if (!ctor->isGenericContext()) {
-        auto args = ctor->getMethodInterfaceType()
-                        ->castTo<FunctionType>()->getParams();
-        if (args.size() == 1 && !args[0].hasLabel() &&
-            args[0].getPlainType()->isEqual(favoredType)) {
-          if (!isDeclUnavailable(decl, memberLocator))
-            result.FavoredChoice = result.ViableCandidates.size();
+        if (!ctor->getMethodInterfaceType()->hasError()) {
+          // The constructor might have an error type because we don't skip
+          // invalid decls for code completion
+          auto args = ctor->getMethodInterfaceType()
+                          ->castTo<FunctionType>()
+                          ->getParams();
+          if (args.size() == 1 && !args[0].hasLabel() &&
+              args[0].getPlainType()->isEqual(favoredType)) {
+            if (!isDeclUnavailable(decl, memberLocator))
+              result.FavoredChoice = result.ViableCandidates.size();
+          }
         }
       }
     }
diff --git a/lib/Sema/TypeCheckCodeCompletion.cpp b/lib/Sema/TypeCheckCodeCompletion.cpp
@@ -624,6 +624,7 @@ bool TypeChecker::typeCheckForCodeCompletion(
 
     // If solve failed to generate constraints or with some other
     // issue, we need to fallback to type-checking a sub-expression.
+    cs.setSolutionApplicationTarget(target.getAsExpr(), target);
     if (!cs.solveForCodeCompletion(target, solutions))
       return CompletionResult::Fallback;
 
diff --git a/test/IDE/complete_call_arg.swift b/test/IDE/complete_call_arg.swift
@@ -1231,7 +1231,7 @@ private extension Sequence {
   func SubstitutableBaseTyOfSubscript<T: Comparable>(by keyPath: KeyPath<Element, T>) -> [Element] {
     return sorted { a, b in a[#^GENERICBASE_SUB^#] }
     // GENERICBASE_SUB: Begin completions, 1 item
-    // GENERICBASE_SUB: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<Self.Element, Value>#}[']'][#Value#];
+    // GENERICBASE_SUB: Pattern/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['[']{#keyPath: KeyPath<Self.Element, Value>#}[']'][#Value#];
     // GENERICBASE_SUB: End completions
   }
 }
diff --git a/test/IDE/complete_expr_after_paren.swift b/test/IDE/complete_expr_after_paren.swift
@@ -1,7 +1,5 @@
-// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=INITIALIZER | %FileCheck %s --check-prefix=INITIALIZER
-// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=METHOD | %FileCheck %s --check-prefix=METHOD
-// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=AVAILABILITY | %FileCheck %s --check-prefix=AVAILABILITY
-// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=STATIC | %FileCheck %s --check-prefix=STATIC
+// RUN: %empty-directory(%t)
+// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t
 
 protocol MyProtocol {
   init(init1: Int)
@@ -89,3 +87,33 @@ func testStaticFunc() {
 // STATIC-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]:   ['(']{#(self): TestStatic#}[')'][#() -> Void#];
 // STATIC: End completions
 }
+
+protocol TestShadowedProtocol {}
+
+extension TestShadowedProtocol {
+  func argOverloaded(arg: String) {}
+  func argOverloaded(arg: Int) {}
+
+  func returnTypeOverloaded() -> String {}
+  func returnTypeOverloaded() -> Int {}
+}
+
+struct TestShadowedStruct: TestShadowedProtocol {
+  func argOverloaded(arg: String) {}
+
+  func returnTypeOverloaded() -> String {}
+
+  func test() {
+    self.argOverloaded(#^ARG_OVERLOADED^#)
+    // ARG_OVERLOADED: Begin completions, 2 items
+    // ARG_OVERLOADED-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#arg: String#}[')'][#Void#]; name=arg:
+    // ARG_OVERLOADED-DAG: Decl[InstanceMethod]/Super/Flair[ArgLabels]: ['(']{#arg: Int#}[')'][#Void#]; name=arg:
+    // ARG_OVERLOADED: End completions
+
+    self.returnTypeOverloaded(#^RETURN_OVERLOADED^#)
+    // RETURN_OVERLOADED: Begin completions, 2 items
+    // RETURN_OVERLOADED-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]: ['('][')'][#String#]; name=
+    // RETURN_OVERLOADED-DAG: Decl[InstanceMethod]/Super/Flair[ArgLabels]: ['('][')'][#Int#]; name=
+    // RETURN_OVERLOADED: End completions
+  }
+}
diff --git a/test/IDE/complete_subscript.swift b/test/IDE/complete_subscript.swift
@@ -162,7 +162,7 @@ func testSubcscriptTuple(val: (x: Int, String)) {
 }
 
 struct HasSettableSub {
-    subscript(a: String) -> Any {
+    subscript(a: String) -> Int {
         get { return 1 }
         set { }
     }
@@ -174,6 +174,6 @@ func testSettableSub(x: inout HasSettableSub) {
 }
 // SETTABLE_SUBSCRIPT: Begin completions
 // SETTABLE_SUBSCRIPT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<HasSettableSub, Value>#}[']'][#Value#];
-// SETTABLE_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#(a): String#}[']'][#@lvalue Any#];
+// SETTABLE_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['[']{#(a): String#}[']'][#@lvalue Int#];
 // SETTABLE_SUBSCRIPT-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: local[#String#]; name=local
 // SETTABLE_SUBSCRIPT: End completions
diff --git a/validation-test/IDE/crashers_2_fixed/0037-constructor-with-error-type.swift b/validation-test/IDE/crashers_2_fixed/0037-constructor-with-error-type.swift
@@ -0,0 +1,12 @@
+// Make sure we don't crash
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token COMPLETE
+
+public struct AnyError {
+
+  public init(_ error: Swift.
+
+extension AnyError {
+  public static func error(from error: Error) -> AnyError {
+    return AnyError(error)#^COMPLETE^#
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -1231,7 +1231,7 @@ private extension Sequence {`
`1231`	`1231`	`func SubstitutableBaseTyOfSubscript<T: Comparable>(by keyPath: KeyPath<Element, T>) -> [Element] {`
`1232`	`1232`	`return sorted { a, b in a[#^GENERICBASE_SUB^#] }`
`1233`	`1233`	`// GENERICBASE_SUB: Begin completions, 1 item`
`1234`		`- // GENERICBASE_SUB: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<Self.Element, Value>#}[']'][#Value#];`
	`1234`	`+ // GENERICBASE_SUB: Pattern/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['[']{#keyPath: KeyPath<Self.Element, Value>#}[']'][#Value#];`
`1235`	`1235`	`// GENERICBASE_SUB: End completions`
`1236`	`1236`	`}`
`1237`	`1237`	`}`
Original file line number	Diff line number	Diff line change
`@@ -162,7 +162,7 @@ func testSubcscriptTuple(val: (x: Int, String)) {`
`162`	`162`	`}`
`163`	`163`
`164`	`164`	`struct HasSettableSub {`
`165`		`- subscript(a: String) -> Any {`
	`165`	`+ subscript(a: String) -> Int {`
`166`	`166`	`get { return 1 }`
`167`	`167`	`set { }`
`168`	`168`	`}`
`@@ -174,6 +174,6 @@ func testSettableSub(x: inout HasSettableSub) {`
`174`	`174`	`}`
`175`	`175`	`// SETTABLE_SUBSCRIPT: Begin completions`
`176`	`176`	`// SETTABLE_SUBSCRIPT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<HasSettableSub, Value>#}[']'][#Value#];`
`177`		`-// SETTABLE_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#(a): String#}[']'][#@lvalue Any#];`
	`177`	`+// SETTABLE_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['[']{#(a): String#}[']'][#@lvalue Int#];`
`178`	`178`	`// SETTABLE_SUBSCRIPT-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: local[#String#]; name=local`
`179`	`179`	`// SETTABLE_SUBSCRIPT: End completions`