Skip to content

Commit e703ba6

Browse files
committed
[CodeCompletion] Don’t report a call pattern as convertible if its result type doesn’t match
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`. To fix this, check if we recorded a fix that allowed a type mismatch between `ParentCall` and its surrounding context. If this is the case use the surrounding type as the `ExpectedCallType`. Finally, to make this work, we need to ensure that we don’t suppress the recording of such type mismatch fixes in code completion mode.
1 parent 6cd49ef commit e703ba6

File tree

5 files changed

+63
-27
lines changed

5 files changed

+63
-27
lines changed

include/swift/Sema/CSFix.h

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2467,9 +2467,10 @@ class AllowInOutConversion final : public ContextualMismatch {
24672467
class AllowArgumentMismatch : public ContextualMismatch {
24682468
protected:
24692469
AllowArgumentMismatch(ConstraintSystem &cs, Type argType, Type paramType,
2470-
ConstraintLocator *locator)
2470+
ConstraintLocator *locator,
2471+
FixBehavior fixBehavior = FixBehavior::Error)
24712472
: AllowArgumentMismatch(cs, FixKind::AllowArgumentTypeMismatch, argType,
2472-
paramType, locator) {}
2473+
paramType, locator, fixBehavior) {}
24732474

24742475
AllowArgumentMismatch(ConstraintSystem &cs, FixKind kind, Type argType,
24752476
Type paramType, ConstraintLocator *locator,
@@ -2486,9 +2487,10 @@ class AllowArgumentMismatch : public ContextualMismatch {
24862487

24872488
bool diagnose(const Solution &solution, bool asNote = false) const override;
24882489

2489-
static AllowArgumentMismatch *create(ConstraintSystem &cs, Type argType,
2490-
Type paramType,
2491-
ConstraintLocator *locator);
2490+
static AllowArgumentMismatch *
2491+
create(ConstraintSystem &cs, Type argType, Type paramType,
2492+
ConstraintLocator *locator,
2493+
FixBehavior fixBehavior = FixBehavior::Error);
24922494

24932495
static bool classof(const ConstraintFix *fix) {
24942496
return fix->getKind() == FixKind::AllowArgumentTypeMismatch;

lib/IDE/ArgumentCompletion.cpp

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,21 @@ void ArgumentTypeCheckCompletionCallback::sawSolutionImpl(const Solution &S) {
175175
if (!isExpressionResultTypeUnconstrained(S, ParentCall)) {
176176
ExpectedCallType = getTypeForCompletion(S, ParentCall);
177177
}
178+
// If we recorded a fix to allow a type mismatch for the ParentCall,
179+
// use the type `ParentCall` should have had as the expected call type.
180+
// For example, in the following the call to `bar` gets assigned a type
181+
// of `String` but we want to report `Int` as the expected call type.
182+
//
183+
// func foo(_ x: Int) {}
184+
// func bar() -> String {}
185+
// foo(bar(#^COMPLETE^#))
186+
for (auto Fix : S.Fixes) {
187+
if (auto CM = dyn_cast<AllowArgumentMismatch>(Fix)) {
188+
if (simplifyLocatorToAnchor(CM->getLocator()) == ASTNode(ParentCall)) {
189+
ExpectedCallType = CM->getToType();
190+
}
191+
}
192+
}
178193

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

lib/Sema/CSFix.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1842,11 +1842,13 @@ bool AllowArgumentMismatch::diagnose(const Solution &solution,
18421842
return failure.diagnose(asNote);
18431843
}
18441844

1845-
AllowArgumentMismatch *
1846-
AllowArgumentMismatch::create(ConstraintSystem &cs, Type argType,
1847-
Type paramType, ConstraintLocator *locator) {
1845+
AllowArgumentMismatch *AllowArgumentMismatch::create(ConstraintSystem &cs,
1846+
Type argType,
1847+
Type paramType,
1848+
ConstraintLocator *locator,
1849+
FixBehavior fixBehavior) {
18481850
return new (cs.getAllocator())
1849-
AllowArgumentMismatch(cs, argType, paramType, locator);
1851+
AllowArgumentMismatch(cs, argType, paramType, locator, fixBehavior);
18501852
}
18511853

18521854
bool RemoveInvalidCall::diagnose(const Solution &solution, bool asNote) const {

lib/Sema/CSSimplify.cpp

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5663,23 +5663,6 @@ bool ConstraintSystem::repairFailures(
56635663
}
56645664
}
56655665

5666-
if (isForCodeCompletion()) {
5667-
// If the argument contains the code completion location, the user has not
5668-
// finished typing out this argument yet. Treat the mismatch as valid so
5669-
// we don't penalize this solution.
5670-
if (auto *arg = getAsExpr(simplifyLocatorToAnchor(loc))) {
5671-
// Ignore synthesized args like $match in implicit pattern match
5672-
// operator calls. Their source location is usually the same as the
5673-
// other (explicit) argument's so source range containment alone isn't
5674-
// sufficient.
5675-
bool isSynthesizedArg = arg->isImplicit() && isa<DeclRefExpr>(arg);
5676-
if (!isSynthesizedArg && isForCodeCompletion() &&
5677-
containsIDEInspectionTarget(arg) && !lhs->isVoid() &&
5678-
!lhs->isUninhabited())
5679-
return true;
5680-
}
5681-
}
5682-
56835666
if (repairByInsertingExplicitCall(lhs, rhs))
56845667
break;
56855668

@@ -5947,8 +5930,27 @@ bool ConstraintSystem::repairFailures(
59475930
}
59485931
}
59495932

5933+
FixBehavior fixBehavior = FixBehavior::Error;
5934+
5935+
if (isForCodeCompletion()) {
5936+
// If the argument contains the code completion location, the user has not
5937+
// finished typing out this argument yet. Treat the mismatch as valid so
5938+
// we don't penalize this solution.
5939+
if (auto *arg = getAsExpr(simplifyLocatorToAnchor(loc))) {
5940+
// Ignore synthesized args like $match in implicit pattern match
5941+
// operator calls. Their source location is usually the same as the
5942+
// other (explicit) argument's so source range containment alone isn't
5943+
// sufficient.
5944+
bool isSynthesizedArg = arg->isImplicit() && isa<DeclRefExpr>(arg);
5945+
if (!isSynthesizedArg && isForCodeCompletion() &&
5946+
containsIDEInspectionTarget(arg) && !lhs->isVoid() &&
5947+
!lhs->isUninhabited())
5948+
fixBehavior = FixBehavior::Suppress;
5949+
}
5950+
}
5951+
59505952
conversionsOrFixes.push_back(
5951-
AllowArgumentMismatch::create(*this, lhs, rhs, loc));
5953+
AllowArgumentMismatch::create(*this, lhs, rhs, loc, fixBehavior));
59525954
break;
59535955
}
59545956

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// RUN: %batch-code-completion
2+
3+
func foo(_ x: Int) {}
4+
5+
struct Bar {
6+
func bar(withString: String) -> String {}
7+
func bar(withInt: Int) -> Int {}
8+
}
9+
10+
func test() {
11+
foo(Bar().bar(#^COMPLETE^#))
12+
}
13+
// Ensure that we don't report the call pattern for `bar` as `Convertible`
14+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]: ['(']{#withString: String#}[')'][#String#]; name=withString:
15+
// COMPLETE-DAG: Decl[InstanceMethod]/CurrNominal/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#withInt: Int#}[')'][#Int#]; name=withInt:

0 commit comments

Comments
 (0)