Skip to content

Commit f292eb6

Browse files
committed
[Diagnostics] Introduce a warning diagnostic for backward trailing closure matches
Diagnose situations when trailing closure has been matched to a specific parameter via a deprecated backward scan. ```swift func multiple_trailing_with_defaults( duration: Int, animations: (() -> Void)? = nil, completion: (() -> Void)? = nil) {} multiple_trailing_with_defaults(duration: 42) {} // picks `completion:` ```
1 parent adbbc00 commit f292eb6

File tree

3 files changed

+124
-0
lines changed

3 files changed

+124
-0
lines changed

lib/Sema/CSDiagnostics.cpp

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6571,3 +6571,95 @@ SourceLoc MissingOptionalUnwrapKeyPathFailure::getLoc() const {
65716571
auto *SE = castToExpr<SubscriptExpr>(getAnchor());
65726572
return SE->getBase()->getEndLoc();
65736573
}
6574+
6575+
bool TrailingClosureRequiresExplicitLabel::diagnoseAsError() {
6576+
auto argInfo = *getFunctionArgApplyInfo(getLocator());
6577+
6578+
{
6579+
auto diagnostic = emitDiagnostic(
6580+
diag::unlabeled_trailing_closure_deprecated, argInfo.getParamLabel());
6581+
fixIt(diagnostic, argInfo);
6582+
}
6583+
6584+
if (auto *callee = argInfo.getCallee()) {
6585+
emitDiagnosticAt(callee, diag::decl_declared_here, callee->getName());
6586+
}
6587+
6588+
return true;
6589+
}
6590+
6591+
void TrailingClosureRequiresExplicitLabel::fixIt(
6592+
InFlightDiagnostic &diagnostic, const FunctionArgApplyInfo &info) const {
6593+
auto &ctx = getASTContext();
6594+
6595+
// Dig out source locations.
6596+
SourceLoc existingRParenLoc;
6597+
SourceLoc leadingCommaLoc;
6598+
6599+
auto anchor = getRawAnchor();
6600+
auto *arg = info.getArgListExpr();
6601+
Expr *fn = nullptr;
6602+
6603+
if (auto *applyExpr = getAsExpr<ApplyExpr>(anchor)) {
6604+
fn = applyExpr->getFn();
6605+
} else {
6606+
// Covers subscripts, unresolved members etc.
6607+
fn = getAsExpr(anchor);
6608+
}
6609+
6610+
if (!fn)
6611+
return;
6612+
6613+
auto *trailingClosure = info.getArgExpr();
6614+
6615+
if (auto tupleExpr = dyn_cast<TupleExpr>(arg)) {
6616+
existingRParenLoc = tupleExpr->getRParenLoc();
6617+
assert(tupleExpr->getNumElements() >= 2 && "Should be a ParenExpr?");
6618+
leadingCommaLoc = Lexer::getLocForEndOfToken(
6619+
ctx.SourceMgr,
6620+
tupleExpr->getElements()[tupleExpr->getNumElements() - 2]->getEndLoc());
6621+
} else {
6622+
auto parenExpr = cast<ParenExpr>(arg);
6623+
existingRParenLoc = parenExpr->getRParenLoc();
6624+
}
6625+
6626+
// Figure out the text to be inserted before the trailing closure.
6627+
SmallString<16> insertionText;
6628+
SourceLoc insertionLoc;
6629+
if (leadingCommaLoc.isValid()) {
6630+
insertionText += ", ";
6631+
assert(existingRParenLoc.isValid());
6632+
insertionLoc = leadingCommaLoc;
6633+
} else if (existingRParenLoc.isInvalid()) {
6634+
insertionText += "(";
6635+
insertionLoc = Lexer::getLocForEndOfToken(ctx.SourceMgr, fn->getEndLoc());
6636+
} else {
6637+
insertionLoc = existingRParenLoc;
6638+
}
6639+
6640+
// Add the label, if there is one.
6641+
auto paramName = info.getParamLabel();
6642+
if (!paramName.empty()) {
6643+
insertionText += paramName.str();
6644+
insertionText += ": ";
6645+
}
6646+
6647+
// If there is an existing right parentheses/brace, remove it while we
6648+
// insert the new text.
6649+
if (existingRParenLoc.isValid()) {
6650+
SourceLoc afterExistingRParenLoc =
6651+
Lexer::getLocForEndOfToken(ctx.SourceMgr, existingRParenLoc);
6652+
diagnostic.fixItReplaceChars(insertionLoc, afterExistingRParenLoc,
6653+
insertionText);
6654+
} else {
6655+
// Insert the appropriate prefix.
6656+
diagnostic.fixItInsert(insertionLoc, insertionText);
6657+
}
6658+
6659+
// Insert a right parenthesis/brace after the closing '}' of the trailing
6660+
// closure;
6661+
SourceLoc newRParenLoc =
6662+
Lexer::getLocForEndOfToken(ctx.SourceMgr, trailingClosure->getEndLoc());
6663+
diagnostic.fixItInsert(newRParenLoc,
6664+
isExpr<SubscriptExpr>(anchor) ? "]" : ")");
6665+
}

lib/Sema/CSDiagnostics.h

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,6 +2214,30 @@ class MissingOptionalUnwrapKeyPathFailure final : public ContextualFailure {
22142214
SourceLoc getLoc() const override;
22152215
};
22162216

2217+
/// Diagnose situations when trailing closure has been matched to a specific
2218+
/// parameter via deprecated backward scan.
2219+
///
2220+
/// \code
2221+
/// func multiple_trailing_with_defaults(
2222+
/// duration: Int,
2223+
/// animations: (() -> Void)? = nil,
2224+
/// completion: (() -> Void)? = nil) {}
2225+
///
2226+
/// multiple_trailing_with_defaults(duration: 42) {} // picks `completion:`
2227+
/// \endcode
2228+
class TrailingClosureRequiresExplicitLabel final : public FailureDiagnostic {
2229+
public:
2230+
TrailingClosureRequiresExplicitLabel(const Solution &solution,
2231+
ConstraintLocator *locator)
2232+
: FailureDiagnostic(solution, locator) {}
2233+
2234+
bool diagnoseAsError() override;
2235+
2236+
private:
2237+
void fixIt(InFlightDiagnostic &diagnostic,
2238+
const FunctionArgApplyInfo &info) const;
2239+
};
2240+
22172241
} // end namespace constraints
22182242
} // end namespace swift
22192243

lib/Sema/ConstraintSystem.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -554,6 +554,9 @@ class FunctionArgApplyInfo {
554554
ArgType(argType), ParamIdx(paramIdx), FnInterfaceType(fnInterfaceType),
555555
FnType(fnType), Callee(callee) {}
556556

557+
/// \returns The list of the arguments used for this application.
558+
Expr *getArgListExpr() const { return ArgListExpr; }
559+
557560
/// \returns The argument being applied.
558561
Expr *getArgExpr() const { return ArgExpr; }
559562

@@ -581,6 +584,11 @@ class FunctionArgApplyInfo {
581584
return Identifier();
582585
}
583586

587+
Identifier getParamLabel() const {
588+
auto param = FnType->getParams()[ParamIdx];
589+
return param.getLabel();
590+
}
591+
584592
/// \returns A textual description of the argument suitable for diagnostics.
585593
/// For an argument with an unambiguous label, this will the label. Otherwise
586594
/// it will be its position in the argument list.

0 commit comments

Comments
 (0)