Skip to content

[CodeCompletion] Don’t report a call pattern as convertible if its result type doesn’t match #72568

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/swift/Sema/ConstraintSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -908,7 +908,7 @@ class FunctionArgApplyInfo {
bool lookThroughAutoclosure) const {
auto param = fnTy->getParams()[ParamIdx];
auto paramTy = param.getPlainType();
if (lookThroughAutoclosure && param.isAutoClosure())
if (lookThroughAutoclosure && param.isAutoClosure() && paramTy->is<FunctionType>())
paramTy = paramTy->castTo<FunctionType>()->getResult();
return paramTy;
}
Expand Down
54 changes: 14 additions & 40 deletions lib/IDE/ArgumentCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,43 +89,6 @@ 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 target = S.getTargetFor(E);
if (!target)
return false;

assert(target->kind == SyntacticElementTarget::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;
}
}

/// Returns whether `E` has a parent expression with arguments.
static bool hasParentCallLikeExpr(Expr *E, ConstraintSystem &CS) {
E = CS.getParentExpr(E);
Expand Down Expand Up @@ -172,10 +135,21 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
auto ArgIdx = ArgInfo->completionIdx;

Type ExpectedCallType;
if (!isExpressionResultTypeUnconstrained(S, ParentCall)) {
ExpectedCallType = getTypeForCompletion(S, ParentCall);
if (auto ArgLoc = S.getConstraintSystem().getArgumentLocator(ParentCall)) {
if (auto FuncArgApplyInfo = S.getFunctionArgApplyInfo(ArgLoc)) {
Type ParamType = FuncArgApplyInfo->getParamInterfaceType();
ExpectedCallType = S.simplifyTypeForCodeCompletion(ParamType);
}
}

if (!ExpectedCallType) {
if (auto ContextualType = S.getContextualType(ParentCall)) {
ExpectedCallType = ContextualType;
}
}
if (ExpectedCallType && ExpectedCallType->hasUnresolvedType()) {
ExpectedCallType = Type();
}

auto *CallLocator = CS.getConstraintLocator(ParentCall);
auto *CalleeLocator = S.getCalleeLocator(CallLocator);

Expand Down
29 changes: 15 additions & 14 deletions test/IDE/complete_call_arg.swift
Original file line number Diff line number Diff line change
Expand Up @@ -576,17 +576,17 @@ func testStaticMemberCall() {
func testImplicitMember() {
let _: TestStaticMemberCall = .create1(#^IMPLICIT_MEMBER_AFTERPAREN_1^#)
// IMPLICIT_MEMBER_AFTERPAREN_1: Begin completions, 1 items
// IMPLICIT_MEMBER_AFTERPAREN_1: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#arg1: Int#}[')'][#TestStaticMemberCall#]; name=arg1:
// IMPLICIT_MEMBER_AFTERPAREN_1: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#arg1: Int#}[')'][#TestStaticMemberCall#]; name=arg1:

let _: TestStaticMemberCall = .create2(#^IMPLICIT_MEMBER_AFTERPAREN_2^#)
// IMPLICIT_MEMBER_AFTERPAREN_2-DAG: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#(arg1): Int#}[')'][#TestStaticMemberCall#];
// IMPLICIT_MEMBER_AFTERPAREN_2-DAG: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#(arg1): Int#}, {#arg2: Int#}, {#arg3: Int#}, {#arg4: Int#}[')'][#TestStaticMemberCall#];
// IMPLICIT_MEMBER_AFTERPAREN_2-DAG: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#(arg1): Int#}[')'][#TestStaticMemberCall#];
// IMPLICIT_MEMBER_AFTERPAREN_2-DAG: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#(arg1): Int#}, {#arg2: Int#}, {#arg3: Int#}, {#arg4: Int#}[')'][#TestStaticMemberCall#];
// IMPLICIT_MEMBER_AFTERPAREN_2-DAG: Decl[Struct]/OtherModule[Swift]/IsSystem/TypeRelation[Convertible]: Int[#Int#];
// IMPLICIT_MEMBER_AFTERPAREN_2-DAG: Literal[Integer]/None/TypeRelation[Convertible]: 0[#Int#];

let _: TestStaticMemberCall? = .create1(#^IMPLICIT_MEMBER_AFTERPAREN_3^#)
// IMPLICIT_MEMBER_AFTERPAREN_3: Begin completions, 1 items
// IMPLICIT_MEMBER_AFTERPAREN_3: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#arg1: Int#}[')'][#TestStaticMemberCall#]; name=arg1:
// IMPLICIT_MEMBER_AFTERPAREN_3: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#arg1: Int#}[')'][#TestStaticMemberCall#]; name=arg1:

let _: TestStaticMemberCall = .create2(1, #^IMPLICIT_MEMBER_SECOND^#)
// IMPLICIT_MEMBER_SECOND: Begin completions, 3 items
Expand All @@ -600,7 +600,7 @@ func testImplicitMember() {

let _: TestStaticMemberCall = .createOverloaded(#^IMPLICIT_MEMBER_OVERLOADED^#)
// IMPLICIT_MEMBER_OVERLOADED: Begin completions, 1 item
// IMPLICIT_MEMBER_OVERLOADED: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#arg1: Int#}[')'][#TestStaticMemberCall#]; name=arg1:
// IMPLICIT_MEMBER_OVERLOADED: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#arg1: Int#}[')'][#TestStaticMemberCall#]; name=arg1:
}
func testImplicitMemberInArrayLiteral() {
struct Receiver {
Expand Down Expand Up @@ -1325,8 +1325,8 @@ func testSubscriptWithExistingRhs(someString: String) {

// SUBSCRIPT_WITH_EXISTING_RHS: Begin completions
// SUBSCRIPT_WITH_EXISTING_RHS-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<[String : Any], Value>#}[']'][#Value#];
// SUBSCRIPT_WITH_EXISTING_RHS-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem/TypeRelation[Convertible]: ['[']{#(key): String#}[']'][#@lvalue Any?#];
// SUBSCRIPT_WITH_EXISTING_RHS-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem/TypeRelation[Convertible]: ['[']{#(key): String#}, {#default: Any#}[']'][#@lvalue Any#];
// SUBSCRIPT_WITH_EXISTING_RHS-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem: ['[']{#(key): String#}[']'][#@lvalue Any?#];
// SUBSCRIPT_WITH_EXISTING_RHS-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem: ['[']{#(key): String#}, {#default: Any#}[']'][#@lvalue Any#];
// SUBSCRIPT_WITH_EXISTING_RHS-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: someString[#String#];
// SUBSCRIPT_WITH_EXISTING_RHS: End completions
}
Expand All @@ -1348,8 +1348,8 @@ func testOptionalConversionInSrcOfAssignment(myArray: [Int]) {
var optInt: Int?
optInt = myArray[#^OPTIONAL_CONVERSION_IN_ASSIGNMENT^#]
// OPTIONAL_CONVERSION_IN_ASSIGNMENT: Begin completions
// OPTIONAL_CONVERSION_IN_ASSIGNMENT-DAG: Pattern/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['[']{#keyPath: KeyPath<[Int], Value>#}[']'][#Value#]; name=keyPath:
// OPTIONAL_CONVERSION_IN_ASSIGNMENT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem/TypeRelation[Convertible]: ['[']{#(index): Int#}[']'][#Int#]; name=:
// OPTIONAL_CONVERSION_IN_ASSIGNMENT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<[Int], Value>#}[']'][#Value#]; name=keyPath:
// OPTIONAL_CONVERSION_IN_ASSIGNMENT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem: ['[']{#(index): Int#}[']'][#Int#]; name=:
// OPTIONAL_CONVERSION_IN_ASSIGNMENT-DAG: Literal[Integer]/None/TypeRelation[Convertible]: 0[#Int#]; name=0
// OPTIONAL_CONVERSION_IN_ASSIGNMENT: End completions
}
Expand All @@ -1359,7 +1359,7 @@ func testAnyConversionInDestOfAssignment(_ message: String) {
userInfo[#^ANY_CONVERSION_IN_ASSIGNMENT^#] = message
// ANY_CONVERSION_IN_ASSIGNMENT: Begin completions
// ANY_CONVERSION_IN_ASSIGNMENT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<[String : Any], Value>#}[']'][#Value#]; name=keyPath:
// ANY_CONVERSION_IN_ASSIGNMENT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem/TypeRelation[Convertible]: ['[']{#(key): String#}, {#default: Any#}[']'][#@lvalue Any#]; name=:default:
// ANY_CONVERSION_IN_ASSIGNMENT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/IsSystem: ['[']{#(key): String#}, {#default: Any#}[']'][#@lvalue Any#]; name=:default:
// ANY_CONVERSION_IN_ASSIGNMENT-DAG: Decl[LocalVar]/Local: userInfo[#[String : Any]#]; name=userInfo
// ANY_CONVERSION_IN_ASSIGNMENT-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: message[#String#]; name=message
// ANY_CONVERSION_IN_ASSIGNMENT: End completions
Expand Down Expand Up @@ -1427,18 +1427,19 @@ struct NestedCallsWithoutClosingParen {
func testInDictionaryLiteral() {
let a = 1
let b = 2
_ = [a: foo(#^IN_DICTIONARY_LITERAL?check=NESTED_CALL_AT_START^#, b: 1]
_ = [a: foo(#^IN_DICTIONARY_LITERAL?check=NESTED_CALL_WITHOUT_TYPE_RELATION^#, b: 1]
// NESTED_CALL_WITHOUT_TYPE_RELATION-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#arg: Int#}, {#arg2: Int#}[')'][#Int#];
}

func testInArrayLiteral() {
_ = [foo(#^IN_ARRAY_LITERAL?check=NESTED_CALL_AT_START^#, 1]
_ = [foo(#^IN_ARRAY_LITERAL?check=NESTED_CALL_WITHOUT_TYPE_RELATION^#, 1]
}

func testInParen() {
_ = (foo(#^IN_PAREN?check=NESTED_CALL_AT_START^#)
_ = (foo(#^IN_PAREN?check=NESTED_CALL_WITHOUT_TYPE_RELATION^#)
}

func testInTuple() {
_ = (foo(#^IN_TUPLE?check=NESTED_CALL_AT_START^#, 1)
_ = (foo(#^IN_TUPLE?check=NESTED_CALL_WITHOUT_TYPE_RELATION^#, 1)
}
}
4 changes: 2 additions & 2 deletions test/IDE/complete_dont_filter_overloads_with_cc_token.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ func test(text: String) {
}

// COMPLETE: Begin completions
// COMPLETE-DAG: Decl[Constructor]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#verbatim: String#}[')'][#MyText#];
// COMPLETE-DAG: Decl[Constructor]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#(content): StringProtocol#}[')'][#MyText#];
// COMPLETE-DAG: Decl[Constructor]/CurrNominal/Flair[ArgLabels]: ['(']{#verbatim: String#}[')'][#MyText#];
// COMPLETE-DAG: Decl[Constructor]/CurrNominal/Flair[ArgLabels]: ['(']{#(content): StringProtocol#}[')'][#MyText#];
// COMPLETE-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: text[#String#];
// COMPLETE: End completions
4 changes: 2 additions & 2 deletions test/IDE/complete_enum_unresolved_dot_argument_labels.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ func testInit() {
var state = DragState.inactive
state = .dragging(#^SIGNATURE^#)
// SIGNATURE: Begin completions, 1 item
// SIGNATURE: Pattern/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#translationX: Int#}, {#translationY: Int#}[')'][#DragState#];
// SIGNATURE: Pattern/CurrNominal/Flair[ArgLabels]: ['(']{#translationX: Int#}, {#translationY: Int#}[')'][#DragState#];

state = .dragging(translationX: 2, #^ARGUMENT^#)
// ARGUMENT: Begin completions, 1 item
// ARGUMENT: Pattern/Local/Flair[ArgLabels]: {#translationY: Int#}[#Int#];

state = .defaulted(#^DEFAULTED^#)
// DEFAULTED: Begin completions, 1 items
// DEFAULTED: Pattern/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#x: Int#}, {#y: Int#}, {#z: Int#}, {#extra: Int#}[')'][#DragState#];
// DEFAULTED: Pattern/CurrNominal/Flair[ArgLabels]: ['(']{#x: Int#}, {#y: Int#}, {#z: Int#}, {#extra: Int#}[')'][#DragState#];

state = .defaulted(x: 1, #^DEFAULTEDARG^#)
state = .defaulted(x: "Wrong type", #^ERRORTYPE?check=DEFAULTEDARG^#)
Expand Down
2 changes: 1 addition & 1 deletion test/IDE/complete_in_result_builder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func testCompleteFunctionArgumentLabels() {
@TupleBuilder<String> var x1 {
StringFactory.makeString(#^FUNCTION_ARGUMENT_LABEL^#)
// FUNCTION_ARGUMENT_LABEL: Begin completions, 1 item
// FUNCTION_ARGUMENT_LABEL: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#x: String#}[')'][#String#];
// FUNCTION_ARGUMENT_LABEL: Decl[StaticMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#x: String#}[')'][#String#];
}
}

Expand Down
2 changes: 1 addition & 1 deletion test/IDE/complete_subscript.swift
Original file line number Diff line number Diff line change
Expand Up @@ -150,5 +150,5 @@ func testSettableSub(x: inout HasSettableSub) {
x[#^SETTABLE_SUBSCRIPT^#] = 32
}
// SETTABLE_SUBSCRIPT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath<HasSettableSub, Value>#}[']'][#Value#];
// SETTABLE_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['[']{#(a): String#}[']'][#@lvalue Int#];
// SETTABLE_SUBSCRIPT-DAG: Decl[Subscript]/CurrNominal/Flair[ArgLabels]: ['[']{#(a): String#}[']'][#@lvalue Int#];
// SETTABLE_SUBSCRIPT-DAG: Decl[LocalVar]/Local/TypeRelation[Convertible]: local[#String#]; name=local
15 changes: 15 additions & 0 deletions test/IDE/expected_type_of_call_pattern.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %batch-code-completion

func foo(_ x: Int) {}

struct Bar {
func bar(withString: String) -> String {}
func bar(withInt: Int) -> Int {}
}

func test() {
foo(Bar().bar(#^COMPLETE^#))
}
// Ensure that we don't report the call pattern for `bar` as `Convertible`
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#withString: String#}[')'][#String#]; name=withString:
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#withInt: Int#}[')'][#Int#]; name=withInt:
2 changes: 1 addition & 1 deletion validation-test/IDE/issues_fixed/issue-57149.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ struct ContentView: View {
}
}

// COMPLETE: Decl[InstanceMethod]/Super/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#foo: Int#}[')'][#Never#]; name=foo:
// COMPLETE: Decl[InstanceMethod]/Super/Flair[ArgLabels]: ['(']{#foo: Int#}[')'][#Never#]; name=foo: