Skip to content

[lib/Sema] Suggest return when the last statement would be a valid … #42415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -4229,6 +4229,8 @@ WARNING(trailing_closure_requires_parens,none,
ERROR(opaque_type_no_underlying_type_candidates,none,
"function declares an opaque return type, but has no return statements "
"in its body from which to infer an underlying type", ())
NOTE(opaque_type_missing_return_last_expr_note,none,
"did you mean to return the last expression?", ())
ERROR(opaque_type_mismatched_underlying_type_candidates,none,
"function declares an opaque return type %0, but the return statements "
"in its body do not have matching underlying types", (TypeRepr *))
Expand Down
37 changes: 36 additions & 1 deletion lib/Sema/MiscDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2632,7 +2632,7 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
void check() {
Body->walk(*this);

// If given function has any invalid returns in the body
// If given function has any invalid `return`s in the body
// let's not try to validate the types, since it wouldn't
// be accurate.
if (HasInvalidReturn)
Expand All @@ -2642,6 +2642,41 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
// we have nothing to infer the underlying type from.
if (Candidates.empty()) {
Implementation->diagnose(diag::opaque_type_no_underlying_type_candidates);

// We try to find if the last element of the `Body` multi element
// `BraceStmt` is an expression that produces a value that satisfies all
// the opaque type requirements and if that is the case, it means we can
// suggest a fix-it note to add an explicit `return`.
if (Body->getNumElements() > 1) {
auto element = Body->getLastElement();
// Let's see if the last statement would make for a valid return value.
if (auto expr = element.dyn_cast<Expr *>()) {
bool conforms = llvm::all_of(OpaqueDecl->getOpaqueInterfaceGenericSignature().getRequirements(),
[&expr, this](auto requirement) {
if (requirement.getKind() == RequirementKind::Conformance) {
auto conformance =
TypeChecker::conformsToProtocol(expr->getType()->getRValueType(),
requirement.getProtocolDecl(),
Implementation->getModuleContext(),
/*allowMissing=*/ false);
return !conformance.isInvalid();
}
// If we encounter any requirements other than `Conformance`, we do
// not attempt to type check the expression.
return false;
});

// If all requirements are fulfilled, we offer to insert `return` to
// fix the issue.
if (conforms) {
Ctx.Diags
.diagnose(expr->getStartLoc(),
diag::opaque_type_missing_return_last_expr_note)
.fixItInsert(expr->getStartLoc(), "return ");
}
}
}

return;
}

Expand Down
1 change: 1 addition & 0 deletions test/Constraints/result_builder_infer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ struct DoNotTupleMe {
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}}
"hello" // expected-warning{{string literal is unused}}
"world" // expected-warning{{string literal is unused}}
// expected-note@-1 {{did you mean to return the last expression?}} {{5-5=return }}
}
}

Expand Down
34 changes: 34 additions & 0 deletions test/type/opaque.swift
Original file line number Diff line number Diff line change
Expand Up @@ -533,3 +533,37 @@ func test_diagnostic_with_contextual_generic_params() {
}
}
}

// SR-10988 - Suggest `return` when the last statement of a multi-statement function body would be a valid return value
protocol P1 {
}
protocol P2 {
}
func sr10988() {
func test() -> some Numeric {
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
let x = 0
x // expected-note {{did you mean to return the last expression?}} {{5-5=return }}
// expected-warning@-1 {{expression of type 'Int' is unused}}
}
func test2() -> some Numeric {
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
let x = "s"
x // expected-warning {{expression of type 'String' is unused}}
}
struct S1: P1, P2 {
}
struct S2: P1 {
}
func test3() -> some P1 & P2 {
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
let x = S1()
x // expected-note {{did you mean to return the last expression?}} {{5-5=return }}
// expected-warning@-1 {{expression of type 'S1' is unused}}
}
func test4() -> some P1 & P2 {
// expected-error@-1 {{function declares an opaque return type, but has no return statements in its body from which to infer an underlying type}}
let x = S2()
x // expected-warning {{expression of type 'S2' is unused}}
}
}