Skip to content

Commit 5d55ffe

Browse files
[lib/Sema] Suggest return when the last statement would be a valid … (#42415)
* [lib/Sema] Suggest `return` when the last statement would be a valid return value Addresses SR-10988. When a multi-statement function body contains no return keyword but the last statement would be a valid return value, the compiler currently proposes a fixit to add the return keyword. This changes extends this behavior to functions with opaque return types.
1 parent 5f2b790 commit 5d55ffe

File tree

4 files changed

+73
-1
lines changed

4 files changed

+73
-1
lines changed

Diff for: include/swift/AST/DiagnosticsSema.def

+2
Original file line numberDiff line numberDiff line change
@@ -4229,6 +4229,8 @@ WARNING(trailing_closure_requires_parens,none,
42294229
ERROR(opaque_type_no_underlying_type_candidates,none,
42304230
"function declares an opaque return type, but has no return statements "
42314231
"in its body from which to infer an underlying type", ())
4232+
NOTE(opaque_type_missing_return_last_expr_note,none,
4233+
"did you mean to return the last expression?", ())
42324234
ERROR(opaque_type_mismatched_underlying_type_candidates,none,
42334235
"function declares an opaque return type %0, but the return statements "
42344236
"in its body do not have matching underlying types", (TypeRepr *))

Diff for: lib/Sema/MiscDiagnostics.cpp

+36-1
Original file line numberDiff line numberDiff line change
@@ -2632,7 +2632,7 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
26322632
void check() {
26332633
Body->walk(*this);
26342634

2635-
// If given function has any invalid returns in the body
2635+
// If given function has any invalid `return`s in the body
26362636
// let's not try to validate the types, since it wouldn't
26372637
// be accurate.
26382638
if (HasInvalidReturn)
@@ -2642,6 +2642,41 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
26422642
// we have nothing to infer the underlying type from.
26432643
if (Candidates.empty()) {
26442644
Implementation->diagnose(diag::opaque_type_no_underlying_type_candidates);
2645+
2646+
// We try to find if the last element of the `Body` multi element
2647+
// `BraceStmt` is an expression that produces a value that satisfies all
2648+
// the opaque type requirements and if that is the case, it means we can
2649+
// suggest a fix-it note to add an explicit `return`.
2650+
if (Body->getNumElements() > 1) {
2651+
auto element = Body->getLastElement();
2652+
// Let's see if the last statement would make for a valid return value.
2653+
if (auto expr = element.dyn_cast<Expr *>()) {
2654+
bool conforms = llvm::all_of(OpaqueDecl->getOpaqueInterfaceGenericSignature().getRequirements(),
2655+
[&expr, this](auto requirement) {
2656+
if (requirement.getKind() == RequirementKind::Conformance) {
2657+
auto conformance =
2658+
TypeChecker::conformsToProtocol(expr->getType()->getRValueType(),
2659+
requirement.getProtocolDecl(),
2660+
Implementation->getModuleContext(),
2661+
/*allowMissing=*/ false);
2662+
return !conformance.isInvalid();
2663+
}
2664+
// If we encounter any requirements other than `Conformance`, we do
2665+
// not attempt to type check the expression.
2666+
return false;
2667+
});
2668+
2669+
// If all requirements are fulfilled, we offer to insert `return` to
2670+
// fix the issue.
2671+
if (conforms) {
2672+
Ctx.Diags
2673+
.diagnose(expr->getStartLoc(),
2674+
diag::opaque_type_missing_return_last_expr_note)
2675+
.fixItInsert(expr->getStartLoc(), "return ");
2676+
}
2677+
}
2678+
}
2679+
26452680
return;
26462681
}
26472682

Diff for: test/Constraints/result_builder_infer.swift

+1
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ struct DoNotTupleMe {
7474
var tuple: some Any { // expected-error{{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
7575
"hello" // expected-warning{{string literal is unused}}
7676
"world" // expected-warning{{string literal is unused}}
77+
// expected-note@-1 {{did you mean to return the last expression?}} {{5-5=return }}
7778
}
7879
}
7980

Diff for: test/type/opaque.swift

+34
Original file line numberDiff line numberDiff line change
@@ -533,3 +533,37 @@ func test_diagnostic_with_contextual_generic_params() {
533533
}
534534
}
535535
}
536+
537+
// SR-10988 - Suggest `return` when the last statement of a multi-statement function body would be a valid return value
538+
protocol P1 {
539+
}
540+
protocol P2 {
541+
}
542+
func sr10988() {
543+
func test() -> some Numeric {
544+
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
545+
let x = 0
546+
x // expected-note {{did you mean to return the last expression?}} {{5-5=return }}
547+
// expected-warning@-1 {{expression of type 'Int' is unused}}
548+
}
549+
func test2() -> some Numeric {
550+
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
551+
let x = "s"
552+
x // expected-warning {{expression of type 'String' is unused}}
553+
}
554+
struct S1: P1, P2 {
555+
}
556+
struct S2: P1 {
557+
}
558+
func test3() -> some P1 & P2 {
559+
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
560+
let x = S1()
561+
x // expected-note {{did you mean to return the last expression?}} {{5-5=return }}
562+
// expected-warning@-1 {{expression of type 'S1' is unused}}
563+
}
564+
func test4() -> some P1 & P2 {
565+
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
566+
let x = S2()
567+
x // expected-warning {{expression of type 'S2' is unused}}
568+
}
569+
}

0 commit comments

Comments
 (0)