From e2e2d505ad19391af099019f0149763b3c1ae73f Mon Sep 17 00:00:00 2001 From: Suyash Srijan Date: Thu, 18 Apr 2019 23:23:52 +0100 Subject: [PATCH] [Diagnostics] Update diagnostics for T! to Any (#23617) Add a tailored diagnostic for the case where implicitly unwrapped optional is coerced to `Any` --- include/swift/AST/DiagnosticsSema.def | 9 +++ lib/Sema/MiscDiagnostics.cpp | 78 ++++++++++++------- ...unintended_optional_behavior_swift_5.swift | 60 +++++++------- 3 files changed, 92 insertions(+), 55 deletions(-) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index 5c78748ebcc89..bb743f7cfb35e 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -3031,6 +3031,15 @@ WARNING(optional_pattern_match_promotion,none, (Type, Type)) WARNING(optional_to_any_coercion,none, "expression implicitly coerced from %0 to %1", (Type, Type)) +WARNING(iuo_to_any_coercion,none, + "coercion of implicitly unwrappable value of type %0 to %1 does not " + "unwrap optional", (Type, Type)) +NOTE(iuo_to_any_coercion_note,none, + "implicitly unwrapped %0 %1 declared here", + (DescriptiveDeclKind, DeclName)) +NOTE(iuo_to_any_coercion_note_func_result,none, + "%0 %1 with implicitly unwrapped result type is declared here", + (DescriptiveDeclKind, DeclName)) NOTE(default_optional_to_any,none, "provide a default value to avoid this warning", ()) NOTE(force_optional_to_any,none, diff --git a/lib/Sema/MiscDiagnostics.cpp b/lib/Sema/MiscDiagnostics.cpp index 57d54ba18fd4c..47986f772bd1d 100644 --- a/lib/Sema/MiscDiagnostics.cpp +++ b/lib/Sema/MiscDiagnostics.cpp @@ -3580,36 +3580,39 @@ static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E, } static bool hasImplicitlyUnwrappedResult(Expr *E) { - auto getDeclForExpr = [&](Expr *E) -> ValueDecl * { - if (auto *call = dyn_cast(E)) - E = call->getDirectCallee(); + auto *decl = getDeclForImplicitlyUnwrappedExpr(E); - if (auto *subscript = dyn_cast(E)) { - if (subscript->hasDecl()) - return subscript->getDecl().getDecl(); - - return nullptr; - } - - if (auto *memberRef = dyn_cast(E)) - return memberRef->getMember().getDecl(); - if (auto *declRef = dyn_cast(E)) - return declRef->getDecl(); - if (auto *apply = dyn_cast(E)) - return apply->getCalledValue(); + return decl + && decl->getAttrs().hasAttribute(); + } - return nullptr; - }; + static ValueDecl *getDeclForImplicitlyUnwrappedExpr(Expr *E) { + E = E->getValueProvidingExpr(); // Look through implicit conversions like loads, derived-to-base // conversion, etc. - if (auto *ICE = dyn_cast(E)) + if (auto *ICE = dyn_cast(E)) { E = ICE->getSubExpr(); + } - auto *decl = getDeclForExpr(E); + if (auto *subscript = dyn_cast(E)) { + if (subscript->hasDecl()) + return subscript->getDecl().getDecl(); + return nullptr; + } - return decl - && decl->getAttrs().hasAttribute(); + if (auto *memberRef = dyn_cast(E)) + return memberRef->getMember().getDecl(); + + if (auto *declRef = dyn_cast(E)) + return declRef->getDecl(); + + if (auto *apply = dyn_cast(E)) { + auto *decl = apply->getCalledValue(); + if (decl && isa(decl)) + return decl; + } + return nullptr; } void visitErasureExpr(ErasureExpr *E, OptionalToAnyCoercion coercion) { @@ -3640,15 +3643,32 @@ static void diagnoseUnintendedOptionalBehavior(TypeChecker &TC, const Expr *E, size_t optionalityDifference = 0; if (!isOptionalToAnyCoercion(srcType, destType, optionalityDifference)) return; - - TC.diagnose(subExpr->getStartLoc(), diag::optional_to_any_coercion, - /* from */ srcType, /* to */ destType) - .highlight(subExpr->getSourceRange()); - + + // If we're implicitly unwrapping from IUO to Any then emit a custom + // diagnostic + if (hasImplicitlyUnwrappedResult(subExpr)) { + if (auto decl = getDeclForImplicitlyUnwrappedExpr(subExpr)) { + TC.diagnose(subExpr->getStartLoc(), diag::iuo_to_any_coercion, + /* from */ srcType, /* to */ destType) + .highlight(subExpr->getSourceRange()); + + auto noteDiag = isa(decl) + ? diag::iuo_to_any_coercion_note_func_result + : diag::iuo_to_any_coercion_note; + + TC.diagnose(decl->getLoc(), noteDiag, decl->getDescriptiveKind(), + decl->getFullName()); + } + } else { + TC.diagnose(subExpr->getStartLoc(), diag::optional_to_any_coercion, + /* from */ srcType, /* to */ destType) + .highlight(subExpr->getSourceRange()); + } + if (optionalityDifference == 1) { TC.diagnose(subExpr->getLoc(), diag::default_optional_to_any) - .highlight(subExpr->getSourceRange()) - .fixItInsertAfter(subExpr->getEndLoc(), " ?? <#default value#>"); + .highlight(subExpr->getSourceRange()) + .fixItInsertAfter(subExpr->getEndLoc(), " ?? <#default value#>"); } SmallString<4> forceUnwrapString; diff --git a/test/Sema/diag_unintended_optional_behavior_swift_5.swift b/test/Sema/diag_unintended_optional_behavior_swift_5.swift index 901bf0083781e..a87f6d6cce674 100644 --- a/test/Sema/diag_unintended_optional_behavior_swift_5.swift +++ b/test/Sema/diag_unintended_optional_behavior_swift_5.swift @@ -9,60 +9,60 @@ func takeAny(_ left: Any, _ right: Any) -> Int? { func takesOptionalAny(_: Any?, _: Any?) {} class C { - var a: Int! - var b: Any?! - func returningIUO() -> Int! { return a } - func returningAny() -> Any { return a } // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}} + var a: Int! // expected-note 2{{implicitly unwrapped property 'a' declared here}} + var b: Any?! // expected-note {{implicitly unwrapped property 'b' declared here}} + func returningIUO() -> Int! { return a } // expected-note {{instance method 'returningIUO()' with implicitly unwrapped result type is declared here}} + func returningAny() -> Any { return a } // expected-warning {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{40-40= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{40-40=!}} // expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{40-40= as Any}} - subscript(i: Int) -> Int! { return 0 } - subscript(i: Float) -> Any! { return 0 } + subscript(i: Int) -> Int! { return 0 } // expected-note {{implicitly unwrapped subscript 'subscript(_:)' declared here}} + subscript(i: Float) -> Any! { return 0 } // expected-note {{implicitly unwrapped subscript 'subscript(_:)' declared here}} } class D { - init!() {} + init!() {} // expected-note 2{{implicitly unwrapped initializer 'init()' declared here}} } -func returningIUO() -> Int! { return 1 } +func returningIUO() -> Int! { return 1 } // expected-note {{global function 'returningIUO()' with implicitly unwrapped result type is declared here}} -func warnIUOToAnyCoercion(_ a: Int!, _ b: Any?!) { - _ = takeAny(a, b) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}} +func warnIUOToAnyCoercion(_ a: Int!, _ b: Any?!) { // expected-note {{implicitly unwrapped parameter 'a' declared here}} // expected-note {{implicitly unwrapped parameter 'b' declared here}} + _ = takeAny(a, b) // expected-warning {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{16-16= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{16-16=!}} // expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{16-16= as Any}} - // expected-warning@-4 {{expression implicitly coerced from 'Any??' to 'Any'}} + // expected-warning@-4 {{coercion of implicitly unwrappable value of type 'Any??' to 'Any' does not unwrap optional}} // expected-note@-5 {{force-unwrap the value to avoid this warning}}{{19-19=!!}} // expected-note@-6 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{19-19= as Any}} - _ = takeAny(returningIUO(), C().returningIUO()) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}} + _ = takeAny(returningIUO(), C().returningIUO()) // expected-warning {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{29-29= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{29-29=!}} // expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{29-29= as Any}} - // expected-warning@-4 {{expression implicitly coerced from 'Int?' to 'Any'}} + // expected-warning@-4 {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-5 {{provide a default value to avoid this warning}}{{49-49= ?? <#default value#>}} // expected-note@-6 {{force-unwrap the value to avoid this warning}}{{49-49=!}} // expected-note@-7 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{49-49= as Any}} - _ = takeAny(C().a, C().b) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}} + _ = takeAny(C().a, C().b) // expected-warning {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{20-20= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{20-20=!}} // expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{20-20= as Any}} - // expected-warning@-4 {{expression implicitly coerced from 'Any??' to 'Any'}} + // expected-warning@-4 {{coercion of implicitly unwrappable value of type 'Any??' to 'Any' does not unwrap optional}} // expected-note@-5 {{force-unwrap the value to avoid this warning}}{{27-27=!!}} // expected-note@-6 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{27-27= as Any}} - _ = takeAny(C()[0], C()[1.0]) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}} + _ = takeAny(C()[0], C()[1.0]) // expected-warning {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{21-21= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{21-21=!}} // expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{21-21= as Any}} - // expected-warning@-4 {{expression implicitly coerced from 'Any?' to 'Any'}} + // expected-warning@-4 {{coercion of implicitly unwrappable value of type 'Any?' to 'Any' does not unwrap optional}} // expected-note@-5 {{provide a default value to avoid this warning}}{{31-31= ?? <#default value#>}} // expected-note@-6 {{force-unwrap the value to avoid this warning}}{{31-31=!}} // expected-note@-7 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{31-31= as Any}} - _ = takeAny(D(), D()) // expected-warning {{expression implicitly coerced from 'D?' to 'Any'}} + _ = takeAny(D(), D()) // expected-warning {{coercion of implicitly unwrappable value of type 'D?' to 'Any' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{18-18= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{18-18=!}} // expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{18-18= as Any}} - // expected-warning@-4 {{expression implicitly coerced from 'D?' to 'Any'}} + // expected-warning@-4 {{coercion of implicitly unwrappable value of type 'D?' to 'Any' does not unwrap optional}} // expected-note@-5 {{provide a default value to avoid this warning}}{{23-23= ?? <#default value#>}} // expected-note@-6 {{force-unwrap the value to avoid this warning}}{{23-23=!}} // expected-note@-7 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{23-23= as Any}} @@ -70,8 +70,8 @@ func warnIUOToAnyCoercion(_ a: Int!, _ b: Any?!) { _ = takeAny(a as Any, b as Any) } -func warnIUOToOptionalAnyCoercion(_ a: Int!, _ b: Any?!, _ c: Int??!, _ d: Any???!) { - takesOptionalAny(a, b) // expected-warning {{expression implicitly coerced from 'Any??' to 'Any?'}} +func warnIUOToOptionalAnyCoercion(_ a: Int!, _ b: Any?!, _ c: Int??!, _ d: Any???!) { // expected-note {{implicitly unwrapped parameter 'b' declared here}} // expected-note {{implicitly unwrapped parameter 'c' declared here}} // expected-note {{implicitly unwrapped parameter 'd' declared here}} + takesOptionalAny(a, b) // expected-warning {{coercion of implicitly unwrappable value of type 'Any??' to 'Any?' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{24-24= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{24-24=!}} // expected-note@-3 {{explicitly cast to 'Any?' with 'as Any?' to silence this warning}}{{24-24= as Any?}} @@ -80,10 +80,10 @@ func warnIUOToOptionalAnyCoercion(_ a: Int!, _ b: Any?!, _ c: Int??!, _ d: Any?? takesOptionalAny(a, b!) takesOptionalAny(a, b as Any?) - takesOptionalAny(c, d) // expected-warning {{expression implicitly coerced from 'Int???' to 'Any?'}} + takesOptionalAny(c, d) // expected-warning {{coercion of implicitly unwrappable value of type 'Int???' to 'Any?' does not unwrap optional}} // expected-note@-1 {{force-unwrap the value to avoid this warning}}{{21-21=!!}} // expected-note@-2 {{explicitly cast to 'Any?' with 'as Any?' to silence this warning}}{{21-21= as Any?}} - // expected-warning@-3 {{expression implicitly coerced from 'Any????' to 'Any?'}} + // expected-warning@-3 {{coercion of implicitly unwrappable value of type 'Any????' to 'Any?' does not unwrap optional}} // expected-note@-4 {{force-unwrap the value to avoid this warning}}{{24-24=!!!}} // expected-note@-5 {{explicitly cast to 'Any?' with 'as Any?' to silence this warning}}{{24-24= as Any?}} @@ -93,13 +93,21 @@ func warnIUOToOptionalAnyCoercion(_ a: Int!, _ b: Any?!, _ c: Int??!, _ d: Any?? func takesCollectionOfAny(_ a: [Any], _ d: [String : Any]) {} -func warnCollectionOfIUOToAnyCoercion(_ a: Int!) { - takesCollectionOfAny([a], ["test" : a]) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}} +func warnCollectionOfIUOToAnyCoercion(_ a: Int!) { // expected-note 2{{implicitly unwrapped parameter 'a' declared here}} + takesCollectionOfAny([a], ["test" : a]) // expected-warning {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-1 {{provide a default value to avoid this warning}}{{26-26= ?? <#default value#>}} // expected-note@-2 {{force-unwrap the value to avoid this warning}}{{26-26=!}} // expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{26-26= as Any}} - // expected-warning@-4 {{expression implicitly coerced from 'Int?' to 'Any'}} + // expected-warning@-4 {{coercion of implicitly unwrappable value of type 'Int?' to 'Any' does not unwrap optional}} // expected-note@-5 {{provide a default value to avoid this warning}}{{40-40= ?? <#default value#>}} // expected-note@-6 {{force-unwrap the value to avoid this warning}}{{40-40=!}} // expected-note@-7 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}{{40-40= as Any}} } + +func takesAny_sr10199(_ x: Any) {} + +let fn_sr10199: (() -> Int?)! = { return nil } +takesAny_sr10199(fn_sr10199()) // expected-warning {{expression implicitly coerced from 'Int?' to 'Any'}} +// expected-note@-1 {{provide a default value to avoid this warning}} +// expected-note@-2 {{force-unwrap the value to avoid this warning}} +// expected-note@-3 {{explicitly cast to 'Any' with 'as Any' to silence this warning}}