Skip to content

Commit

Permalink
Parse Ty? as Option<Ty> and provide structured suggestion
Browse files Browse the repository at this point in the history
Swift has specific syntax that desugars to `Option<T>` similar to our
`?` operator, which means that people might try to use it in Rust. Parse
it and gracefully recover.
  • Loading branch information
estebank committed Jan 14, 2022
1 parent 89b9f7b commit cfc0bd1
Show file tree
Hide file tree
Showing 12 changed files with 125 additions and 37 deletions.
30 changes: 29 additions & 1 deletion compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use super::pat::Expected;
use super::ty::AllowPlus;
use super::ty::{AllowPlus, IsAsCast};
use super::{
BlockMode, Parser, PathStyle, RecoverColon, RecoverComma, Restrictions, SemiColonMode, SeqSep,
TokenExpectType, TokenType,
Expand Down Expand Up @@ -1032,6 +1032,34 @@ impl<'a> Parser<'a> {
}
}

/// Swift lets users write `Ty?` to mean `Option<Ty>`. Parse the construct and recover from it.
pub(super) fn maybe_recover_from_question_mark(
&mut self,
ty: P<Ty>,
is_as_cast: IsAsCast,
) -> P<Ty> {
if let IsAsCast::Yes = is_as_cast {
return ty;
}
if self.token == token::Question {
self.bump();
self.struct_span_err(self.prev_token.span, "invalid `?` in type")
.span_label(self.prev_token.span, "`?` is only allowed on expressions, not types")
.multipart_suggestion(
"if you meant to express that the type might not contain a value, use the `Option` wrapper type",
vec![
(ty.span.shrink_to_lo(), "Option<".to_string()),
(self.prev_token.span, ">".to_string()),
],
Applicability::MachineApplicable,
)
.emit();
self.mk_ty(ty.span.to(self.prev_token.span), TyKind::Err)
} else {
ty
}
}

pub(super) fn maybe_recover_from_bad_type_plus(
&mut self,
allow_plus: AllowPlus,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ impl<'a> Parser<'a> {
// Save the state of the parser before parsing type normally, in case there is a
// LessThan comparison after this cast.
let parser_snapshot_before_type = self.clone();
let cast_expr = match self.parse_ty_no_plus() {
let cast_expr = match self.parse_as_cast_ty() {
Ok(rhs) => mk_expr(self, lhs, rhs),
Err(mut type_err) => {
// Rewind to before attempting to parse the type with generics, to recover
Expand Down Expand Up @@ -808,7 +808,7 @@ impl<'a> Parser<'a> {
"casts cannot be followed by {}",
match with_postfix.kind {
ExprKind::Index(_, _) => "indexing",
ExprKind::Try(_) => "?",
ExprKind::Try(_) => "`?`",
ExprKind::Field(_, _) => "a field access",
ExprKind::MethodCall(_, _, _) => "a method call",
ExprKind::Call(_, _) => "a function call",
Expand Down
26 changes: 26 additions & 0 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ pub(super) enum RecoverQPath {
No,
}

pub(super) enum IsAsCast {
Yes,
No,
}

/// Signals whether parsing a type should recover `->`.
///
/// More specifically, when parsing a function like:
Expand Down Expand Up @@ -100,6 +105,7 @@ impl<'a> Parser<'a> {
RecoverQPath::Yes,
RecoverReturnSign::Yes,
None,
IsAsCast::No,
)
}

Expand All @@ -113,6 +119,7 @@ impl<'a> Parser<'a> {
RecoverQPath::Yes,
RecoverReturnSign::Yes,
Some(ty_params),
IsAsCast::No,
)
}

Expand All @@ -126,6 +133,7 @@ impl<'a> Parser<'a> {
RecoverQPath::Yes,
RecoverReturnSign::Yes,
None,
IsAsCast::No,
)
}

Expand All @@ -142,9 +150,22 @@ impl<'a> Parser<'a> {
RecoverQPath::Yes,
RecoverReturnSign::Yes,
None,
IsAsCast::No,
)
}

/// Parses a type following an `as` cast. Similar to `parse_ty_no_plus`, but signaling origin
/// for better diagnostics involving `?`.
pub(super) fn parse_as_cast_ty(&mut self) -> PResult<'a, P<Ty>> {
self.parse_ty_common(
AllowPlus::No,
AllowCVariadic::No,
RecoverQPath::Yes,
RecoverReturnSign::Yes,
None,
IsAsCast::Yes,
)
}
/// Parse a type without recovering `:` as `->` to avoid breaking code such as `where fn() : for<'a>`
pub(super) fn parse_ty_for_where_clause(&mut self) -> PResult<'a, P<Ty>> {
self.parse_ty_common(
Expand All @@ -153,6 +174,7 @@ impl<'a> Parser<'a> {
RecoverQPath::Yes,
RecoverReturnSign::OnlyFatArrow,
None,
IsAsCast::No,
)
}

Expand All @@ -171,6 +193,7 @@ impl<'a> Parser<'a> {
recover_qpath,
recover_return_sign,
None,
IsAsCast::No,
)?;
FnRetTy::Ty(ty)
} else if recover_return_sign.can_recover(&self.token.kind) {
Expand All @@ -191,6 +214,7 @@ impl<'a> Parser<'a> {
recover_qpath,
recover_return_sign,
None,
IsAsCast::No,
)?;
FnRetTy::Ty(ty)
} else {
Expand All @@ -205,6 +229,7 @@ impl<'a> Parser<'a> {
recover_qpath: RecoverQPath,
recover_return_sign: RecoverReturnSign,
ty_generics: Option<&Generics>,
is_as_cast: IsAsCast,
) -> PResult<'a, P<Ty>> {
let allow_qpath_recovery = recover_qpath == RecoverQPath::Yes;
maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery);
Expand Down Expand Up @@ -280,6 +305,7 @@ impl<'a> Parser<'a> {
// Try to recover from use of `+` with incorrect priority.
self.maybe_report_ambiguous_plus(allow_plus, impl_dyn_multi, &ty);
self.maybe_recover_from_bad_type_plus(allow_plus, &ty)?;
let ty = self.maybe_recover_from_question_mark(ty, is_as_cast);
self.maybe_recover_from_bad_qpath(ty, allow_qpath_recovery)
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-35813-postfix-after-cast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ static bar2: &[i32] = &(&[1i32,2,3]: &[i32; 3][0..1]);

pub fn cast_then_try() -> Result<u64,u64> {
Err(0u64) as Result<u64,u64>?;
//~^ ERROR: casts cannot be followed by ?
//~^ ERROR: casts cannot be followed by `?`
Err(0u64): Result<u64,u64>?;
//~^ ERROR: casts cannot be followed by ?
//~^ ERROR: casts cannot be followed by `?`
Ok(1)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ help: try surrounding the expression in parentheses
LL | static bar2: &[i32] = &((&[1i32,2,3]: &[i32; 3])[0..1]);
| + +

error: casts cannot be followed by ?
error: casts cannot be followed by `?`
--> $DIR/issue-35813-postfix-after-cast.rs:119:5
|
LL | Err(0u64) as Result<u64,u64>?;
Expand All @@ -276,7 +276,7 @@ help: try surrounding the expression in parentheses
LL | (Err(0u64) as Result<u64,u64>)?;
| + +

error: casts cannot be followed by ?
error: casts cannot be followed by `?`
--> $DIR/issue-35813-postfix-after-cast.rs:121:5
|
LL | Err(0u64): Result<u64,u64>?;
Expand Down
5 changes: 2 additions & 3 deletions src/test/ui/parser/issues/issue-84148-1.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
fn f(t:for<>t?)
//~^ ERROR: expected parameter name
//~| ERROR: expected one of
//~| ERROR: expected one of
//~^ ERROR: expected one of
//~| ERROR: invalid `?` in type
18 changes: 7 additions & 11 deletions src/test/ui/parser/issues/issue-84148-1.stderr
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
error: expected parameter name, found `?`
error: invalid `?` in type
--> $DIR/issue-84148-1.rs:1:14
|
LL | fn f(t:for<>t?)
| ^ expected parameter name

error: expected one of `(`, `)`, `+`, `,`, `::`, or `<`, found `?`
--> $DIR/issue-84148-1.rs:1:14
| ^ `?` is only allowed on expressions, not types
|
LL | fn f(t:for<>t?)
| ^
| |
| expected one of `(`, `)`, `+`, `,`, `::`, or `<`
| help: missing `,`
help: if you meant to express that the type might not contain a value, use the `Option` wrapper type
|
LL | fn f(t:Option<for<>t>)
| +++++++ ~

error: expected one of `->`, `where`, or `{`, found `<eof>`
--> $DIR/issue-84148-1.rs:1:15
|
LL | fn f(t:for<>t?)
| ^ expected one of `->`, `where`, or `{`

error: aborting due to 3 previous errors
error: aborting due to 2 previous errors

3 changes: 1 addition & 2 deletions src/test/ui/parser/issues/issue-84148-2.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// error-pattern: this file contains an unclosed delimiter
// error-pattern: expected parameter name
// error-pattern: expected one of
// error-pattern: invalid `?` in type
fn f(t:for<>t?
24 changes: 10 additions & 14 deletions src/test/ui/parser/issues/issue-84148-2.stderr
Original file line number Diff line number Diff line change
@@ -1,31 +1,27 @@
error: this file contains an unclosed delimiter
--> $DIR/issue-84148-2.rs:4:16
--> $DIR/issue-84148-2.rs:3:16
|
LL | fn f(t:for<>t?
| - ^
| |
| unclosed delimiter

error: expected parameter name, found `?`
--> $DIR/issue-84148-2.rs:4:14
error: invalid `?` in type
--> $DIR/issue-84148-2.rs:3:14
|
LL | fn f(t:for<>t?
| ^ expected parameter name

error: expected one of `(`, `)`, `+`, `,`, `::`, or `<`, found `?`
--> $DIR/issue-84148-2.rs:4:14
| ^ `?` is only allowed on expressions, not types
|
LL | fn f(t:for<>t?
| ^
| |
| expected one of `(`, `)`, `+`, `,`, `::`, or `<`
| help: missing `,`
help: if you meant to express that the type might not contain a value, use the `Option` wrapper type
|
LL | fn f(t:Option<for<>t>
| +++++++ ~

error: expected one of `->`, `where`, or `{`, found `<eof>`
--> $DIR/issue-84148-2.rs:4:16
--> $DIR/issue-84148-2.rs:3:16
|
LL | fn f(t:for<>t?
| ^ expected one of `->`, `where`, or `{`

error: aborting due to 4 previous errors
error: aborting due to 3 previous errors

10 changes: 10 additions & 0 deletions src/test/ui/parser/trailing-question-in-type.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// run-rustfix

fn foo() -> Option<i32> { //~ ERROR invalid `?` in type
let x: Option<i32> = Some(1); //~ ERROR invalid `?` in type
x
}

fn main() {
let _: Option<i32> = foo();
}
10 changes: 10 additions & 0 deletions src/test/ui/parser/trailing-question-in-type.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// run-rustfix

fn foo() -> i32? { //~ ERROR invalid `?` in type
let x: i32? = Some(1); //~ ERROR invalid `?` in type
x
}

fn main() {
let _: Option<i32> = foo();
}
24 changes: 24 additions & 0 deletions src/test/ui/parser/trailing-question-in-type.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
error: invalid `?` in type
--> $DIR/trailing-question-in-type.rs:3:16
|
LL | fn foo() -> i32? {
| ^ `?` is only allowed on expressions, not types
|
help: if you meant to express that the type might not contain a value, use the `Option` wrapper type
|
LL | fn foo() -> Option<i32> {
| +++++++ ~

error: invalid `?` in type
--> $DIR/trailing-question-in-type.rs:4:15
|
LL | let x: i32? = Some(1);
| ^ `?` is only allowed on expressions, not types
|
help: if you meant to express that the type might not contain a value, use the `Option` wrapper type
|
LL | let x: Option<i32> = Some(1);
| +++++++ ~

error: aborting due to 2 previous errors

0 comments on commit cfc0bd1

Please sign in to comment.