Skip to content

Commit 1158362

Browse files
committed
[Sema] Improve Optional-to-Any diagnostics for collections
When implicitly coercing a collection of optional elements to a collection of less-optional `Any` elements, e.g `[Any?]` to `[Any]`, emit a slightly better diagnostic: Expression implicitly coerced from '[Any?]' to '[Any]' and allow for silencing of the diagnostic with an explicit coercion, e.g `as [Any]`.
1 parent 016af6b commit 1158362

File tree

2 files changed

+66
-21
lines changed

2 files changed

+66
-21
lines changed

lib/Sema/MiscDiagnostics.cpp

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3493,6 +3493,18 @@ static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E,
34933493
}
34943494
}
34953495

3496+
/// Returns true iff the collection upcast coercion is an Optional-to-Any
3497+
/// coercion.
3498+
bool isOptionalToAnyCoercion(CollectionUpcastConversionExpr::ConversionPair
3499+
conversion) {
3500+
if (!conversion.OrigValue || !conversion.Conversion)
3501+
return false;
3502+
3503+
auto srcType = conversion.OrigValue->getType();
3504+
auto destType = conversion.Conversion->getType();
3505+
return isOptionalToAnyCoercion(srcType, destType);
3506+
}
3507+
34963508
/// Looks through OptionalEvaluationExprs and InjectIntoOptionalExprs to
34973509
/// find a child ErasureExpr, returning nullptr if no such child is found.
34983510
/// Any intermediate OptionalEvaluationExprs will be marked as ignored.
@@ -3577,9 +3589,38 @@ static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E,
35773589
emitSilenceOptionalAnyWarningWithCoercion(subExpr, destType);
35783590
}
35793591

3592+
void visitCollectionUpcastExpr(CollectionUpcastConversionExpr *E,
3593+
OptionalToAnyCoercion coercion) {
3594+
// We only need to consider the valueConversion, as the Key type of a
3595+
// Dictionary cannot be implicitly coerced to Any.
3596+
auto valueConversion = E->getValueConversion();
3597+
3598+
// We're handling the coercion of the entire collection, so we don't need
3599+
// to re-visit a nested ErasureExpr for the value.
3600+
if (auto conversionExpr = valueConversion.Conversion)
3601+
if (auto *erasureExpr =
3602+
findErasureExprThroughOptionalInjections(conversionExpr))
3603+
IgnoredExprs.insert(erasureExpr);
3604+
3605+
if (coercion.shouldSuppressDiagnostic() ||
3606+
!isOptionalToAnyCoercion(valueConversion))
3607+
return;
3608+
3609+
auto subExpr = E->getSubExpr();
3610+
3611+
TC.diagnose(subExpr->getStartLoc(), diag::optional_to_any_coercion,
3612+
/* from */ subExpr->getType(), /* to */ E->getType())
3613+
.highlight(subExpr->getSourceRange());
3614+
3615+
emitSilenceOptionalAnyWarningWithCoercion(subExpr, E->getType());
3616+
}
3617+
35803618
void visitPossibleOptionalToAnyExpr(Expr *E,
35813619
OptionalToAnyCoercion coercion) {
3582-
if (auto *erasureExpr = dyn_cast<ErasureExpr>(E)) {
3620+
if (auto *upcastExpr =
3621+
dyn_cast<CollectionUpcastConversionExpr>(E)) {
3622+
visitCollectionUpcastExpr(upcastExpr, coercion);
3623+
} else if (auto *erasureExpr = dyn_cast<ErasureExpr>(E)) {
35833624
visitErasureExpr(erasureExpr, coercion);
35843625
} else if (auto *optionalEvalExpr = dyn_cast<OptionalEvaluationExpr>(E)) {
35853626
// The ErasureExpr could be nested within optional injections and

test/Sema/diag_unintended_optional_behavior.swift

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -132,27 +132,31 @@ func warnOptionalToIUOAny(_ a: Int?, _ b: Any??, _ c: Int???, _ d: Any????) {
132132
}
133133

134134
func takesCollectionOfAny(_ a: [Any], _ d: [String : Any]) {}
135+
func takesCollectionOfOptionalAny(_ a: [Any?], _ d: [String : Any?]) {}
135136

136-
func warnCollectionOfAny(_ a: [Int?], _ d: [String : Int?]) {
137-
// https://bugs.swift.org/browse/SR-2928 - Collection casts from collections of optionals to collections of Any need custom handling
138-
takesCollectionOfAny(a, d) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}}
139-
// expected-note@-1 {{provide a default value to avoid this warning}}{{25-25= ?? <#default value#>}}
140-
// expected-note@-2 {{force-unwrap the value to avoid this warning}}{{25-25=!}}
141-
// expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{25-25= as Any}}
142-
// expected-warning@-4 {{expression implicitly coerced from 'Int?' to 'Any'}}
143-
// expected-note@-5 {{provide a default value to avoid this warning}}{{28-28= ?? <#default value#>}}
144-
// expected-note@-6 {{force-unwrap the value to avoid this warning}}{{28-28=!}}
145-
// expected-note@-7 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{28-28= as Any}}
146-
147-
// https://bugs.swift.org/browse/SR-2928 - Collection casts from collections of optionals to collections of Any need custom handling
148-
takesCollectionOfAny(a as [Any], d as [String : Any]) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}}
149-
// expected-note@-1 {{provide a default value to avoid this warning}}{{25-25= ?? <#default value#>}}
150-
// expected-note@-2 {{force-unwrap the value to avoid this warning}}{{25-25=!}}
151-
// expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{25-25= as Any}}
152-
// expected-warning@-4 {{expression implicitly coerced from 'Int?' to 'Any'}}
153-
// expected-note@-5 {{provide a default value to avoid this warning}}{{37-37= ?? <#default value#>}}
154-
// expected-note@-6 {{force-unwrap the value to avoid this warning}}{{37-37=!}}
155-
// expected-note@-7 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{37-37= as Any}}
137+
func warnCollectionOfOptionalToAnyCoercion(_ a: [Int?], _ d: [String : Int?]) {
138+
takesCollectionOfAny(a, d) // expected-warning {{expression implicitly coerced from '[Int?]' to '[Any]'}}
139+
// expected-note@-1 {{explicitly cast to '[Any]' with 'as [Any]' to silence this warning}}{{25-25= as [Any]}}
140+
// expected-warning@-2 {{expression implicitly coerced from '[String : Int?]' to '[String : Any]'}}
141+
// expected-note@-3 {{explicitly cast to '[String : Any]' with 'as [String : Any]' to silence this warning}}{{28-28= as [String : Any]}}
142+
143+
takesCollectionOfAny(a as [Any], d as [String : Any])
144+
}
145+
146+
func warnCollectionOfTripleOptionalToAnyCoercion(_ a: [Any???], _ d: [String: Any???]) {
147+
takesCollectionOfAny(a, d) // expected-warning {{expression implicitly coerced from '[Any???]' to '[Any]'}}
148+
// expected-note@-1 {{explicitly cast to '[Any]' with 'as [Any]' to silence this warning}}{{25-25= as [Any]}}
149+
// expected-warning@-2 {{expression implicitly coerced from '[String : Any???]' to '[String : Any]'}}
150+
// expected-note@-3 {{explicitly cast to '[String : Any]' with 'as [String : Any]' to silence this warning}}{{28-28= as [String : Any]}}
151+
152+
takesCollectionOfAny(a as [Any], d as [String : Any])
153+
154+
takesCollectionOfOptionalAny(a, d) // expected-warning {{expression implicitly coerced from '[Any???]' to '[Any?]'}}
155+
// expected-note@-1 {{explicitly cast to '[Any?]' with 'as [Any?]' to silence this warning}}{{33-33= as [Any?]}}
156+
// expected-warning@-2 {{expression implicitly coerced from '[String : Any???]' to '[String : Any?]'}}
157+
// expected-note@-3 {{explicitly cast to '[String : Any?]' with 'as [String : Any?]' to silence this warning}}{{36-36= as [String : Any?]}}
158+
159+
takesCollectionOfOptionalAny(a as [Any?], d as [String : Any?])
156160
}
157161

158162
func warnOptionalInStringInterpolationSegment(_ o : Int?) {

0 commit comments

Comments
 (0)