From cfc0bd12581651e5d0f51d0d4c2d8306cc13cb51 Mon Sep 17 00:00:00 2001 From: Esteban Kuber Date: Mon, 10 Jan 2022 22:02:19 +0000 Subject: [PATCH] Parse `Ty?` as `Option` and provide structured suggestion Swift has specific syntax that desugars to `Option` similar to our `?` operator, which means that people might try to use it in Rust. Parse it and gracefully recover. --- .../rustc_parse/src/parser/diagnostics.rs | 30 ++++++++++++++++++- compiler/rustc_parse/src/parser/expr.rs | 4 +-- compiler/rustc_parse/src/parser/ty.rs | 26 ++++++++++++++++ .../issues/issue-35813-postfix-after-cast.rs | 4 +-- .../issue-35813-postfix-after-cast.stderr | 4 +-- src/test/ui/parser/issues/issue-84148-1.rs | 5 ++-- .../ui/parser/issues/issue-84148-1.stderr | 18 +++++------ src/test/ui/parser/issues/issue-84148-2.rs | 3 +- .../ui/parser/issues/issue-84148-2.stderr | 24 +++++++-------- .../ui/parser/trailing-question-in-type.fixed | 10 +++++++ .../ui/parser/trailing-question-in-type.rs | 10 +++++++ .../parser/trailing-question-in-type.stderr | 24 +++++++++++++++ 12 files changed, 125 insertions(+), 37 deletions(-) create mode 100644 src/test/ui/parser/trailing-question-in-type.fixed create mode 100644 src/test/ui/parser/trailing-question-in-type.rs create mode 100644 src/test/ui/parser/trailing-question-in-type.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 9677e7642b88c..b844e96d39c54 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -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, @@ -1032,6 +1032,34 @@ impl<'a> Parser<'a> { } } + /// Swift lets users write `Ty?` to mean `Option`. Parse the construct and recover from it. + pub(super) fn maybe_recover_from_question_mark( + &mut self, + ty: P, + is_as_cast: IsAsCast, + ) -> P { + 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, diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index f706a98a4fcfa..cd3846d5a224e 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -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 @@ -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", diff --git a/compiler/rustc_parse/src/parser/ty.rs b/compiler/rustc_parse/src/parser/ty.rs index 02a774ba1291c..566b77a5e9e55 100644 --- a/compiler/rustc_parse/src/parser/ty.rs +++ b/compiler/rustc_parse/src/parser/ty.rs @@ -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: @@ -100,6 +105,7 @@ impl<'a> Parser<'a> { RecoverQPath::Yes, RecoverReturnSign::Yes, None, + IsAsCast::No, ) } @@ -113,6 +119,7 @@ impl<'a> Parser<'a> { RecoverQPath::Yes, RecoverReturnSign::Yes, Some(ty_params), + IsAsCast::No, ) } @@ -126,6 +133,7 @@ impl<'a> Parser<'a> { RecoverQPath::Yes, RecoverReturnSign::Yes, None, + IsAsCast::No, ) } @@ -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> { + 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> { self.parse_ty_common( @@ -153,6 +174,7 @@ impl<'a> Parser<'a> { RecoverQPath::Yes, RecoverReturnSign::OnlyFatArrow, None, + IsAsCast::No, ) } @@ -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) { @@ -191,6 +214,7 @@ impl<'a> Parser<'a> { recover_qpath, recover_return_sign, None, + IsAsCast::No, )?; FnRetTy::Ty(ty) } else { @@ -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> { let allow_qpath_recovery = recover_qpath == RecoverQPath::Yes; maybe_recover_from_interpolated_ty_qpath!(self, allow_qpath_recovery); @@ -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) } diff --git a/src/test/ui/parser/issues/issue-35813-postfix-after-cast.rs b/src/test/ui/parser/issues/issue-35813-postfix-after-cast.rs index e725aa5d73d1f..23f245a51681b 100644 --- a/src/test/ui/parser/issues/issue-35813-postfix-after-cast.rs +++ b/src/test/ui/parser/issues/issue-35813-postfix-after-cast.rs @@ -117,9 +117,9 @@ static bar2: &[i32] = &(&[1i32,2,3]: &[i32; 3][0..1]); pub fn cast_then_try() -> Result { Err(0u64) as Result?; - //~^ ERROR: casts cannot be followed by ? + //~^ ERROR: casts cannot be followed by `?` Err(0u64): Result?; - //~^ ERROR: casts cannot be followed by ? + //~^ ERROR: casts cannot be followed by `?` Ok(1) } diff --git a/src/test/ui/parser/issues/issue-35813-postfix-after-cast.stderr b/src/test/ui/parser/issues/issue-35813-postfix-after-cast.stderr index 19b68556d79ad..e96b67da3364d 100644 --- a/src/test/ui/parser/issues/issue-35813-postfix-after-cast.stderr +++ b/src/test/ui/parser/issues/issue-35813-postfix-after-cast.stderr @@ -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?; @@ -276,7 +276,7 @@ help: try surrounding the expression in parentheses LL | (Err(0u64) as Result)?; | + + -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?; diff --git a/src/test/ui/parser/issues/issue-84148-1.rs b/src/test/ui/parser/issues/issue-84148-1.rs index 25f7ba4d1f88e..9fa8086c2c9bf 100644 --- a/src/test/ui/parser/issues/issue-84148-1.rs +++ b/src/test/ui/parser/issues/issue-84148-1.rs @@ -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 diff --git a/src/test/ui/parser/issues/issue-84148-1.stderr b/src/test/ui/parser/issues/issue-84148-1.stderr index 77f0896e9c155..9261067c22158 100644 --- a/src/test/ui/parser/issues/issue-84148-1.stderr +++ b/src/test/ui/parser/issues/issue-84148-1.stderr @@ -1,17 +1,13 @@ -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:Optiont>) + | +++++++ ~ error: expected one of `->`, `where`, or `{`, found `` --> $DIR/issue-84148-1.rs:1:15 @@ -19,5 +15,5 @@ error: expected one of `->`, `where`, or `{`, found `` 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 diff --git a/src/test/ui/parser/issues/issue-84148-2.rs b/src/test/ui/parser/issues/issue-84148-2.rs index 257a3fd67207e..2f6a7facfb271 100644 --- a/src/test/ui/parser/issues/issue-84148-2.rs +++ b/src/test/ui/parser/issues/issue-84148-2.rs @@ -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? diff --git a/src/test/ui/parser/issues/issue-84148-2.stderr b/src/test/ui/parser/issues/issue-84148-2.stderr index 396208316df67..71d543f9b7344 100644 --- a/src/test/ui/parser/issues/issue-84148-2.stderr +++ b/src/test/ui/parser/issues/issue-84148-2.stderr @@ -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:Optiont> + | +++++++ ~ error: expected one of `->`, `where`, or `{`, found `` - --> $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 diff --git a/src/test/ui/parser/trailing-question-in-type.fixed b/src/test/ui/parser/trailing-question-in-type.fixed new file mode 100644 index 0000000000000..6ea24484e033e --- /dev/null +++ b/src/test/ui/parser/trailing-question-in-type.fixed @@ -0,0 +1,10 @@ +// run-rustfix + +fn foo() -> Option { //~ ERROR invalid `?` in type + let x: Option = Some(1); //~ ERROR invalid `?` in type + x +} + +fn main() { + let _: Option = foo(); +} diff --git a/src/test/ui/parser/trailing-question-in-type.rs b/src/test/ui/parser/trailing-question-in-type.rs new file mode 100644 index 0000000000000..b1c508365cff5 --- /dev/null +++ b/src/test/ui/parser/trailing-question-in-type.rs @@ -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 = foo(); +} diff --git a/src/test/ui/parser/trailing-question-in-type.stderr b/src/test/ui/parser/trailing-question-in-type.stderr new file mode 100644 index 0000000000000..a3cd419c0c718 --- /dev/null +++ b/src/test/ui/parser/trailing-question-in-type.stderr @@ -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 { + | +++++++ ~ + +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 = Some(1); + | +++++++ ~ + +error: aborting due to 2 previous errors +