Skip to content

Commit 9c916fc

Browse files
xedinxedin
authored
Merge pull request swiftlang#23072 from xedin/diag-closure-param-destructuring
[ConstraintSystem] Diagnose closure parameter destructuring via fixes
2 parents 6948319 + 9999a47 commit 9c916fc

File tree

6 files changed

+206
-124
lines changed

6 files changed

+206
-124
lines changed

‎lib/Sema/CSDiag.cpp‎

Lines changed: 3 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -5800,128 +5800,9 @@ bool FailureDiagnosis::diagnoseClosureExpr(
58005800
// error about an attempt to make use of tuple splat or tuple
58015801
// destructuring and provide a proper fix-it.
58025802
if (argTupleTy->getNumElements() == actualArgCount) {
5803-
// In case of implicit parameters e.g. $0, $1 we
5804-
// can't really provide good fix-it because
5805-
// structure of parameter type itself is unclear.
5806-
for (auto *param : params->getArray()) {
5807-
if (param->isImplicit()) {
5808-
diagnose(params->getStartLoc(),
5809-
diag::closure_tuple_parameter_destructuring_implicit,
5810-
argTupleTy);
5811-
return true;
5812-
}
5813-
}
5814-
5815-
auto diag = diagnose(params->getStartLoc(),
5816-
diag::closure_tuple_parameter_destructuring,
5817-
argTupleTy);
5818-
5819-
auto *closureBody = CE->getBody();
5820-
if (!closureBody)
5821-
return true;
5822-
5823-
auto &sourceMgr = CS.getASTContext().SourceMgr;
5824-
auto bodyStmts = closureBody->getElements();
5825-
5826-
SourceLoc bodyLoc;
5827-
// If the body is empty let's put the cursor
5828-
// right after "in", otherwise make it start
5829-
// location of the first statement in the body.
5830-
if (bodyStmts.empty())
5831-
bodyLoc = Lexer::getLocForEndOfToken(sourceMgr, CE->getInLoc());
5832-
else
5833-
bodyLoc = bodyStmts.front().getStartLoc();
5834-
5835-
SmallString<64> fixIt;
5836-
llvm::raw_svector_ostream OS(fixIt);
5837-
5838-
// If this is multi-line closure we'd have to insert new lines
5839-
// in the suggested 'let' to keep the structure of the code intact,
5840-
// otherwise just use ';' to keep everything on the same line.
5841-
auto inLine = sourceMgr.getLineNumber(CE->getInLoc());
5842-
auto bodyLine = sourceMgr.getLineNumber(bodyLoc);
5843-
auto isMultiLineClosure = bodyLine > inLine;
5844-
auto indent = bodyStmts.empty() ? "" : Lexer::getIndentationForLine(
5845-
sourceMgr, bodyLoc);
5846-
5847-
SmallString<16> parameter;
5848-
llvm::raw_svector_ostream parameterOS(parameter);
5849-
5850-
parameterOS << "(";
5851-
interleave(params->getArray(),
5852-
[&](const ParamDecl *param) {
5853-
parameterOS << param->getNameStr();
5854-
},
5855-
[&] { parameterOS << ", "; });
5856-
parameterOS << ")";
5857-
5858-
// Check if there are any explicit types associated
5859-
// with parameters, if there are, we'll have to add
5860-
// type information to the replacement argument.
5861-
bool explicitTypes = false;
5862-
for (auto *param : params->getArray()) {
5863-
if (param->getTypeLoc().getTypeRepr()) {
5864-
explicitTypes = true;
5865-
break;
5866-
}
5867-
}
5868-
5869-
if (isMultiLineClosure)
5870-
OS << '\n' << indent;
5871-
5872-
// Let's form 'let <name> : [<type>]? = arg' expression.
5873-
OS << "let " << parameterOS.str() << " = arg"
5874-
<< (isMultiLineClosure ? "\n" + indent : "; ");
5875-
5876-
SmallString<64> argName;
5877-
llvm::raw_svector_ostream nameOS(argName);
5878-
if (explicitTypes) {
5879-
nameOS << "(arg: " << argTupleTy->getString() << ")";
5880-
} else {
5881-
nameOS << "(arg)";
5882-
}
5883-
5884-
if (CE->hasSingleExpressionBody()) {
5885-
// Let's see if we need to add result type to the argument/fix-it:
5886-
// - if the there is a result type associated with the closure;
5887-
// - and it's not a void type;
5888-
// - and it hasn't been explicitly written.
5889-
auto resultType = fnType->getResult();
5890-
auto hasResult = [](Type resultType) -> bool {
5891-
return resultType && !resultType->isVoid();
5892-
};
5893-
5894-
auto isValidType = [](Type resultType) -> bool {
5895-
return resultType && !resultType->hasUnresolvedType() &&
5896-
!resultType->hasTypeVariable();
5897-
};
5898-
5899-
// If there an expected result type but it hasn't been explicitly
5900-
// provided, let's add it to the argument.
5901-
if (hasResult(resultType) && !CE->hasExplicitResultType()) {
5902-
nameOS << " -> ";
5903-
if (isValidType(resultType))
5904-
nameOS << resultType->getString();
5905-
else
5906-
nameOS << "<#Result#>";
5907-
}
5908-
5909-
if (auto stmt = bodyStmts.front().get<Stmt *>()) {
5910-
// If the body is a single expression with implicit return.
5911-
if (isa<ReturnStmt>(stmt) && stmt->isImplicit()) {
5912-
// And there is non-void expected result type,
5913-
// because we add 'let' expression to the body
5914-
// we need to make such 'return' explicit.
5915-
if (hasResult(resultType))
5916-
OS << "return ";
5917-
}
5918-
}
5919-
}
5920-
5921-
diag.fixItReplace(params->getSourceRange(), nameOS.str())
5922-
.fixItInsert(bodyLoc, OS.str());
5923-
5924-
return true;
5803+
ClosureParamDestructuringFailure failure(
5804+
expr, CS, fnType, CS.getConstraintLocator(CE));
5805+
return failure.diagnoseAsError();
59255806
}
59265807
}
59275808
}

‎lib/Sema/CSDiagnostics.cpp‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "swift/AST/Pattern.h"
2727
#include "swift/AST/ProtocolConformance.h"
2828
#include "swift/AST/ProtocolConformanceRef.h"
29+
#include "swift/AST/Stmt.h"
2930
#include "swift/AST/Types.h"
3031
#include "swift/Basic/SourceLoc.h"
3132
#include "swift/Parse/Lexer.h"
@@ -2009,3 +2010,126 @@ bool MissingArgumentsFailure::diagnoseTrailingClosure(ClosureExpr *closure) {
20092010

20102011
return true;
20112012
}
2013+
2014+
bool ClosureParamDestructuringFailure::diagnoseAsError() {
2015+
auto *closure = cast<ClosureExpr>(getAnchor());
2016+
auto params = closure->getParameters();
2017+
2018+
// In case of implicit parameters e.g. $0, $1 we
2019+
// can't really provide good fix-it because
2020+
// structure of parameter type itself is unclear.
2021+
for (auto *param : params->getArray()) {
2022+
if (param->isImplicit()) {
2023+
emitDiagnostic(params->getStartLoc(),
2024+
diag::closure_tuple_parameter_destructuring_implicit,
2025+
getParameterType());
2026+
return true;
2027+
}
2028+
}
2029+
2030+
auto diag = emitDiagnostic(params->getStartLoc(),
2031+
diag::closure_tuple_parameter_destructuring,
2032+
getParameterType());
2033+
2034+
auto *closureBody = closure->getBody();
2035+
if (!closureBody)
2036+
return true;
2037+
2038+
auto &sourceMgr = getASTContext().SourceMgr;
2039+
auto bodyStmts = closureBody->getElements();
2040+
2041+
SourceLoc bodyLoc;
2042+
// If the body is empty let's put the cursor
2043+
// right after "in", otherwise make it start
2044+
// location of the first statement in the body.
2045+
if (bodyStmts.empty())
2046+
bodyLoc = Lexer::getLocForEndOfToken(sourceMgr, closure->getInLoc());
2047+
else
2048+
bodyLoc = bodyStmts.front().getStartLoc();
2049+
2050+
SmallString<64> fixIt;
2051+
llvm::raw_svector_ostream OS(fixIt);
2052+
2053+
// If this is multi-line closure we'd have to insert new lines
2054+
// in the suggested 'let' to keep the structure of the code intact,
2055+
// otherwise just use ';' to keep everything on the same line.
2056+
auto inLine = sourceMgr.getLineNumber(closure->getInLoc());
2057+
auto bodyLine = sourceMgr.getLineNumber(bodyLoc);
2058+
auto isMultiLineClosure = bodyLine > inLine;
2059+
auto indent =
2060+
bodyStmts.empty() ? "" : Lexer::getIndentationForLine(sourceMgr, bodyLoc);
2061+
2062+
SmallString<16> parameter;
2063+
llvm::raw_svector_ostream parameterOS(parameter);
2064+
2065+
parameterOS << "(";
2066+
interleave(
2067+
params->getArray(),
2068+
[&](const ParamDecl *param) { parameterOS << param->getNameStr(); },
2069+
[&] { parameterOS << ", "; });
2070+
parameterOS << ")";
2071+
2072+
// Check if there are any explicit types associated
2073+
// with parameters, if there are, we'll have to add
2074+
// type information to the replacement argument.
2075+
bool explicitTypes =
2076+
llvm::any_of(params->getArray(), [](const ParamDecl *param) {
2077+
return param->getTypeLoc().getTypeRepr();
2078+
});
2079+
2080+
if (isMultiLineClosure)
2081+
OS << '\n' << indent;
2082+
2083+
// Let's form 'let <name> : [<type>]? = arg' expression.
2084+
OS << "let " << parameterOS.str() << " = arg"
2085+
<< (isMultiLineClosure ? "\n" + indent : "; ");
2086+
2087+
SmallString<64> argName;
2088+
llvm::raw_svector_ostream nameOS(argName);
2089+
if (explicitTypes) {
2090+
nameOS << "(arg: " << getParameterType()->getString() << ")";
2091+
} else {
2092+
nameOS << "(arg)";
2093+
}
2094+
2095+
if (closure->hasSingleExpressionBody()) {
2096+
// Let's see if we need to add result type to the argument/fix-it:
2097+
// - if the there is a result type associated with the closure;
2098+
// - and it's not a void type;
2099+
// - and it hasn't been explicitly written.
2100+
auto resultType = resolveType(ContextualType->getResult());
2101+
auto hasResult = [](Type resultType) -> bool {
2102+
return resultType && !resultType->isVoid();
2103+
};
2104+
2105+
auto isValidType = [](Type resultType) -> bool {
2106+
return resultType && !resultType->hasUnresolvedType() &&
2107+
!resultType->hasTypeVariable();
2108+
};
2109+
2110+
// If there an expected result type but it hasn't been explicitly
2111+
// provided, let's add it to the argument.
2112+
if (hasResult(resultType) && !closure->hasExplicitResultType()) {
2113+
nameOS << " -> ";
2114+
if (isValidType(resultType))
2115+
nameOS << resultType->getString();
2116+
else
2117+
nameOS << "<#Result#>";
2118+
}
2119+
2120+
if (auto stmt = bodyStmts.front().get<Stmt *>()) {
2121+
// If the body is a single expression with implicit return.
2122+
if (isa<ReturnStmt>(stmt) && stmt->isImplicit()) {
2123+
// And there is non-void expected result type,
2124+
// because we add 'let' expression to the body
2125+
// we need to make such 'return' explicit.
2126+
if (hasResult(resultType))
2127+
OS << "return ";
2128+
}
2129+
}
2130+
}
2131+
2132+
diag.fixItReplace(params->getSourceRange(), nameOS.str())
2133+
.fixItInsert(bodyLoc, OS.str());
2134+
return true;
2135+
}

‎lib/Sema/CSDiagnostics.h‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,30 @@ class MissingArgumentsFailure final : public FailureDiagnostic {
899899
bool diagnoseTrailingClosure(ClosureExpr *closure);
900900
};
901901

902+
/// Diagnose an attempt to destructure a single tuple closure parameter
903+
/// into a multiple (possibly anonymous) arguments e.g.
904+
///
905+
/// ```swift
906+
/// let _: ((Int, Int)) -> Void = { $0 + $1 }
907+
/// ```
908+
class ClosureParamDestructuringFailure final : public FailureDiagnostic {
909+
FunctionType *ContextualType;
910+
911+
public:
912+
ClosureParamDestructuringFailure(Expr *root, ConstraintSystem &cs,
913+
FunctionType *contextualType,
914+
ConstraintLocator *locator)
915+
: FailureDiagnostic(root, cs, locator), ContextualType(contextualType) {}
916+
917+
bool diagnoseAsError() override;
918+
919+
private:
920+
Type getParameterType() const {
921+
const auto &param = ContextualType->getParams().front();
922+
return resolveType(param.getPlainType());
923+
}
924+
};
925+
902926
} // end namespace constraints
903927
} // end namespace swift
904928

‎lib/Sema/CSFix.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,20 @@ AllowInvalidInitRef::create(RefKind kind, ConstraintSystem &cs, Type baseTy,
343343
cs, kind, baseTy, init, isStaticallyDerived, baseRange, locator);
344344
}
345345

346+
bool AllowClosureParamDestructuring::diagnose(Expr *root, bool asNote) const {
347+
ClosureParamDestructuringFailure failure(root, getConstraintSystem(),
348+
ContextualType, getLocator());
349+
return failure.diagnose(asNote);
350+
}
351+
352+
AllowClosureParamDestructuring *
353+
AllowClosureParamDestructuring::create(ConstraintSystem &cs,
354+
FunctionType *contextualType,
355+
ConstraintLocator *locator) {
356+
return new (cs.getAllocator())
357+
AllowClosureParamDestructuring(cs, contextualType, locator);
358+
}
359+
346360
bool AddMissingArguments::diagnose(Expr *root, bool asNote) const {
347361
MissingArgumentsFailure failure(root, getConstraintSystem(), Fn,
348362
NumSynthesized, getLocator());

‎lib/Sema/CSFix.h‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ enum class FixKind : uint8_t {
105105
/// fix this issue by pretending that member exists and matches
106106
/// given arguments/result types exactly.
107107
DefineMemberBasedOnUse,
108-
108+
109109
/// Allow access to type member on instance or instance member on type
110110
AllowTypeOrInstanceMember,
111111

@@ -126,6 +126,9 @@ enum class FixKind : uint8_t {
126126
/// If there are fewer arguments than parameters, let's fix that up
127127
/// by adding new arguments to the list represented as type variables.
128128
AddMissingArguments,
129+
130+
/// Allow single tuple closure parameter destructuring into N arguments.
131+
AllowClosureParameterDestructuring,
129132
};
130133

131134
class ConstraintFix {
@@ -618,6 +621,27 @@ class AllowInvalidInitRef final : public ConstraintFix {
618621
ConstraintLocator *locator);
619622
};
620623

624+
class AllowClosureParamDestructuring final : public ConstraintFix {
625+
FunctionType *ContextualType;
626+
627+
AllowClosureParamDestructuring(ConstraintSystem &cs,
628+
FunctionType *contextualType,
629+
ConstraintLocator *locator)
630+
: ConstraintFix(cs, FixKind::AllowClosureParameterDestructuring, locator),
631+
ContextualType(contextualType) {}
632+
633+
public:
634+
std::string getName() const override {
635+
return "allow closure parameter destructuring";
636+
}
637+
638+
bool diagnose(Expr *root, bool asNote = false) const override;
639+
640+
static AllowClosureParamDestructuring *create(ConstraintSystem &cs,
641+
FunctionType *contextualType,
642+
ConstraintLocator *locator);
643+
};
644+
621645
class AddMissingArguments final
622646
: public ConstraintFix,
623647
private llvm::TrailingObjects<AddMissingArguments,

0 commit comments

Comments
 (0)