[DiagnosticQol][SR-14505] Use DeclDescriptive kind in missing return data flow diagnostics (#36952)

* [Diagnostics] Use DeclDescriptiveKind on data flow diagnostics to improve diagnostic message

* [tests] Add regression tests to SILOptimizer/return.swift

* [tests] Adapt other tests to changes of SR-14505

* [Diagnostics] Adapt message for missing return diagnostics, remove article

* [Diagnostics] Adapt message for missing return diagnostics to have a note with fix

* [tests] Adjust tests in validation suit

Author: LucianoPAlmeida

include/swift/AST/DiagnosticsSIL.def

@@ -250,17 +250,25 @@ NOTE(designated_init_c_struct_fix,none,
 
 
 // Control flow diagnostics.
-ERROR(missing_return,none,
-      "missing return in a %select{function|closure}1 expected to return %0",
-      (Type, unsigned))
-ERROR(missing_return_last_expr,none,
-      "missing return in a %select{function|closure}1 expected to return %0; "
-      "did you mean to return the last expression?",
-      (Type, unsigned))
-ERROR(missing_never_call,none,
-      "%select{function|closure}1 with uninhabited return type %0 is missing "
+ERROR(missing_return_closure,none,
+      "missing return in closure expected to return %0",
+      (Type))
+ERROR(missing_never_call_closure,none,
+      "closure with uninhabited return type %0 is missing "
+      "call to another never-returning function on all paths",
+      (Type))
+
+ERROR(missing_return_decl,none,
+      "missing return in %1 expected to return %0",
+      (Type, DescriptiveDeclKind))
+ERROR(missing_never_call_decl,none,
+      "%1 with uninhabited return type %0 is missing "
       "call to another never-returning function on all paths",
-      (Type, unsigned))
+      (Type, DescriptiveDeclKind))
+
+NOTE(missing_return_last_expr_note,none,
+    "did you mean to return the last expression?", ())
+
 ERROR(guard_body_must_not_fallthrough,none,
       "'guard' body must not fall through, consider using a 'return' or 'throw'"
       " to exit the scope", ())

lib/SILOptimizer/Mandatory/DataflowDiagnostics.cpp

@@ -64,22 +64,37 @@ static void diagnoseMissingReturn(const UnreachableInst *UI,
     auto element = BS->getLastElement();
     if (auto expr = element.dyn_cast<Expr *>()) {
       if (expr->getType()->getRValueType()->isEqual(ResTy)) {
-        Context.Diags.diagnose(
-          expr->getStartLoc(),
-          diag::missing_return_last_expr, ResTy,
-          FLoc.isASTNode<ClosureExpr>() ? 1 : 0)
-        .fixItInsert(expr->getStartLoc(), "return ");
+        if (FLoc.isASTNode<ClosureExpr>()) {
+          Context.Diags.diagnose(expr->getStartLoc(),
+                                 diag::missing_return_closure, ResTy);
+        } else {
+          auto *DC = FLoc.getAsDeclContext();
+          assert(DC && DC->getAsDecl() && "not a declaration?");
+          Context.Diags.diagnose(expr->getStartLoc(), diag::missing_return_decl,
+                                 ResTy, DC->getAsDecl()->getDescriptiveKind());
+        }
+        Context.Diags
+            .diagnose(expr->getStartLoc(), diag::missing_return_last_expr_note)
+            .fixItInsert(expr->getStartLoc(), "return ");
+
         return;
       }
     }
   }
-  auto diagID = F->isNoReturnFunction(F->getTypeExpansionContext())
-                    ? diag::missing_never_call
-                    : diag::missing_return;
-  diagnose(Context,
-           L.getEndSourceLoc(),
-           diagID, ResTy,
-           FLoc.isASTNode<ClosureExpr>() ? 1 : 0);
+
+  bool isNoReturnFn = F->isNoReturnFunction(F->getTypeExpansionContext());
+  if (FLoc.isASTNode<ClosureExpr>()) {
+    auto diagID = isNoReturnFn ? diag::missing_never_call_closure
+                               : diag::missing_return_closure;
+    diagnose(Context, L.getEndSourceLoc(), diagID, ResTy);
+  } else {
+    auto *DC = FLoc.getAsDeclContext();
+    assert(DC && DC->getAsDecl() && "not a declaration?");
+    auto diagID = isNoReturnFn ? diag::missing_never_call_decl
+                               : diag::missing_return_decl;
+    diagnose(Context, L.getEndSourceLoc(), diagID, ResTy,
+             DC->getAsDecl()->getDescriptiveKind());
+  }
 }
 
 static void diagnoseUnreachable(const SILInstruction *I,

test/AutoDiff/SILOptimizer/differentiation_diagnostics.swift

@@ -55,7 +55,7 @@ func try_apply_rethrows(_ x: Float) -> Float {
 @differentiable(reverse)
 func noReturn(_ x: Float) -> Float {
   let _ = x
-  // expected-error @+2 {{missing return in a function expected to return 'Float'}}
+  // expected-error @+2 {{missing return in global function expected to return 'Float'}}
   // expected-note @+1 {{missing return for differentiation}}
 }
 

test/FixCode/fixits-omit-return.swift

@@ -2,24 +2,28 @@
 
 func ff_fixit_addreturn() -> String {
     print("entering ff_fixit_addreturn()")
-    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a function expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
+    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in global function expected to return 'String'}} 
+    // expected-note@-1 {{did you mean to return the last expression?}}{{5-5=return }}
 }
 
 let cl_fixit_addreturn: () -> String = {
     print("entering cl_fixit_addreturn()")
-    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a closure expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
+    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in closure expected to return 'String'}}
+    // expected-note@-1 {{did you mean to return the last expression?}}{{5-5=return }}
 }
 
 func ff_fixit_addreturn_ifdecl() -> String {
     #if true
     print("entering ff_fixit_addreturn_ifdecl()")
-    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a function expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
+    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in global function expected to return 'String'}}
+    // expected-note@-1 {{did you mean to return the last expression?}}{{5-5=return }}
     #endif
 }
 
 let cl_fixit_addreturn_ifdecl: () -> String = {
     #if true
     print("entering cl_fixit_addreturn_ifdecl()")
-    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in a closure expected to return 'String'; did you mean to return the last expression?}} {{5-5=return }}
+    "foo" // expected-warning {{string literal is unused}} expected-error {{missing return in closure expected to return 'String'}}
+    // expected-note@-1 {{did you mean to return the last expression?}}{{5-5=return }}
     #endif
 }

test/Frontend/allow-errors.swift

@@ -70,9 +70,9 @@ func test(s: ValidStructInvalidMember) {
 // Check SIL diagnostics are still output (no reason not to output SIL since
 // there were no errors)
 func other() -> Int {}
-// CHECK-VALID: allow-errors.swift:[[@LINE-1]]:22: error: missing return in a function expected to return 'Int'
+// CHECK-VALID: allow-errors.swift:[[@LINE-1]]:22: error: missing return in global function expected to return 'Int'
 func other2() -> Bool {}
-// CHECK-VALID: allow-errors.swift:[[@LINE-1]]:24: error: missing return in a function expected to return 'Bool'
+// CHECK-VALID: allow-errors.swift:[[@LINE-1]]:24: error: missing return in global function expected to return 'Bool'
 #endif
 
 // All invalid uses should have no errors in the file itself, all referenced

test/SILOptimizer/return.swift

@@ -2,12 +2,12 @@
 
 func singleBlock() -> Int {
   _ = 0
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func singleBlock2() -> Int {
   var y = 0 
   y += 1
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 enum NoCasesButNotNever {}
 
@@ -36,7 +36,7 @@ func diagnoseNeverWithBody(i : Int) -> Never {
 } // expected-error {{function with uninhabited return type 'Never' is missing call to another never-returning function on all paths}}
 
 class MyClassWithClosure {
-  var f : (_ s: String) -> String = { (_ s: String) -> String in } // expected-error {{missing return in a closure expected to return 'String'}}
+  var f : (_ s: String) -> String = { (_ s: String) -> String in } // expected-error {{missing return in closure expected to return 'String'}}
 }
 
 func multipleBlocksSingleMissing(b: Bool) -> (String, Int) {
@@ -46,7 +46,7 @@ func multipleBlocksSingleMissing(b: Bool) -> (String, Int) {
   } else if (y == 0) {
     y += 1
   }
-} // expected-error {{missing return in a function expected to return '(String, Int)'}}
+} // expected-error {{missing return in global function expected to return '(String, Int)'}}
 
 func multipleBlocksAllMissing(x: Int) -> Int {
   var y : Int = x + 1 
@@ -56,15 +56,15 @@ func multipleBlocksAllMissing(x: Int) -> Int {
   }
   var x = 0
   x += 1
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 @_silgen_name("exit") func exit () -> Never
 
 func diagnose_missing_return_in_the_else_branch(i: Bool) -> Int {
   if (i) {
     exit() 
   } 
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func diagnose_missing_return_no_error_after_noreturn(i: Bool) -> Int {
   if (i) {
@@ -92,7 +92,7 @@ func whileLoop(flag: Bool) -> Int {
     }
     b += 1
   }
-} //expected-error {{missing return in a function expected to return 'Int'}}
+} //expected-error {{missing return in global function expected to return 'Int'}}
 
 struct S {}
 extension S:ExpressibleByStringLiteral {
@@ -166,21 +166,23 @@ func testSR13753() {
       get { 0 }
       set { }
     }
-    x // expected-error {{missing return in a closure expected to return 'Int'; did you mean to return the last expression?}} {{5-5=return }}
-    // expected-warning@-1 {{setter argument 'newValue' was never used, but the property was accessed}}
-    // expected-note@-2 {{did you mean to use 'newValue' instead of accessing the property's current value?}}
-    // expected-warning@-3 {{variable is unused}}
+    x // expected-error {{missing return in closure expected to return 'Int'}}
+    // expected-note@-1 {{did you mean to return the last expression?}}{{5-5=return }}
+    // expected-warning@-2 {{setter argument 'newValue' was never used, but the property was accessed}}
+    // expected-note@-3 {{did you mean to use 'newValue' instead of accessing the property's current value?}}
+    // expected-warning@-4 {{variable is unused}}
   }
 
   func f() -> Int {
     var x : Int {
         get { 0 }
         set { }
     }
-    x // expected-error {{missing return in a function expected to return 'Int'; did you mean to return the last expression?}} {{5-5=return }}
-    // expected-warning@-1 {{setter argument 'newValue' was never used, but the property was accessed}}
-    // expected-note@-2 {{did you mean to use 'newValue' instead of accessing the property's current value?}}
-    // expected-warning@-3 {{variable is unused}}
+    x // expected-error {{missing return in local function expected to return 'Int'}}
+    // expected-note@-1 {{did you mean to return the last expression?}}{{5-5=return }}
+    // expected-warning@-2 {{setter argument 'newValue' was never used, but the property was accessed}}
+    // expected-note@-3 {{did you mean to use 'newValue' instead of accessing the property's current value?}}
+    // expected-warning@-4 {{variable is unused}}
   } 
 
   let _ : () -> Int = {
@@ -192,7 +194,7 @@ func testSR13753() {
     // expected-warning@-1 {{setter argument 'newValue' was never used, but the property was accessed}}
     // expected-note@-2 {{did you mean to use 'newValue' instead of accessing the property's current value?}}
     // expected-warning@-3 {{variable is unused}}
-  } // expected-error {{missing return in a closure expected to return 'Int'}}
+  } // expected-error {{missing return in closure expected to return 'Int'}}
 
   func f1() -> Int {
     var x : UInt {
@@ -203,13 +205,50 @@ func testSR13753() {
     // expected-warning@-1 {{setter argument 'newValue' was never used, but the property was accessed}}
     // expected-note@-2 {{did you mean to use 'newValue' instead of accessing the property's current value?}}
     // expected-warning@-3 {{variable is unused}}
-  } // expected-error {{missing return in a function expected to return 'Int'}}
+  } // expected-error {{missing return in local function expected to return 'Int'}}
 
   let _ : () -> Int = {
     var x : Int = 0 // expected-warning {{variable 'x' was never mutated; consider changing to 'let' constant}}
     var _ : Int = 0
 
-    x // expected-error{{missing return in a closure expected to return 'Int'; did you mean to return the last expression?}} {{5-5=return }}
-    //expected-warning@-1{{variable is unused}}
+    x // expected-error{{missing return in closure expected to return 'Int'}}
+    // expected-note@-1 {{did you mean to return the last expression?}}{{5-5=return }}
+    //expected-warning@-2{{variable is unused}}
   }
 }
+
+// SR-14505
+struct SR14505 {
+    let b = true
+    var x: Int {
+        if b {
+            return 0
+        }
+    } // expected-error {{missing return in getter expected to return 'Int'}}
+
+    var y: Int {
+        get {
+            if b {
+                return 0
+            }
+        } // expected-error {{missing return in getter expected to return 'Int'}}
+        set {}
+    } 
+}
+
+class SR14505_C {
+  static let a = false
+  let b = true
+
+  func method() -> Int {
+    if b {
+      return 0
+    }
+  } // expected-error {{missing return in instance method expected to return 'Int'}}
+
+  class func method1() -> Int {
+    if a {
+      return 0
+    }
+  } // expected-error {{missing return in class method expected to return 'Int'}}
+}

test/SourceKit/Sema/enum-toraw/enum-toraw.swift.response

@@ -18,7 +18,7 @@
     key.column: 1,
     key.filepath: t2.swift,
     key.severity: source.diagnostic.severity.error,
-    key.description: "missing return in a function expected to return 'Bool'",
+    key.description: "missing return in global function expected to return 'Bool'",
     key.diagnostic_stage: source.diagnostic.stage.swift.sema
   }
 ]

validation-test/stdlib/AssertDiagnosticsSIL.swift

@@ -3,5 +3,5 @@
 func assertionFailure_isNotNoreturn() -> Int {
   _ = 0 // Don't implicitly return the assertionFailure call.
   assertionFailure("")
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 

validation-test/stdlib/BoolDiagnostics_Dataflow.swift

@@ -8,49 +8,49 @@ func test_constantFoldAnd1() -> Int {
     return 42
   }
   // FIXME: this is a false positive.
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func test_constantFoldAnd2() -> Int {
   while true && false {
     return 42
   }
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func test_constantFoldAnd3() -> Int {
   while false && true {
     return 42
   }
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func test_constantFoldAnd4() -> Int {
   while false && false {
     return 42
   }
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func test_constantFoldOr1() -> Int {
   while true || true {
     return 42
   }
   // FIXME: this is a false positive.
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func test_constantFoldOr2() -> Int {
   while true || false {
     return 42
   }
   // FIXME: this is a false positive.
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func test_constantFoldOr3() -> Int {
   while false || true {
     return 42
   }
   // FIXME: this is a false positive.
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}
 
 func test_constantFoldOr4() -> Int {
   while false || false {
     return 42
   }
-} // expected-error {{missing return in a function expected to return 'Int'}}
+} // expected-error {{missing return in global function expected to return 'Int'}}