Skip to content

[CodeCompletion][Sema] Refine logic that ignores missing arguments after the code completion position when solving #35136

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
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
10 changes: 6 additions & 4 deletions include/swift/Sema/ConstraintLocator.h
Original file line number Diff line number Diff line change
Expand Up @@ -511,12 +511,14 @@ class LocatorPathElt::ApplyArgToParam final : public StoredIntegerElement<3> {
}
};

class LocatorPathElt::SynthesizedArgument final : public StoredIntegerElement<1> {
class LocatorPathElt::SynthesizedArgument final : public StoredIntegerElement<2> {
public:
SynthesizedArgument(unsigned index)
: StoredIntegerElement(ConstraintLocator::SynthesizedArgument, index) {}
SynthesizedArgument(unsigned index, bool afterCodeCompletionLoc = false)
: StoredIntegerElement(ConstraintLocator::SynthesizedArgument, index,
afterCodeCompletionLoc) {}

unsigned getIndex() const { return getValue(); }
unsigned getIndex() const { return getValue<0>(); }
bool isAfterCodeCompletionLoc() const { return getValue<1>(); }

static bool classof(const LocatorPathElt *elt) {
return elt->getKind() == ConstraintLocator::SynthesizedArgument;
Expand Down
34 changes: 24 additions & 10 deletions lib/Sema/CSBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1697,20 +1697,34 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {

cs.addConstraint(ConstraintKind::Bind, TypeVar, type, srcLocator);

auto reportHole = [&]() {
if (cs.isForCodeCompletion()) {
// Don't penalize solutions with unresolved generics.
if (TypeVar->getImpl().getGenericParameter())
return false;
// Don't penalize solutions with holes due to missing arguments after the
// code completion position.
auto argLoc = srcLocator->findLast<LocatorPathElt::SynthesizedArgument>();
if (argLoc && argLoc->isAfterCodeCompletionLoc())
return false;
}
// Reflect in the score that this type variable couldn't be
// resolved and had to be bound to a placeholder "hole" type.
cs.increaseScore(SK_Hole);

if (auto fix = fixForHole(cs)) {
if (cs.recordFix(/*fix=*/fix->first, /*impact=*/fix->second))
return true;
}
return false;
};

// If this was from a defaultable binding note that.
if (Binding.isDefaultableBinding()) {
cs.DefaultedConstraints.push_back(srcLocator);

if (type->isPlaceholder()) {
// Reflect in the score that this type variable couldn't be
// resolved and had to be bound to a placeholder "hole" type.
cs.increaseScore(SK_Hole);

if (auto fix = fixForHole(cs)) {
if (cs.recordFix(/*fix=*/fix->first, /*impact=*/fix->second))
return true;
}
}
if (type->isPlaceholder() && reportHole())
return true;
}

return !cs.failedConstraint && !cs.simplify();
Expand Down
77 changes: 54 additions & 23 deletions lib/Sema/CSSimplify.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -986,8 +986,35 @@ constraints::matchCallArguments(
};
}

static Optional<unsigned>
getCompletionArgIndex(ASTNode anchor, ConstraintSystem &CS) {
struct CompletionArgInfo {
unsigned completionIdx;
Optional<unsigned> firstTrailingIdx;

bool isAllowableMissingArg(unsigned argInsertIdx,
AnyFunctionType::Param param) {
// If the argument is before or at the index of the argument containing the
// completion, the user would likely have already written it if they
// intended this overload.
if (completionIdx >= argInsertIdx)
return false;

// If the argument is after the first trailing closure, the user can only
// continue on to write more trailing arguments, so only allow this overload
// if the missing argument is of function type.
if (firstTrailingIdx && argInsertIdx > *firstTrailingIdx) {
if (param.isInOut())
return false;

Type expectedTy = param.getPlainType()->lookThroughAllOptionalTypes();
return expectedTy->is<FunctionType>() || expectedTy->isAny() ||
expectedTy->isTypeVariableOrMember();
}
return true;
}
};

static Optional<CompletionArgInfo>
getCompletionArgInfo(ASTNode anchor, ConstraintSystem &CS) {
Expr *arg = nullptr;
if (auto *CE = getAsExpr<CallExpr>(anchor))
arg = CE->getArg();
Expand All @@ -999,16 +1026,15 @@ getCompletionArgIndex(ASTNode anchor, ConstraintSystem &CS) {
if (!arg)
return None;

if (auto *TE = dyn_cast<TupleExpr>(arg)) {
auto elems = TE->getElements();
auto idx = llvm::find_if(elems, [&](Expr *elem) {
return CS.containsCodeCompletionLoc(elem);
});
if (idx != elems.end())
return std::distance(elems.begin(), idx);
} else if (auto *PE = dyn_cast<ParenExpr>(arg)) {
auto trailingIdx = arg->getUnlabeledTrailingClosureIndexOfPackedArgument();
if (auto *PE = dyn_cast<ParenExpr>(arg)) {
if (CS.containsCodeCompletionLoc(PE->getSubExpr()))
return 0;
return CompletionArgInfo{ 0, trailingIdx };
} else if (auto *TE = dyn_cast<TupleExpr>(arg)) {
for (unsigned i: indices(TE->getElements())) {
if (CS.containsCodeCompletionLoc(TE->getElement(i)))
return CompletionArgInfo{ i, trailingIdx };
}
}
return None;
}
Expand All @@ -1021,7 +1047,7 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {

SmallVector<SynthesizedArg, 4> MissingArguments;
SmallVector<std::pair<unsigned, AnyFunctionType::Param>, 4> ExtraArguments;
Optional<unsigned> CompletionArgIdx;
Optional<CompletionArgInfo> CompletionArgInfo;

public:
ArgumentFailureTracker(ConstraintSystem &cs,
Expand All @@ -1048,10 +1074,19 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
const auto &param = Parameters[paramIdx];

unsigned newArgIdx = Arguments.size();

bool isAfterCodeCompletionLoc = false;
if (CS.isForCodeCompletion()) {
if (!CompletionArgInfo)
CompletionArgInfo = getCompletionArgInfo(Locator.getAnchor(), CS);
isAfterCodeCompletionLoc = CompletionArgInfo &&
CompletionArgInfo->isAllowableMissingArg(argInsertIdx, param);
}

auto *argLoc = CS.getConstraintLocator(
Locator, {LocatorPathElt::ApplyArgToParam(newArgIdx, paramIdx,
param.getParameterFlags()),
LocatorPathElt::SynthesizedArgument(newArgIdx)});
LocatorPathElt::SynthesizedArgument(newArgIdx, isAfterCodeCompletionLoc)});

auto *argType =
CS.createTypeVariable(argLoc, TVO_CanBindToInOut | TVO_CanBindToLValue |
Expand All @@ -1060,16 +1095,12 @@ class ArgumentFailureTracker : public MatchCallArgumentListener {
auto synthesizedArg = param.withType(argType);
Arguments.push_back(synthesizedArg);

if (CS.isForCodeCompletion()) {
// When solving for code completion, if any argument contains the
// completion location, later arguments shouldn't be considered missing
// (causing the solution to have a worse score) as the user just hasn't
// written them yet. Early exit to avoid recording them in this case.
if (!CompletionArgIdx)
CompletionArgIdx = getCompletionArgIndex(Locator.getAnchor(), CS);
if (CompletionArgIdx && *CompletionArgIdx < argInsertIdx)
return newArgIdx;
}
// When solving for code completion, if any argument contains the
// completion location, later arguments shouldn't be considered missing
// (causing the solution to have a worse score) as the user just hasn't
// written them yet. Early exit to avoid recording them in this case.
if (isAfterCodeCompletionLoc)
return newArgIdx;

MissingArguments.push_back(SynthesizedArg{paramIdx, synthesizedArg});

Expand Down
54 changes: 51 additions & 3 deletions test/IDE/complete_ambiguous.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=REGULAR_MULTICLOSURE_APPLIED | %FileCheck %s --check-prefix=POINT_MEMBER
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=BEST_SOLUTION_FILTER | %FileCheck %s --check-prefix=BEST_SOLUTION_FILTER
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=BEST_SOLUTION_FILTER2 | %FileCheck %s --check-prefix=BEST_SOLUTION_FILTER
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=MISSINGARG_INLINE | %FileCheck %s --check-prefix=MISSINGARG_INLINE
// RUN: %swift-ide-test -code-completion -source-filename %s -code-completion-token=MISSINGARG_TRAILING | %FileCheck %s --check-prefix=MISSINGARG_TRAILING


struct A {
Expand Down Expand Up @@ -313,9 +315,55 @@ func testMissingArgs() {

test3(after: Test.#^OVERLOADEDFUNC_MISSINGARG_BEFORE^#);
test4(both: Test.#^OVERLOADEDFUNC_MISSINGARG_BEFOREANDAFTER^#)
}


enum Bop { case bop }
enum Bix { case bix }
enum Blu { case blu }
enum Baz { case baz }
enum Boy { case boy }

func trailing(x: Int, _ y: () -> Foo, z: () -> ()) {}
func trailing(x: Int, _ y: () -> Bar, z: (() -> ())?) {}
func trailing(x: Int, _ y: () -> Bop, z: Any) {}
func trailing<T>(x: Int, _ y: () -> Bix, z: T) {}
func trailing<T>(x: Int, _ y: () -> Boy, z: T?) {}
func trailing<T>(x: Int, _ y: () -> Blu, z: [T]?) {}
func trailing(x: Int, _ y: () -> Blu, z: inout Any) {}
func trailing(x: Int, _ y: () -> Baz, z: Int) {}

trailing(x: 2, { .#^MISSINGARG_INLINE^# })
trailing(x: 2) { .#^MISSINGARG_TRAILING^# }

// MISSINGARG_INLINE: Begin completions, 14 items
// MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: foo[#Foo#]; name=foo
// MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Foo#})[#(into: inout Hasher) -> Void#]; name=hash(self: Foo)
// MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bar[#Bar#]; name=bar
// MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Bar#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bar)
// MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bop[#Bop#]; name=bop
// MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Bop#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bop)
// MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bix[#Bix#]; name=bix
// MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Bix#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bix)
// MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: boy[#Boy#]; name=boy
// MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Boy#})[#(into: inout Hasher) -> Void#]; name=hash(self: Boy)
// MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: blu[#Blu#]; name=blu
// MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Blu#})[#(into: inout Hasher) -> Void#]; name=hash(self: Blu)
// MISSINGARG_INLINE-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: baz[#Baz#]; name=baz
// MISSINGARG_INLINE-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Baz#})[#(into: inout Hasher) -> Void#]; name=hash(self: Baz)
// MISSINGARG_INLINE: End completions

// MISSINGARG_TRAILING: Begin completions, 10 items
// MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: foo[#Foo#]; name=foo
// MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Foo#})[#(into: inout Hasher) -> Void#]; name=hash(self: Foo)
// MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bar[#Bar#]; name=bar
// MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Bar#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bar)
// MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bop[#Bop#]; name=bop
// MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Bop#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bop)
// MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: bix[#Bix#]; name=bix
// MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Bix#})[#(into: inout Hasher) -> Void#]; name=hash(self: Bix)
// MISSINGARG_TRAILING-DAG: Decl[EnumElement]/ExprSpecific/TypeRelation[Identical]: boy[#Boy#]; name=boy
// MISSINGARG_TRAILING-DAG: Decl[InstanceMethod]/CurrNominal: hash({#(self): Boy#})[#(into: inout Hasher) -> Void#]; name=hash(self: Boy)
// MISSINGARG_TRAILING: End completions
}

protocol C {
associatedtype Element
Expand Down Expand Up @@ -450,7 +498,7 @@ struct Struct123: Equatable {
func testNoBestSolutionFilter() {
let a = Struct123();
let b = [Struct123]().first(where: { $0 == a && 1 + 90 * 5 / 8 == 45 * -10 })?.structMem != .#^BEST_SOLUTION_FILTER^#
let c = min(10.3, 10 / 10.4) < 6 + 5 / (10 - 3) ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2^#
let c = min(10.3, 10 / 10.4) < 6 / 7 ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2^#
}

// BEST_SOLUTION_FILTER: Begin completions
Expand Down