From fc9343fe8a30b9e8d7c39afce79cc4b2afb61042 Mon Sep 17 00:00:00 2001 From: Suyash Srijan Date: Mon, 10 Feb 2020 23:12:25 +0000 Subject: [PATCH 1/3] [CSDiagnostics] Offer a fix-it to insert a return type when returning from a void function --- include/swift/AST/DiagnosticsSema.def | 2 ++ lib/Sema/CSDiagnostics.cpp | 15 ++++++++ test/Constraints/diagnostics.swift | 50 +++++++++++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index c42896b356477..610c9c897fd28 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -703,6 +703,8 @@ NOTE(selector_construction_suppress_warning,none, ERROR(cannot_return_value_from_void_func,none, "unexpected non-void return value in void function", ()) +NOTE(add_return_type_note,none, + "did you mean to add a return type?", ()) //------------------------------------------------------------------------------ // MARK: Name Binding diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index cfecbcb68ec2a..6e8866b673cf4 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -4792,6 +4792,21 @@ bool InvalidUseOfAddressOf::diagnoseAsError() { bool ExtraneousReturnFailure::diagnoseAsError() { auto *anchor = getAnchor(); emitDiagnostic(anchor->getLoc(), diag::cannot_return_value_from_void_func); + if (auto FD = dyn_cast(getDC())) { + // We only want to emit the note + fix-it if the function does not + // have an explicit return type. The reason we also need to check + // whether the parameter list has a valid loc is to guard against + // cases like like 'var foo: () { return 1 }' as here that loc will + // be invalid. + if (FD->getBodyResultTypeLoc().getLoc().isInvalid() && + FD->getParameters()->getStartLoc().isValid()) { + auto fixItLoc = Lexer::getLocForEndOfToken( + getASTContext().SourceMgr, FD->getParameters()->getEndLoc()); + emitDiagnostic(anchor->getLoc(), diag::add_return_type_note) + .fixItInsert(fixItLoc, " -> <# Return Type #>"); + } + } + return true; } diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift index 043775a16017c..a5a638fff23a4 100644 --- a/test/Constraints/diagnostics.swift +++ b/test/Constraints/diagnostics.swift @@ -1315,3 +1315,53 @@ takesGenericFunction(true) // expected-error {{cannot convert value of type 'Boo func takesTuple(_ x: ([T], [T])) {} // expected-note {{in call to function 'takesTuple'}} takesTuple(true) // expected-error {{cannot convert value of type 'Bool' to expected argument type '([T], [T])'}} // expected-error@-1 {{generic parameter 'T' could not be inferred}} + +// Void function returns non-void result fix-it + +func voidFunc() { + return 1 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}}{{15-15= -> <# Return Type #>}} +} + +func voidFuncWithArgs(arg1: Int) { + return 1 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}}{{33-33= -> <# Return Type #>}} +} + +func voidFuncWithCondFlow() { + if Bool.random() { + return 1 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}}{{28-28= -> <# Return Type #>}} + } else { + return 2 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}}{{28-28= -> <# Return Type #>}} + } +} + +func voidFuncWithNestedVoidFunc() { + func nestedVoidFunc() { + return 1 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}}{{24-24= -> <# Return Type #>}} + } +} + +// Special cases: These should not offer a note + fix-it + +func voidFuncExplicitType() -> Void { + return 1 // expected-error {{unexpected non-void return value in void function}} +} + +class ClassWithDeinit { + deinit { + return 0 // expected-error {{unexpected non-void return value in void function}} + } +} + +class ClassWithVoidProp { + var propertyWithVoidType: () { return 5 } // expected-error {{unexpected non-void return value in void function}} +} From 44ad1d3e2776773427feec00f7ac37e92c3f60f1 Mon Sep 17 00:00:00 2001 From: Suyash Srijan Date: Tue, 11 Feb 2020 00:18:20 +0000 Subject: [PATCH 2/3] [CSDiagnostics] Make sure the function name is not empty The function name will be empty in some cases, for example for property setters. In cases where the function name is empty, skip the note and fix-it. --- lib/Sema/CSDiagnostics.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index 6e8866b673cf4..f853f146d9919 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -4797,13 +4797,15 @@ bool ExtraneousReturnFailure::diagnoseAsError() { // have an explicit return type. The reason we also need to check // whether the parameter list has a valid loc is to guard against // cases like like 'var foo: () { return 1 }' as here that loc will - // be invalid. + // be invalid. We also need to check that the name is not empty, + // because certain decls will have empty name (like setters). if (FD->getBodyResultTypeLoc().getLoc().isInvalid() && - FD->getParameters()->getStartLoc().isValid()) { + FD->getParameters()->getStartLoc().isValid() && + !FD->getName().empty()) { auto fixItLoc = Lexer::getLocForEndOfToken( getASTContext().SourceMgr, FD->getParameters()->getEndLoc()); emitDiagnostic(anchor->getLoc(), diag::add_return_type_note) - .fixItInsert(fixItLoc, " -> <# Return Type #>"); + .fixItInsert(fixItLoc, " -> <#Return Type#>"); } } From ae3292016c57bf9dbde72d042e723e2f9154fc12 Mon Sep 17 00:00:00 2001 From: Suyash Srijan Date: Tue, 11 Feb 2020 07:58:56 +0000 Subject: [PATCH 3/3] [Test] Update existing diagnostics --- test/Constraints/diagnostics.swift | 46 +++++++++++++++++++++--------- test/Constraints/overload.swift | 4 ++- test/Misc/misc_diagnostics.swift | 17 +++++++---- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/test/Constraints/diagnostics.swift b/test/Constraints/diagnostics.swift index a5a638fff23a4..b9ffb4a484c80 100644 --- a/test/Constraints/diagnostics.swift +++ b/test/Constraints/diagnostics.swift @@ -127,7 +127,9 @@ protocol Shoes { // Here the opaque value has type (metatype_type (archetype_type ... )) func f(_ x: Shoes, asType t: Shoes.Type) { - return t.select(x) // expected-error{{unexpected non-void return value in void function}} + return t.select(x) + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } precedencegroup Starry { @@ -184,7 +186,9 @@ func perform() {} // expected-error {{generic parameter 'T' is not used in f // Error Message QOI - wrong return type in an overload func recArea(_ h: Int, w : Int) { - return h * w // expected-error {{unexpected non-void return value in void function}} + return h * w + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } // QoI: Error In Ternary Condition is Wrong @@ -752,7 +756,9 @@ func segfault23433271(_ a : UnsafeMutableRawPointer) { // Poor diagnostic due to contextual constraint func r23272739(_ contentType: String) { let actualAcceptableContentTypes: Set = [] - return actualAcceptableContentTypes.contains(contentType) // expected-error {{unexpected non-void return value in void function}} + return actualAcceptableContentTypes.contains(contentType) + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } // QoI: Strings in Swift cannot be indexed directly with integer offsets @@ -863,8 +869,8 @@ class Foo23752537 { extension Foo23752537 { func isEquivalent(other: Foo23752537) { // TODO: QoI: Nonsensical "binary operator '&&' cannot be applied to two 'Bool' operands" - // expected-error @+1 {{unexpected non-void return value in void function}} - return (self.title != other.title && self.message != other.message) + // expected-error@+1 {{unexpected non-void return value in void function}} + return (self.title != other.title && self.message != other.message) // expected-note {{did you mean to add a return type?}} } } @@ -890,7 +896,9 @@ func f23213302() { // QoI: Return of call to overloaded function in void-return context func rdar24202058(a : Int) { - return a <= 480 // expected-error {{unexpected non-void return value in void function}} + return a <= 480 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } // SR-1752: Warning about unused result with ternary operator @@ -953,7 +961,9 @@ r27212391(a: 1, 3, x: 5) // expected-error {{argument 'x' must precede unname // SR-1255 func foo1255_1() { - return true || false // expected-error {{unexpected non-void return value in void function}} + return true || false + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } func foo1255_2() -> Int { return true || false // expected-error {{cannot convert return expression of type 'Bool' to return type 'Int'}} @@ -1150,6 +1160,7 @@ func sr5045() { let doubles: [Double] = [1, 2, 3] return doubles.reduce(0, +) // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } // rdar://problem/32934129 - QoI: misleading diagnostic @@ -1165,7 +1176,9 @@ class L_32934129 { func length() -> Int { func inner(_ list: L_32934129?, _ count: Int) { - guard let list = list else { return count } // expected-error {{unexpected non-void return value in void function}} + guard let list = list else { return count } + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} return inner(list.next, count + 1) } @@ -1321,24 +1334,24 @@ takesTuple(true) // expected-error {{cannot convert value of type 'Bool' to expe func voidFunc() { return 1 // expected-error@-1 {{unexpected non-void return value in void function}} - // expected-note@-2 {{did you mean to add a return type?}}{{15-15= -> <# Return Type #>}} + // expected-note@-2 {{did you mean to add a return type?}}{{16-16= -> <#Return Type#>}} } func voidFuncWithArgs(arg1: Int) { return 1 // expected-error@-1 {{unexpected non-void return value in void function}} - // expected-note@-2 {{did you mean to add a return type?}}{{33-33= -> <# Return Type #>}} + // expected-note@-2 {{did you mean to add a return type?}}{{33-33= -> <#Return Type#>}} } func voidFuncWithCondFlow() { if Bool.random() { return 1 // expected-error@-1 {{unexpected non-void return value in void function}} - // expected-note@-2 {{did you mean to add a return type?}}{{28-28= -> <# Return Type #>}} + // expected-note@-2 {{did you mean to add a return type?}}{{28-28= -> <#Return Type#>}} } else { return 2 // expected-error@-1 {{unexpected non-void return value in void function}} - // expected-note@-2 {{did you mean to add a return type?}}{{28-28= -> <# Return Type #>}} + // expected-note@-2 {{did you mean to add a return type?}}{{28-28= -> <#Return Type#>}} } } @@ -1346,7 +1359,7 @@ func voidFuncWithNestedVoidFunc() { func nestedVoidFunc() { return 1 // expected-error@-1 {{unexpected non-void return value in void function}} - // expected-note@-2 {{did you mean to add a return type?}}{{24-24= -> <# Return Type #>}} + // expected-note@-2 {{did you mean to add a return type?}}{{24-24= -> <#Return Type#>}} } } @@ -1365,3 +1378,10 @@ class ClassWithDeinit { class ClassWithVoidProp { var propertyWithVoidType: () { return 5 } // expected-error {{unexpected non-void return value in void function}} } + +class ClassWithPropContainingSetter { + var propWithSetter: Int { + get { return 0 } + set { return 1 } // expected-error {{unexpected non-void return value in void function}} + } +} diff --git a/test/Constraints/overload.swift b/test/Constraints/overload.swift index 5838918cba3be..58d7ed89d9a06 100644 --- a/test/Constraints/overload.swift +++ b/test/Constraints/overload.swift @@ -135,7 +135,9 @@ func test_contextual_result_1() { } func test_contextual_result_2() { - return overloaded_identity(1) // expected-error {{unexpected non-void return value in void function}} + return overloaded_identity(1) + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } // rdar://problem/24128153 diff --git a/test/Misc/misc_diagnostics.swift b/test/Misc/misc_diagnostics.swift index c7d3e72f94738..4449414e68fa5 100644 --- a/test/Misc/misc_diagnostics.swift +++ b/test/Misc/misc_diagnostics.swift @@ -56,7 +56,9 @@ class A { } } -func retV() { return true } // expected-error {{unexpected non-void return value in void function}} +func retV() { return true } +// expected-error@-1 {{unexpected non-void return value in void function}} +// expected-note@-2 {{did you mean to add a return type?}} func retAI() -> Int { let a = [""] @@ -65,7 +67,9 @@ func retAI() -> Int { } func bad_return1() { - return 42 // expected-error {{unexpected non-void return value in void function}} + return 42 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } func bad_return2() -> (Int, Int) { @@ -74,7 +78,9 @@ func bad_return2() -> (Int, Int) { // QoI: Diagnostics for trying to return values from void functions func bad_return3(lhs: Int, rhs: Int) { - return lhs != 0 // expected-error {{unexpected non-void return value in void function}} + return lhs != 0 + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } class MyBadReturnClass { @@ -82,7 +88,9 @@ class MyBadReturnClass { } func ==(lhs:MyBadReturnClass, rhs:MyBadReturnClass) { - return MyBadReturnClass.intProperty == MyBadReturnClass.intProperty // expected-error{{unexpected non-void return value in void function}} + return MyBadReturnClass.intProperty == MyBadReturnClass.intProperty + // expected-error@-1 {{unexpected non-void return value in void function}} + // expected-note@-2 {{did you mean to add a return type?}} } @@ -156,4 +164,3 @@ func tuple_splat2(_ q : (a : Int, b : Int)) { func is_foreign(a: AnyObject) -> Bool { return a is CGColor // expected-warning {{'is' test is always true because 'CGColor' is a Core Foundation type}} } -