From 278ecef28420d882e632a365e17decd9d7faee6c Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Wed, 10 Apr 2024 16:06:47 -0700 Subject: [PATCH] =?UTF-8?q?[CodeCompletion]=20Don=E2=80=99t=20report=20a?= =?UTF-8?q?=20call=20pattern=20as=20convertible=20if=20its=20result=20type?= =?UTF-8?q?=20doesn=E2=80=99t=20match?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To compute the expected type of a call pattern (which is the return type of the function if that call pattern is being used), we called `getTypeForCompletion` for the entire call, in the same way that we do for the code completion token. However, this pattern does not generally work. For the code completion token it worked because the code completion expression doesn’t have an inherent type and it inherits the type solely from its context. Calls, however, have an inherent return type and that type gets assigned as the `typeForCompletion`. Implement targeted checks for the two most common cases where an expected type exists: If the call that we suggest call patterns for is itself an argument to another function or if it is used in a place that has a contextual type in the constraint system (eg. a variable binding or a `return` statement). This means that we no longer return `Convertible` for call patterns in some more complex scenarios. But given that this information was computed based on incorrect results and that in those cases all call patterns had a `Convertible` type relation, I think that’s acceptable. Fixing this would require recording more information in the constraints system, which is out-of-scope for now. --- include/swift/Sema/ConstraintSystem.h | 2 +- lib/IDE/ArgumentCompletion.cpp | 54 +++++-------------- test/IDE/complete_call_arg.swift | 29 +++++----- ..._dont_filter_overloads_with_cc_token.swift | 4 +- ..._enum_unresolved_dot_argument_labels.swift | 4 +- test/IDE/complete_in_result_builder.swift | 2 +- test/IDE/complete_subscript.swift | 2 +- test/IDE/expected_type_of_call_pattern.swift | 15 ++++++ .../IDE/issues_fixed/issue-57149.swift | 2 +- 9 files changed, 52 insertions(+), 62 deletions(-) create mode 100644 test/IDE/expected_type_of_call_pattern.swift diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 9eeeb4317e69c..b524b1f973178 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -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()) paramTy = paramTy->castTo()->getResult(); return paramTy; } diff --git a/lib/IDE/ArgumentCompletion.cpp b/lib/IDE/ArgumentCompletion.cpp index 8ffc5777004c4..991f8ee1da199 100644 --- a/lib/IDE/ArgumentCompletion.cpp +++ b/lib/IDE/ArgumentCompletion.cpp @@ -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(ParentExpr)) { - if (isa(Assign->getDest())) { - // _ = is unconstrained - return true; - } - } else if (isa(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 = is unconstrained - auto contextualType = target->getExprContextualType(); - return !contextualType || contextualType->is(); - } - 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); @@ -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); diff --git a/test/IDE/complete_call_arg.swift b/test/IDE/complete_call_arg.swift index b2ee4eee4deeb..33acc03d41682 100644 --- a/test/IDE/complete_call_arg.swift +++ b/test/IDE/complete_call_arg.swift @@ -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 @@ -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 { @@ -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 } @@ -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 } @@ -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 @@ -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) } } diff --git a/test/IDE/complete_dont_filter_overloads_with_cc_token.swift b/test/IDE/complete_dont_filter_overloads_with_cc_token.swift index 796ccaf371eb7..a257da1c8abdd 100644 --- a/test/IDE/complete_dont_filter_overloads_with_cc_token.swift +++ b/test/IDE/complete_dont_filter_overloads_with_cc_token.swift @@ -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 diff --git a/test/IDE/complete_enum_unresolved_dot_argument_labels.swift b/test/IDE/complete_enum_unresolved_dot_argument_labels.swift index 86133d0222686..66dd3c68ac7c1 100644 --- a/test/IDE/complete_enum_unresolved_dot_argument_labels.swift +++ b/test/IDE/complete_enum_unresolved_dot_argument_labels.swift @@ -10,7 +10,7 @@ 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 @@ -18,7 +18,7 @@ func testInit() { 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^#) diff --git a/test/IDE/complete_in_result_builder.swift b/test/IDE/complete_in_result_builder.swift index 34cae37583436..66d5ffe42dec0 100644 --- a/test/IDE/complete_in_result_builder.swift +++ b/test/IDE/complete_in_result_builder.swift @@ -134,7 +134,7 @@ func testCompleteFunctionArgumentLabels() { @TupleBuilder 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#]; } } diff --git a/test/IDE/complete_subscript.swift b/test/IDE/complete_subscript.swift index 28dad17def01b..aa6c3a9dc48d9 100644 --- a/test/IDE/complete_subscript.swift +++ b/test/IDE/complete_subscript.swift @@ -150,5 +150,5 @@ func testSettableSub(x: inout HasSettableSub) { x[#^SETTABLE_SUBSCRIPT^#] = 32 } // SETTABLE_SUBSCRIPT-DAG: Pattern/CurrNominal/Flair[ArgLabels]: ['[']{#keyPath: KeyPath#}[']'][#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 diff --git a/test/IDE/expected_type_of_call_pattern.swift b/test/IDE/expected_type_of_call_pattern.swift new file mode 100644 index 0000000000000..7ce7a38c38a03 --- /dev/null +++ b/test/IDE/expected_type_of_call_pattern.swift @@ -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: diff --git a/validation-test/IDE/issues_fixed/issue-57149.swift b/validation-test/IDE/issues_fixed/issue-57149.swift index bcf9d48334743..d021e3756ad6f 100644 --- a/validation-test/IDE/issues_fixed/issue-57149.swift +++ b/validation-test/IDE/issues_fixed/issue-57149.swift @@ -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: