From c9cc43aa66911b38e9da68de6c2c7ee1f37911ea Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 6 Sep 2021 16:16:52 -0700 Subject: [PATCH] Move increment checks to improve errors --- .../rustc_parse/src/parser/diagnostics.rs | 228 ++++++++++++++---- compiler/rustc_parse/src/parser/expr.rs | 25 ++ src/test/ui/parser/increment.fixed | 36 +++ src/test/ui/parser/increment.rs | 27 ++- src/test/ui/parser/increment.stderr | 49 ++-- 5 files changed, 286 insertions(+), 79 deletions(-) create mode 100644 src/test/ui/parser/increment.fixed diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 0361f7063fc..99844bef1ff 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -156,6 +156,52 @@ impl AttemptLocalParseRecovery { } } +#[derive(Debug, Copy, Clone)] +struct IncDecRecovery { + standalone: bool, + op: IncOrDec, + fixity: UnaryFixity, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum IncOrDec { + Inc, + // FIXME: `i--` recovery isn't implemented yet + #[allow(dead_code)] + Dec, +} + +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +enum UnaryFixity { + Pre, + Post, +} + +impl IncOrDec { + fn chr(&self) -> char { + match self { + Self::Inc => '+', + Self::Dec => '-', + } + } + + fn name(&self) -> &'static str { + match self { + Self::Inc => "increment", + Self::Dec => "decrement", + } + } +} + +impl std::fmt::Display for UnaryFixity { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Pre => write!(f, "prefix"), + Self::Post => write!(f, "postfix"), + } + } +} + // SnapshotParser is used to create a snapshot of the parser // without causing duplicate errors being emitted when the `Parser` // is dropped. @@ -1167,6 +1213,145 @@ impl<'a> Parser<'a> { Ok(()) } + pub(super) fn maybe_recover_from_prefix_increment( + &mut self, + operand_expr: P, + op_span: Span, + prev_is_semi: bool, + ) -> PResult<'a, P> { + let kind = IncDecRecovery { + standalone: prev_is_semi, + op: IncOrDec::Inc, + fixity: UnaryFixity::Pre, + }; + + self.recover_from_inc_dec(operand_expr, kind, op_span) + } + + pub(super) fn maybe_recover_from_postfix_increment( + &mut self, + operand_expr: P, + op_span: Span, + prev_is_semi: bool, + ) -> PResult<'a, P> { + let kind = IncDecRecovery { + standalone: prev_is_semi, + op: IncOrDec::Inc, + fixity: UnaryFixity::Post, + }; + + self.recover_from_inc_dec(operand_expr, kind, op_span) + } + + fn recover_from_inc_dec( + &mut self, + base: P, + kind: IncDecRecovery, + op_span: Span, + ) -> PResult<'a, P> { + let mut err = self.struct_span_err( + op_span, + &format!("Rust has no {} {} operator", kind.fixity, kind.op.name()), + ); + err.span_label(op_span, &format!("not a valid {} operator", kind.fixity)); + + if let ExprKind::Path(_, path) = &base.kind { + if let [segment] = path.segments.as_slice() { + let ident = segment.ident; + // (pre, post) + let spans = match kind.fixity { + UnaryFixity::Pre => (op_span, ident.span.shrink_to_hi()), + UnaryFixity::Post => (ident.span.shrink_to_lo(), op_span), + }; + + if !ident.is_reserved() { + if kind.standalone { + return self.inc_dec_standalone_recovery(base, err, kind, ident, spans); + } else { + match kind.fixity { + UnaryFixity::Pre => { + return self.prefix_inc_dec_suggest_and_recover( + base, err, kind, ident, spans, + ); + } + UnaryFixity::Post => { + return self.postfix_inc_dec_suggest_and_recover( + base, err, kind, ident, spans, + ); + } + } + } + } + } + } + + // base case + err.help(&format!("use `{}= 1` instead", kind.op.chr())); + err.emit(); + + Ok(base) + } + + fn prefix_inc_dec_suggest_and_recover( + &mut self, + base: P, + mut err: DiagnosticBuilder<'_>, + kind: IncDecRecovery, + ident: Ident, + (pre_span, post_span): (Span, Span), + ) -> PResult<'a, P> { + err.multipart_suggestion( + &format!("use `{}= 1` instead", kind.op.chr()), + vec![ + (pre_span, "{ ".to_string()), + (post_span, format!(" {}= 1; {} }}", kind.op.chr(), ident)), + ], + Applicability::MachineApplicable, + ); + err.emit(); + // TODO: recover + Ok(base) + } + + fn postfix_inc_dec_suggest_and_recover( + &mut self, + base: P, + mut err: DiagnosticBuilder<'_>, + kind: IncDecRecovery, + ident: Ident, + (pre_span, post_span): (Span, Span), + ) -> PResult<'a, P> { + err.multipart_suggestion( + &format!("use `{}= 1` instead", kind.op.chr()), + vec![ + (pre_span, "{ let tmp = ".to_string()), + (post_span, format!("; {} {}= 1; tmp }}", ident, kind.op.chr())), + ], + Applicability::MachineApplicable, + ); + err.emit(); + // TODO: recover + Ok(base) + } + + fn inc_dec_standalone_recovery( + &mut self, + base: P, + mut err: DiagnosticBuilder<'_>, + kind: IncDecRecovery, + _ident: Ident, + (pre_span, post_span): (Span, Span), + ) -> PResult<'a, P> { + err.multipart_suggestion( + &format!("use `{}= 1` instead", kind.op.chr()), + vec![(pre_span, String::new()), (post_span, format!(" {}= 1", kind.op.chr()))], + Applicability::MachineApplicable, + ); + err.emit(); + // TODO: recover + Ok(base) + } + /// Tries to recover from associated item paths like `[T]::AssocItem` / `(T, U)::AssocItem`. /// Attempts to convert the base expression/pattern/type into a type, parses the `::AssocItem` /// tail, and combines them into a `::AssocItem` expression/pattern/type. @@ -1882,49 +2067,6 @@ impl<'a> Parser<'a> { self.sess.expr_parentheses_needed(&mut err, *sp); } err.span_label(span, "expected expression"); - if self.prev_token.kind == TokenKind::BinOp(token::Plus) - && self.token.kind == TokenKind::BinOp(token::Plus) - && self.look_ahead(1, |t| !t.is_lit()) - { - let span = self.prev_token.span.to(self.token.span); - err.note("Rust has no dedicated increment operator"); - err.span_suggestion_verbose( - span, - "try using `+= 1` instead", - " += 1".into(), - Applicability::Unspecified, - ); - } else if self.token.kind == TokenKind::BinOp(token::Plus) - && self.look_ahead(1, |t| t.kind == TokenKind::BinOp(token::Plus)) - && self.look_ahead(2, |t| !t.is_lit()) - { - let target_span = self.look_ahead(2, |t| t.span); - let left_brace_span = target_span.shrink_to_lo(); - let pre_span = self.token.span.to(self.look_ahead(1, |t| t.span)); - let post_span = target_span.shrink_to_hi(); - - err.note("Rust has no dedicated increment operator"); - - if self.prev_token.kind == TokenKind::Semi { - err.multipart_suggestion( - "try using `+= 1` instead", - vec![(pre_span, String::new()), (post_span, " += 1".into())], - Applicability::MachineApplicable, - ); - } else if let Ok(target_snippet) = self.span_to_snippet(target_span) { - err.multipart_suggestion( - "try using `+= 1` instead", - vec![ - (left_brace_span, "{ ".to_string()), - (pre_span, String::new()), - (post_span, format!(" += 1; {} }}", target_snippet)), - ], - Applicability::MachineApplicable, - ); - } else { - err.span_help(pre_span.to(target_span), "try using `+= 1` instead"); - } - } err } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 148e0a24ec3..39d96b8a9e3 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -267,6 +267,18 @@ impl<'a> Parser<'a> { self.bump(); } + if self.prev_token == token::BinOp(token::Plus) + && self.token == token::BinOp(token::Plus) + { + let op_span = self.prev_token.span.to(self.token.span); + // Eat the second `+` + self.bump(); + // TODO: implement + let start_is_semi = false; + lhs = self.maybe_recover_from_postfix_increment(lhs, op_span, start_is_semi)?; + continue; + } + let op = op.node; // Special cases: if op == AssocOp::As { @@ -586,6 +598,19 @@ impl<'a> Parser<'a> { token::Ident(..) if this.is_mistaken_not_ident_negation() => { make_it!(this, attrs, |this, _| this.recover_not_expr(lo)) } + // Recover from `++x` + token::BinOp(token::Plus) + if this.look_ahead(1, |t| *t == token::BinOp(token::Plus)) => + { + let prev_is_semi = this.prev_token == token::Semi; + let pre_span = this.token.span.to(this.look_ahead(1, |t| t.span)); + // Eat both `+`s. + this.bump(); + this.bump(); + + let operand_expr = this.parse_path_start_expr(Default::default())?; + this.maybe_recover_from_prefix_increment(operand_expr, pre_span, prev_is_semi) + } _ => return this.parse_dot_or_call_expr(Some(attrs)), } } diff --git a/src/test/ui/parser/increment.fixed b/src/test/ui/parser/increment.fixed new file mode 100644 index 00000000000..ad61c4e66d2 --- /dev/null +++ b/src/test/ui/parser/increment.fixed @@ -0,0 +1,36 @@ +// run-rustfix + +fn post_regular() { + let mut i = 0; + { let tmp = i; i += 1; tmp }; //~ ERROR Rust has no postfix increment operator + println!("{}", i); +} + +fn post_while() { + let mut i = 0; + while { let tmp = i; i += 1; tmp } < 5 { + //~^ ERROR Rust has no postfix increment operator + println!("{}", i); + } +} + +fn pre_regular() { + let mut i = 0; + i += 1; //~ ERROR Rust has no prefix increment operator + println!("{}", i); +} + +fn pre_while() { + let mut i = 0; + while { i += 1; i } < 5 { + //~^ ERROR Rust has no prefix increment operator + println!("{}", i); + } +} + +fn main() { + post_regular(); + post_while(); + pre_regular(); + pre_while(); +} diff --git a/src/test/ui/parser/increment.rs b/src/test/ui/parser/increment.rs index 77a94b65f1f..f31031fed3a 100644 --- a/src/test/ui/parser/increment.rs +++ b/src/test/ui/parser/increment.rs @@ -1,27 +1,36 @@ +// run-rustfix + fn post_regular() { - let i = 0; - i++; //~ ERROR + let mut i = 0; + i++; //~ ERROR Rust has no postfix increment operator + println!("{}", i); } fn post_while() { - let i = 0; + let mut i = 0; while i++ < 5 { - //~^ ERROR + //~^ ERROR Rust has no postfix increment operator println!("{}", i); } } fn pre_regular() { - let i = 0; - ++i; //~ ERROR + let mut i = 0; + ++i; //~ ERROR Rust has no prefix increment operator + println!("{}", i); } fn pre_while() { - let i = 0; + let mut i = 0; while ++i < 5 { - //~^ ERROR + //~^ ERROR Rust has no prefix increment operator println!("{}", i); } } -fn main() {} +fn main() { + post_regular(); + post_while(); + pre_regular(); + pre_while(); +} diff --git a/src/test/ui/parser/increment.stderr b/src/test/ui/parser/increment.stderr index 667956cdd7d..6a2b37e3ddc 100644 --- a/src/test/ui/parser/increment.stderr +++ b/src/test/ui/parser/increment.stderr @@ -1,52 +1,47 @@ -error: expected expression, found `+` - --> $DIR/increment.rs:3:7 +error: Rust has no postfix increment operator + --> $DIR/increment.rs:5:6 | LL | i++; - | ^ expected expression + | ^^ not a valid postfix operator | - = note: Rust has no dedicated increment operator -help: try using `+= 1` instead +help: use `+= 1` instead | -LL | i += 1; - | ~~~~ +LL | { let tmp = i; i += 1; tmp }; + | +++++++++++ ~~~~~~~~~~~~~~~ -error: expected expression, found `+` - --> $DIR/increment.rs:8:13 +error: Rust has no postfix increment operator + --> $DIR/increment.rs:11:12 | LL | while i++ < 5 { - | ^ expected expression + | ^^ not a valid postfix operator | - = note: Rust has no dedicated increment operator -help: try using `+= 1` instead +help: use `+= 1` instead | -LL | while i += 1 < 5 { - | ~~~~ +LL | while { let tmp = i; i += 1; tmp } < 5 { + | +++++++++++ ~~~~~~~~~~~~~~~ -error: expected expression, found `+` - --> $DIR/increment.rs:16:5 +error: Rust has no prefix increment operator + --> $DIR/increment.rs:19:5 | LL | ++i; - | ^ expected expression + | ^^ not a valid prefix operator | - = note: Rust has no dedicated increment operator -help: try using `+= 1` instead +help: use `+= 1` instead | LL - ++i; LL + i += 1; | -error: expected expression, found `+` - --> $DIR/increment.rs:21:11 +error: Rust has no prefix increment operator + --> $DIR/increment.rs:25:11 | LL | while ++i < 5 { - | ^ expected expression + | ^^ not a valid prefix operator | - = note: Rust has no dedicated increment operator -help: try using `+= 1` instead +help: use `+= 1` instead | -LL - while ++i < 5 { -LL + while { i += 1; i } < 5 { - | +LL | while { i += 1; i } < 5 { + | ~ +++++++++ error: aborting due to 4 previous errors