From 5d9cd4b851f121d0bc84cab474d6c536aba207df Mon Sep 17 00:00:00 2001 From: Camelid Date: Thu, 25 Mar 2021 21:04:03 -0700 Subject: [PATCH 01/16] Suggest `i += 1` when we see `i++` or `++i` --- .../rustc_parse/src/parser/diagnostics.rs | 43 +++++++++++++++ src/test/ui/parser/increment.rs | 27 ++++++++++ src/test/ui/parser/increment.stderr | 52 +++++++++++++++++++ 3 files changed, 122 insertions(+) create mode 100644 src/test/ui/parser/increment.rs create mode 100644 src/test/ui/parser/increment.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index b5b628a3f55bd..0361f7063fced 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1882,6 +1882,49 @@ 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/src/test/ui/parser/increment.rs b/src/test/ui/parser/increment.rs new file mode 100644 index 0000000000000..77a94b65f1f0e --- /dev/null +++ b/src/test/ui/parser/increment.rs @@ -0,0 +1,27 @@ +fn post_regular() { + let i = 0; + i++; //~ ERROR +} + +fn post_while() { + let i = 0; + while i++ < 5 { + //~^ ERROR + println!("{}", i); + } +} + +fn pre_regular() { + let i = 0; + ++i; //~ ERROR +} + +fn pre_while() { + let i = 0; + while ++i < 5 { + //~^ ERROR + println!("{}", i); + } +} + +fn main() {} diff --git a/src/test/ui/parser/increment.stderr b/src/test/ui/parser/increment.stderr new file mode 100644 index 0000000000000..667956cdd7d40 --- /dev/null +++ b/src/test/ui/parser/increment.stderr @@ -0,0 +1,52 @@ +error: expected expression, found `+` + --> $DIR/increment.rs:3:7 + | +LL | i++; + | ^ expected expression + | + = note: Rust has no dedicated increment operator +help: try using `+= 1` instead + | +LL | i += 1; + | ~~~~ + +error: expected expression, found `+` + --> $DIR/increment.rs:8:13 + | +LL | while i++ < 5 { + | ^ expected expression + | + = note: Rust has no dedicated increment operator +help: try using `+= 1` instead + | +LL | while i += 1 < 5 { + | ~~~~ + +error: expected expression, found `+` + --> $DIR/increment.rs:16:5 + | +LL | ++i; + | ^ expected expression + | + = note: Rust has no dedicated increment operator +help: try using `+= 1` instead + | +LL - ++i; +LL + i += 1; + | + +error: expected expression, found `+` + --> $DIR/increment.rs:21:11 + | +LL | while ++i < 5 { + | ^ expected expression + | + = note: Rust has no dedicated increment operator +help: try using `+= 1` instead + | +LL - while ++i < 5 { +LL + while { i += 1; i } < 5 { + | + +error: aborting due to 4 previous errors + From c9cc43aa66911b38e9da68de6c2c7ee1f37911ea Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 6 Sep 2021 16:16:52 -0700 Subject: [PATCH 02/16] 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 0361f7063fced..99844bef1ffec 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 148e0a24ec304..39d96b8a9e326 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 0000000000000..ad61c4e66d28c --- /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 77a94b65f1f0e..f31031fed3aff 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 667956cdd7d40..6a2b37e3ddcf2 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 From d915606d50c273b084bf71bff1f31e347f0ccdee Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Tue, 15 Feb 2022 19:59:04 -0800 Subject: [PATCH 03/16] Remove error recovery todos --- .../rustc_parse/src/parser/diagnostics.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 99844bef1ffec..b29ea808d3240 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1294,8 +1294,8 @@ impl<'a> Parser<'a> { fn prefix_inc_dec_suggest_and_recover( &mut self, - base: P, - mut err: DiagnosticBuilder<'_>, + _base: P, + mut err: DiagnosticBuilder<'a>, kind: IncDecRecovery, ident: Ident, (pre_span, post_span): (Span, Span), @@ -1308,15 +1308,13 @@ impl<'a> Parser<'a> { ], Applicability::MachineApplicable, ); - err.emit(); - // TODO: recover - Ok(base) + Err(err) } fn postfix_inc_dec_suggest_and_recover( &mut self, - base: P, - mut err: DiagnosticBuilder<'_>, + _base: P, + mut err: DiagnosticBuilder<'a>, kind: IncDecRecovery, ident: Ident, (pre_span, post_span): (Span, Span), @@ -1329,15 +1327,13 @@ impl<'a> Parser<'a> { ], Applicability::MachineApplicable, ); - err.emit(); - // TODO: recover - Ok(base) + Err(err) } fn inc_dec_standalone_recovery( &mut self, - base: P, - mut err: DiagnosticBuilder<'_>, + _base: P, + mut err: DiagnosticBuilder<'a>, kind: IncDecRecovery, _ident: Ident, (pre_span, post_span): (Span, Span), @@ -1347,9 +1343,7 @@ impl<'a> Parser<'a> { vec![(pre_span, String::new()), (post_span, format!(" {}= 1", kind.op.chr()))], Applicability::MachineApplicable, ); - err.emit(); - // TODO: recover - Ok(base) + Err(err) } /// Tries to recover from associated item paths like `[T]::AssocItem` / `(T, U)::AssocItem`. From 80e57e223e98f7075cc4af83fa0eef948493d0df Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Tue, 15 Feb 2022 20:05:24 -0800 Subject: [PATCH 04/16] Reduce rightward drift --- .../rustc_parse/src/parser/diagnostics.rs | 61 +++++++++---------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index b29ea808d3240..89cb1c89f8190 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1255,41 +1255,40 @@ impl<'a> Parser<'a> { ); 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), - }; + let help_base_case = |mut err: DiagnosticBuilder<'_>, base| { + err.help(&format!("use `{}= 1` instead", kind.op.chr())); + err.emit(); + Ok(base) + }; - 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, - ); - } - } - } + let ExprKind::Path(_, path) = &base.kind + else { return help_base_case(err, base) }; + let [segment] = path.segments.as_slice() + else { return help_base_case(err, base) }; + 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() { + return help_base_case(err, base); + } + + if kind.standalone { + self.inc_dec_standalone_recovery(base, err, kind, ident, spans) + } else { + match kind.fixity { + UnaryFixity::Pre => { + self.prefix_inc_dec_suggest_and_recover(base, err, kind, ident, spans) + } + UnaryFixity::Post => { + 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( From 67a9adbb541f7c62e993d05ff3687a8695d5d349 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 17 Feb 2022 12:28:07 -0800 Subject: [PATCH 05/16] Refactor, handle fields better, add field tests --- .../rustc_parse/src/parser/diagnostics.rs | 20 +++++++------- compiler/rustc_parse/src/parser/expr.rs | 16 ++++++------ ...ncrement.fixed => increment-autofix.fixed} | 0 .../{increment.rs => increment-autofix.rs} | 0 ...rement.stderr => increment-autofix.stderr} | 8 +++--- src/test/ui/parser/increment-notfixed.rs | 26 +++++++++++++++++++ src/test/ui/parser/increment-notfixed.stderr | 18 +++++++++++++ 7 files changed, 66 insertions(+), 22 deletions(-) rename src/test/ui/parser/{increment.fixed => increment-autofix.fixed} (100%) rename src/test/ui/parser/{increment.rs => increment-autofix.rs} (100%) rename src/test/ui/parser/{increment.stderr => increment-autofix.stderr} (85%) create mode 100644 src/test/ui/parser/increment-notfixed.rs create mode 100644 src/test/ui/parser/increment-notfixed.stderr diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 89cb1c89f8190..38e67790d748e 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -156,10 +156,15 @@ impl AttemptLocalParseRecovery { } } +/// Information for emitting suggestions and recovering from +/// C-style `i++`, `--i`, etc. #[derive(Debug, Copy, Clone)] struct IncDecRecovery { + /// This increment/decrement is not a subexpression. standalone: bool, + /// Is this an increment or decrement? op: IncOrDec, + /// Is this pre- or postfix? fixity: UnaryFixity, } @@ -1278,20 +1283,16 @@ impl<'a> Parser<'a> { } if kind.standalone { - self.inc_dec_standalone_recovery(base, err, kind, ident, spans) + self.inc_dec_standalone_recovery(base, err, kind, spans) } else { match kind.fixity { - UnaryFixity::Pre => { - self.prefix_inc_dec_suggest_and_recover(base, err, kind, ident, spans) - } - UnaryFixity::Post => { - self.postfix_inc_dec_suggest_and_recover(base, err, kind, ident, spans) - } + UnaryFixity::Pre => self.prefix_inc_dec_suggest(base, err, kind, ident, spans), + UnaryFixity::Post => self.postfix_inc_dec_suggest(base, err, kind, ident, spans), } } } - fn prefix_inc_dec_suggest_and_recover( + fn prefix_inc_dec_suggest( &mut self, _base: P, mut err: DiagnosticBuilder<'a>, @@ -1310,7 +1311,7 @@ impl<'a> Parser<'a> { Err(err) } - fn postfix_inc_dec_suggest_and_recover( + fn postfix_inc_dec_suggest( &mut self, _base: P, mut err: DiagnosticBuilder<'a>, @@ -1334,7 +1335,6 @@ impl<'a> Parser<'a> { _base: P, mut err: DiagnosticBuilder<'a>, kind: IncDecRecovery, - _ident: Ident, (pre_span, post_span): (Span, Span), ) -> PResult<'a, P> { err.multipart_suggestion( diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 39d96b8a9e326..34ccd167e4ec1 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -592,13 +592,7 @@ impl<'a> Parser<'a> { this.bump(); this.parse_prefix_expr(None) } // `+expr` - token::Ident(..) if this.token.is_keyword(kw::Box) => { - make_it!(this, attrs, |this, _| this.parse_box_expr(lo)) - } - token::Ident(..) if this.is_mistaken_not_ident_negation() => { - make_it!(this, attrs, |this, _| this.recover_not_expr(lo)) - } - // Recover from `++x` + // Recover from `++x`: token::BinOp(token::Plus) if this.look_ahead(1, |t| *t == token::BinOp(token::Plus)) => { @@ -608,9 +602,15 @@ impl<'a> Parser<'a> { this.bump(); this.bump(); - let operand_expr = this.parse_path_start_expr(Default::default())?; + let operand_expr = this.parse_dot_or_call_expr(Default::default())?; this.maybe_recover_from_prefix_increment(operand_expr, pre_span, prev_is_semi) } + token::Ident(..) if this.token.is_keyword(kw::Box) => { + make_it!(this, attrs, |this, _| this.parse_box_expr(lo)) + } + token::Ident(..) if this.is_mistaken_not_ident_negation() => { + make_it!(this, attrs, |this, _| this.recover_not_expr(lo)) + } _ => return this.parse_dot_or_call_expr(Some(attrs)), } } diff --git a/src/test/ui/parser/increment.fixed b/src/test/ui/parser/increment-autofix.fixed similarity index 100% rename from src/test/ui/parser/increment.fixed rename to src/test/ui/parser/increment-autofix.fixed diff --git a/src/test/ui/parser/increment.rs b/src/test/ui/parser/increment-autofix.rs similarity index 100% rename from src/test/ui/parser/increment.rs rename to src/test/ui/parser/increment-autofix.rs diff --git a/src/test/ui/parser/increment.stderr b/src/test/ui/parser/increment-autofix.stderr similarity index 85% rename from src/test/ui/parser/increment.stderr rename to src/test/ui/parser/increment-autofix.stderr index 6a2b37e3ddcf2..46ab48f36843b 100644 --- a/src/test/ui/parser/increment.stderr +++ b/src/test/ui/parser/increment-autofix.stderr @@ -1,5 +1,5 @@ error: Rust has no postfix increment operator - --> $DIR/increment.rs:5:6 + --> $DIR/increment-autofix.rs:5:6 | LL | i++; | ^^ not a valid postfix operator @@ -10,7 +10,7 @@ LL | { let tmp = i; i += 1; tmp }; | +++++++++++ ~~~~~~~~~~~~~~~ error: Rust has no postfix increment operator - --> $DIR/increment.rs:11:12 + --> $DIR/increment-autofix.rs:11:12 | LL | while i++ < 5 { | ^^ not a valid postfix operator @@ -21,7 +21,7 @@ LL | while { let tmp = i; i += 1; tmp } < 5 { | +++++++++++ ~~~~~~~~~~~~~~~ error: Rust has no prefix increment operator - --> $DIR/increment.rs:19:5 + --> $DIR/increment-autofix.rs:19:5 | LL | ++i; | ^^ not a valid prefix operator @@ -33,7 +33,7 @@ LL + i += 1; | error: Rust has no prefix increment operator - --> $DIR/increment.rs:25:11 + --> $DIR/increment-autofix.rs:25:11 | LL | while ++i < 5 { | ^^ not a valid prefix operator diff --git a/src/test/ui/parser/increment-notfixed.rs b/src/test/ui/parser/increment-notfixed.rs new file mode 100644 index 0000000000000..d0efe95298234 --- /dev/null +++ b/src/test/ui/parser/increment-notfixed.rs @@ -0,0 +1,26 @@ +struct Foo { + bar: Bar, +} + +struct Bar { + qux: i32, +} + +fn post_field() { + let foo = Foo { bar: Bar { qux: 0 } }; + foo.bar.qux++; + //~^ ERROR Rust has no postfix increment operator + println!("{}", foo.bar.qux); +} + +fn pre_field() { + let foo = Foo { bar: Bar { qux: 0 } }; + ++foo.bar.qux; + //~^ ERROR Rust has no prefix increment operator + println!("{}", foo.bar.qux); +} + +fn main() { + post_field(); + pre_field(); +} diff --git a/src/test/ui/parser/increment-notfixed.stderr b/src/test/ui/parser/increment-notfixed.stderr new file mode 100644 index 0000000000000..cf60075d00cde --- /dev/null +++ b/src/test/ui/parser/increment-notfixed.stderr @@ -0,0 +1,18 @@ +error: Rust has no postfix increment operator + --> $DIR/increment-notfixed.rs:11:16 + | +LL | foo.bar.qux++; + | ^^ not a valid postfix operator + | + = help: use `+= 1` instead + +error: Rust has no prefix increment operator + --> $DIR/increment-notfixed.rs:18:5 + | +LL | ++foo.bar.qux; + | ^^ not a valid prefix operator + | + = help: use `+= 1` instead + +error: aborting due to 2 previous errors + From 7287f929b9c4a684be6ea8980970de7693eef841 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 17 Feb 2022 14:19:59 -0800 Subject: [PATCH 06/16] Emit structured suggestions for field accesses too --- .../rustc_parse/src/parser/diagnostics.rs | 33 +++++++------------ src/test/ui/parser/increment-notfixed.stderr | 11 +++++-- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 38e67790d748e..a8a1842f3771d 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1266,45 +1266,36 @@ impl<'a> Parser<'a> { Ok(base) }; - let ExprKind::Path(_, path) = &base.kind - else { return help_base_case(err, base) }; - let [segment] = path.segments.as_slice() - else { return help_base_case(err, base) }; - 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), + UnaryFixity::Pre => (op_span, base.span.shrink_to_hi()), + UnaryFixity::Post => (base.span.shrink_to_lo(), op_span), }; - if ident.is_reserved() { - return help_base_case(err, base); - } - if kind.standalone { - self.inc_dec_standalone_recovery(base, err, kind, spans) + self.inc_dec_standalone_recovery(err, kind, spans) } else { + let Ok(base_src) = self.span_to_snippet(base.span) + else { return help_base_case(err, base) }; match kind.fixity { - UnaryFixity::Pre => self.prefix_inc_dec_suggest(base, err, kind, ident, spans), - UnaryFixity::Post => self.postfix_inc_dec_suggest(base, err, kind, ident, spans), + UnaryFixity::Pre => self.prefix_inc_dec_suggest(base_src, err, kind, spans), + UnaryFixity::Post => self.postfix_inc_dec_suggest(base_src, err, kind, spans), } } } fn prefix_inc_dec_suggest( &mut self, - _base: P, + base_src: String, mut err: DiagnosticBuilder<'a>, 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)), + (post_span, format!(" {}= 1; {} }}", kind.op.chr(), base_src)), ], Applicability::MachineApplicable, ); @@ -1313,17 +1304,16 @@ impl<'a> Parser<'a> { fn postfix_inc_dec_suggest( &mut self, - _base: P, + base_src: String, mut err: DiagnosticBuilder<'a>, 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())), + (post_span, format!("; {} {}= 1; tmp }}", base_src, kind.op.chr())), ], Applicability::MachineApplicable, ); @@ -1332,7 +1322,6 @@ impl<'a> Parser<'a> { fn inc_dec_standalone_recovery( &mut self, - _base: P, mut err: DiagnosticBuilder<'a>, kind: IncDecRecovery, (pre_span, post_span): (Span, Span), diff --git a/src/test/ui/parser/increment-notfixed.stderr b/src/test/ui/parser/increment-notfixed.stderr index cf60075d00cde..3110e24271235 100644 --- a/src/test/ui/parser/increment-notfixed.stderr +++ b/src/test/ui/parser/increment-notfixed.stderr @@ -4,7 +4,10 @@ error: Rust has no postfix increment operator LL | foo.bar.qux++; | ^^ not a valid postfix operator | - = help: use `+= 1` instead +help: use `+= 1` instead + | +LL | { let tmp = foo.bar.qux; foo.bar.qux += 1; tmp }; + | +++++++++++ ~~~~~~~~~~~~~~~~~~~~~~~~~ error: Rust has no prefix increment operator --> $DIR/increment-notfixed.rs:18:5 @@ -12,7 +15,11 @@ error: Rust has no prefix increment operator LL | ++foo.bar.qux; | ^^ not a valid prefix operator | - = help: use `+= 1` instead +help: use `+= 1` instead + | +LL - ++foo.bar.qux; +LL + foo.bar.qux += 1; + | error: aborting due to 2 previous errors From 62b8ea67b79de77fca36ca82de4b934307b6de30 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 17 Feb 2022 14:52:52 -0800 Subject: [PATCH 07/16] Emit both subexp and standalone sugg for postfix This solves the TODO. --- .../rustc_parse/src/parser/diagnostics.rs | 82 ++++++++++++------- compiler/rustc_parse/src/parser/expr.rs | 4 +- src/test/ui/parser/increment-autofix.stderr | 10 +++ src/test/ui/parser/increment-notfixed.stderr | 5 ++ 4 files changed, 70 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index a8a1842f3771d..26ca018685e01 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -160,8 +160,10 @@ impl AttemptLocalParseRecovery { /// C-style `i++`, `--i`, etc. #[derive(Debug, Copy, Clone)] struct IncDecRecovery { - /// This increment/decrement is not a subexpression. - standalone: bool, + /// Is this increment/decrement its own statement? + /// + /// This is `None` when we are unsure. + standalone: Option, /// Is this an increment or decrement? op: IncOrDec, /// Is this pre- or postfix? @@ -1225,7 +1227,7 @@ impl<'a> Parser<'a> { prev_is_semi: bool, ) -> PResult<'a, P> { let kind = IncDecRecovery { - standalone: prev_is_semi, + standalone: Some(prev_is_semi), op: IncOrDec::Inc, fixity: UnaryFixity::Pre, }; @@ -1237,13 +1239,9 @@ impl<'a> Parser<'a> { &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, - }; + let kind = + IncDecRecovery { standalone: None, op: IncOrDec::Inc, fixity: UnaryFixity::Post }; self.recover_from_inc_dec(operand_expr, kind, op_span) } @@ -1272,25 +1270,44 @@ impl<'a> Parser<'a> { UnaryFixity::Post => (base.span.shrink_to_lo(), op_span), }; - if kind.standalone { - self.inc_dec_standalone_recovery(err, kind, spans) - } else { - let Ok(base_src) = self.span_to_snippet(base.span) - else { return help_base_case(err, base) }; - match kind.fixity { - UnaryFixity::Pre => self.prefix_inc_dec_suggest(base_src, err, kind, spans), - UnaryFixity::Post => self.postfix_inc_dec_suggest(base_src, err, kind, spans), + match kind.standalone { + Some(true) => self.inc_dec_standalone_recovery(&mut err, kind, spans, false), + Some(false) => { + let Ok(base_src) = self.span_to_snippet(base.span) + else { return help_base_case(err, base) }; + match kind.fixity { + UnaryFixity::Pre => { + self.prefix_inc_dec_suggest(base_src, &mut err, kind, spans) + } + UnaryFixity::Post => { + self.postfix_inc_dec_suggest(base_src, &mut err, kind, spans) + } + } + } + None => { + let Ok(base_src) = self.span_to_snippet(base.span) + else { return help_base_case(err, base) }; + match kind.fixity { + UnaryFixity::Pre => { + self.prefix_inc_dec_suggest(base_src, &mut err, kind, spans) + } + UnaryFixity::Post => { + self.postfix_inc_dec_suggest(base_src, &mut err, kind, spans) + } + } + self.inc_dec_standalone_recovery(&mut err, kind, spans, true) } } + Err(err) } fn prefix_inc_dec_suggest( &mut self, base_src: String, - mut err: DiagnosticBuilder<'a>, + err: &mut DiagnosticBuilder<'a>, kind: IncDecRecovery, (pre_span, post_span): (Span, Span), - ) -> PResult<'a, P> { + ) { err.multipart_suggestion( &format!("use `{}= 1` instead", kind.op.chr()), vec![ @@ -1299,16 +1316,15 @@ impl<'a> Parser<'a> { ], Applicability::MachineApplicable, ); - Err(err) } fn postfix_inc_dec_suggest( &mut self, base_src: String, - mut err: DiagnosticBuilder<'a>, + err: &mut DiagnosticBuilder<'a>, kind: IncDecRecovery, (pre_span, post_span): (Span, Span), - ) -> PResult<'a, P> { + ) { err.multipart_suggestion( &format!("use `{}= 1` instead", kind.op.chr()), vec![ @@ -1317,21 +1333,31 @@ impl<'a> Parser<'a> { ], Applicability::MachineApplicable, ); - Err(err) } fn inc_dec_standalone_recovery( &mut self, - mut err: DiagnosticBuilder<'a>, + err: &mut DiagnosticBuilder<'a>, kind: IncDecRecovery, (pre_span, post_span): (Span, Span), - ) -> PResult<'a, P> { + maybe_not_standalone: bool, + ) { + let msg = if maybe_not_standalone { + "or, if you don't need to use it as an expression, change it to this".to_owned() + } else { + format!("use `{}= 1` instead", kind.op.chr()) + }; + let applicability = if maybe_not_standalone { + // FIXME: Unspecified isn't right, but it's the least wrong option + Applicability::Unspecified + } else { + Applicability::MachineApplicable + }; err.multipart_suggestion( - &format!("use `{}= 1` instead", kind.op.chr()), + &msg, vec![(pre_span, String::new()), (post_span, format!(" {}= 1", kind.op.chr()))], - Applicability::MachineApplicable, + applicability, ); - Err(err) } /// Tries to recover from associated item paths like `[T]::AssocItem` / `(T, U)::AssocItem`. diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 34ccd167e4ec1..c9864fb9fa313 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -273,9 +273,7 @@ impl<'a> Parser<'a> { 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)?; + lhs = self.maybe_recover_from_postfix_increment(lhs, op_span)?; continue; } diff --git a/src/test/ui/parser/increment-autofix.stderr b/src/test/ui/parser/increment-autofix.stderr index 46ab48f36843b..e5386c7bdbaa6 100644 --- a/src/test/ui/parser/increment-autofix.stderr +++ b/src/test/ui/parser/increment-autofix.stderr @@ -8,6 +8,11 @@ help: use `+= 1` instead | LL | { let tmp = i; i += 1; tmp }; | +++++++++++ ~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this + | +LL - i++; +LL + i += 1; + | error: Rust has no postfix increment operator --> $DIR/increment-autofix.rs:11:12 @@ -19,6 +24,11 @@ help: use `+= 1` instead | LL | while { let tmp = i; i += 1; tmp } < 5 { | +++++++++++ ~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this + | +LL - while i++ < 5 { +LL + while i += 1 < 5 { + | error: Rust has no prefix increment operator --> $DIR/increment-autofix.rs:19:5 diff --git a/src/test/ui/parser/increment-notfixed.stderr b/src/test/ui/parser/increment-notfixed.stderr index 3110e24271235..43586c4c25efb 100644 --- a/src/test/ui/parser/increment-notfixed.stderr +++ b/src/test/ui/parser/increment-notfixed.stderr @@ -8,6 +8,11 @@ help: use `+= 1` instead | LL | { let tmp = foo.bar.qux; foo.bar.qux += 1; tmp }; | +++++++++++ ~~~~~~~~~~~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this + | +LL - foo.bar.qux++; +LL + foo.bar.qux += 1; + | error: Rust has no prefix increment operator --> $DIR/increment-notfixed.rs:18:5 From 073010d425d7b6ff5e79fd41f209e4915daa6066 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 17 Feb 2022 14:58:46 -0800 Subject: [PATCH 08/16] Improve handling of `tmp` variable name conflicts --- .../rustc_parse/src/parser/diagnostics.rs | 8 +++- src/test/ui/parser/increment-autofix.rs | 16 ++++++++ src/test/ui/parser/increment-autofix.stderr | 38 +++++++++++++++++-- src/test/ui/parser/increment-notfixed.rs | 11 ++++++ src/test/ui/parser/increment-notfixed.stderr | 20 +++++++++- 5 files changed, 86 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 26ca018685e01..dcb150f18aa42 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1325,13 +1325,17 @@ impl<'a> Parser<'a> { kind: IncDecRecovery, (pre_span, post_span): (Span, Span), ) { + let mut msg = format!("use `{}= 1` instead", kind.op.chr()); + if base_src.trim() == "tmp" { + msg.push_str(" (rename `tmp` so it doesn't conflict with your variable)"); + } err.multipart_suggestion( - &format!("use `{}= 1` instead", kind.op.chr()), + &msg, vec![ (pre_span, "{ let tmp = ".to_string()), (post_span, format!("; {} {}= 1; tmp }}", base_src, kind.op.chr())), ], - Applicability::MachineApplicable, + Applicability::HasPlaceholders, ); } diff --git a/src/test/ui/parser/increment-autofix.rs b/src/test/ui/parser/increment-autofix.rs index f31031fed3aff..909c8f8c371ea 100644 --- a/src/test/ui/parser/increment-autofix.rs +++ b/src/test/ui/parser/increment-autofix.rs @@ -14,6 +14,20 @@ fn post_while() { } } +fn post_regular_tmp() { + let mut tmp = 0; + tmp++; //~ ERROR Rust has no postfix increment operator + println!("{}", tmp); +} + +fn post_while_tmp() { + let mut tmp = 0; + while tmp++ < 5 { + //~^ ERROR Rust has no postfix increment operator + println!("{}", tmp); + } +} + fn pre_regular() { let mut i = 0; ++i; //~ ERROR Rust has no prefix increment operator @@ -31,6 +45,8 @@ fn pre_while() { fn main() { post_regular(); post_while(); + post_regular_tmp(); + post_while_tmp(); pre_regular(); pre_while(); } diff --git a/src/test/ui/parser/increment-autofix.stderr b/src/test/ui/parser/increment-autofix.stderr index e5386c7bdbaa6..8c934c9efde35 100644 --- a/src/test/ui/parser/increment-autofix.stderr +++ b/src/test/ui/parser/increment-autofix.stderr @@ -30,8 +30,40 @@ LL - while i++ < 5 { LL + while i += 1 < 5 { | +error: Rust has no postfix increment operator + --> $DIR/increment-autofix.rs:19:8 + | +LL | tmp++; + | ^^ not a valid postfix operator + | +help: use `+= 1` instead (rename `tmp` so it doesn't conflict with your variable) + | +LL | { let tmp = tmp; tmp += 1; tmp }; + | +++++++++++ ~~~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this + | +LL - tmp++; +LL + tmp += 1; + | + +error: Rust has no postfix increment operator + --> $DIR/increment-autofix.rs:25:14 + | +LL | while tmp++ < 5 { + | ^^ not a valid postfix operator + | +help: use `+= 1` instead (rename `tmp` so it doesn't conflict with your variable) + | +LL | while { let tmp = tmp; tmp += 1; tmp } < 5 { + | +++++++++++ ~~~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this + | +LL - while tmp++ < 5 { +LL + while tmp += 1 < 5 { + | + error: Rust has no prefix increment operator - --> $DIR/increment-autofix.rs:19:5 + --> $DIR/increment-autofix.rs:33:5 | LL | ++i; | ^^ not a valid prefix operator @@ -43,7 +75,7 @@ LL + i += 1; | error: Rust has no prefix increment operator - --> $DIR/increment-autofix.rs:25:11 + --> $DIR/increment-autofix.rs:39:11 | LL | while ++i < 5 { | ^^ not a valid prefix operator @@ -53,5 +85,5 @@ help: use `+= 1` instead LL | while { i += 1; i } < 5 { | ~ +++++++++ -error: aborting due to 4 previous errors +error: aborting due to 6 previous errors diff --git a/src/test/ui/parser/increment-notfixed.rs b/src/test/ui/parser/increment-notfixed.rs index d0efe95298234..3db8a4c3326f3 100644 --- a/src/test/ui/parser/increment-notfixed.rs +++ b/src/test/ui/parser/increment-notfixed.rs @@ -13,6 +13,16 @@ fn post_field() { println!("{}", foo.bar.qux); } +fn post_field_tmp() { + struct S { + tmp: i32 + } + let s = S { tmp: 0 }; + s.tmp++; + //~^ ERROR Rust has no postfix increment operator + println!("{}", s.tmp); +} + fn pre_field() { let foo = Foo { bar: Bar { qux: 0 } }; ++foo.bar.qux; @@ -22,5 +32,6 @@ fn pre_field() { fn main() { post_field(); + post_field_tmp(); pre_field(); } diff --git a/src/test/ui/parser/increment-notfixed.stderr b/src/test/ui/parser/increment-notfixed.stderr index 43586c4c25efb..c4c83b5113b4d 100644 --- a/src/test/ui/parser/increment-notfixed.stderr +++ b/src/test/ui/parser/increment-notfixed.stderr @@ -14,8 +14,24 @@ LL - foo.bar.qux++; LL + foo.bar.qux += 1; | +error: Rust has no postfix increment operator + --> $DIR/increment-notfixed.rs:21:10 + | +LL | s.tmp++; + | ^^ not a valid postfix operator + | +help: use `+= 1` instead + | +LL | { let tmp = s.tmp; s.tmp += 1; tmp }; + | +++++++++++ ~~~~~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this + | +LL - s.tmp++; +LL + s.tmp += 1; + | + error: Rust has no prefix increment operator - --> $DIR/increment-notfixed.rs:18:5 + --> $DIR/increment-notfixed.rs:28:5 | LL | ++foo.bar.qux; | ^^ not a valid prefix operator @@ -26,5 +42,5 @@ LL - ++foo.bar.qux; LL + foo.bar.qux += 1; | -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors From 713a11b9197d2b4d1a02fabfe04fe6446c5da292 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 17 Feb 2022 16:22:45 -0800 Subject: [PATCH 09/16] Bless tests --- src/test/ui/associated-types/issue-36499.stderr | 14 +++++++++----- .../ui/parser/issues/issue-88276-unary-plus.stderr | 14 +++++++++----- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/test/ui/associated-types/issue-36499.stderr b/src/test/ui/associated-types/issue-36499.stderr index 610798d880f0a..b49a34be98f3d 100644 --- a/src/test/ui/associated-types/issue-36499.stderr +++ b/src/test/ui/associated-types/issue-36499.stderr @@ -1,13 +1,17 @@ -error: leading `+` is not supported - --> $DIR/issue-36499.rs:4:9 +error: Rust has no postfix increment operator + --> $DIR/issue-36499.rs:4:7 | LL | 2 + +2; - | ^ unexpected `+` + | ^^^ not a valid postfix operator | -help: try removing the `+` +help: use `+= 1` instead + | +LL | { let tmp = 2 ; 2 += 1; tmp }2; + | +++++++++++ ~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this | LL - 2 + +2; -LL + 2 + 2; +LL + 2 += 12; | error: aborting due to previous error diff --git a/src/test/ui/parser/issues/issue-88276-unary-plus.stderr b/src/test/ui/parser/issues/issue-88276-unary-plus.stderr index b26761729a837..e65bac2c5cc15 100644 --- a/src/test/ui/parser/issues/issue-88276-unary-plus.stderr +++ b/src/test/ui/parser/issues/issue-88276-unary-plus.stderr @@ -10,16 +10,20 @@ LL - let _ = +1; LL + let _ = 1; | -error: leading `+` is not supported - --> $DIR/issue-88276-unary-plus.rs:5:20 +error: Rust has no postfix increment operator + --> $DIR/issue-88276-unary-plus.rs:5:18 | LL | let _ = (1.0 + +2.0) * +3.0; - | ^ unexpected `+` + | ^^^ not a valid postfix operator | -help: try removing the `+` +help: use `+= 1` instead + | +LL | let _ = ({ let tmp = 1.0 ; 1.0 += 1; tmp }2.0) * +3.0; + | +++++++++++ ~~~~~~~~~~~~~~~~~ +help: or, if you don't need to use it as an expression, change it to this | LL - let _ = (1.0 + +2.0) * +3.0; -LL + let _ = (1.0 + 2.0) * +3.0; +LL + let _ = (1.0 += 12.0) * +3.0; | error: leading `+` is not supported From 29a5c363c7af619c1264c8b1e80241e503b27b47 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 17 Feb 2022 16:24:22 -0800 Subject: [PATCH 10/16] Improve function names --- compiler/rustc_parse/src/parser/diagnostics.rs | 4 ++-- compiler/rustc_parse/src/parser/expr.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index dcb150f18aa42..4f2a308f2e9b7 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1220,7 +1220,7 @@ impl<'a> Parser<'a> { Ok(()) } - pub(super) fn maybe_recover_from_prefix_increment( + pub(super) fn recover_from_prefix_increment( &mut self, operand_expr: P, op_span: Span, @@ -1235,7 +1235,7 @@ impl<'a> Parser<'a> { self.recover_from_inc_dec(operand_expr, kind, op_span) } - pub(super) fn maybe_recover_from_postfix_increment( + pub(super) fn recover_from_postfix_increment( &mut self, operand_expr: P, op_span: Span, diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index c9864fb9fa313..8ac29c2b5f60e 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -273,7 +273,7 @@ impl<'a> Parser<'a> { let op_span = self.prev_token.span.to(self.token.span); // Eat the second `+` self.bump(); - lhs = self.maybe_recover_from_postfix_increment(lhs, op_span)?; + lhs = self.recover_from_postfix_increment(lhs, op_span)?; continue; } @@ -601,7 +601,7 @@ impl<'a> Parser<'a> { this.bump(); let operand_expr = this.parse_dot_or_call_expr(Default::default())?; - this.maybe_recover_from_prefix_increment(operand_expr, pre_span, prev_is_semi) + this.recover_from_prefix_increment(operand_expr, pre_span, prev_is_semi) } token::Ident(..) if this.token.is_keyword(kw::Box) => { make_it!(this, attrs, |this, _| this.parse_box_expr(lo)) From 4212835d993537158aa39406a489351a4efdda71 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Thu, 17 Feb 2022 16:30:48 -0800 Subject: [PATCH 11/16] Add heuristic to avoid treating `x + +2` as increment --- compiler/rustc_parse/src/parser/expr.rs | 1 + src/test/ui/associated-types/issue-36499.stderr | 14 +++++--------- .../ui/parser/issues/issue-88276-unary-plus.stderr | 14 +++++--------- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 8ac29c2b5f60e..8c4ec8dc6e362 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -269,6 +269,7 @@ impl<'a> Parser<'a> { if self.prev_token == token::BinOp(token::Plus) && self.token == token::BinOp(token::Plus) + && self.prev_token.span.between(self.token.span).is_empty() { let op_span = self.prev_token.span.to(self.token.span); // Eat the second `+` diff --git a/src/test/ui/associated-types/issue-36499.stderr b/src/test/ui/associated-types/issue-36499.stderr index b49a34be98f3d..610798d880f0a 100644 --- a/src/test/ui/associated-types/issue-36499.stderr +++ b/src/test/ui/associated-types/issue-36499.stderr @@ -1,17 +1,13 @@ -error: Rust has no postfix increment operator - --> $DIR/issue-36499.rs:4:7 +error: leading `+` is not supported + --> $DIR/issue-36499.rs:4:9 | LL | 2 + +2; - | ^^^ not a valid postfix operator + | ^ unexpected `+` | -help: use `+= 1` instead - | -LL | { let tmp = 2 ; 2 += 1; tmp }2; - | +++++++++++ ~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this +help: try removing the `+` | LL - 2 + +2; -LL + 2 += 12; +LL + 2 + 2; | error: aborting due to previous error diff --git a/src/test/ui/parser/issues/issue-88276-unary-plus.stderr b/src/test/ui/parser/issues/issue-88276-unary-plus.stderr index e65bac2c5cc15..b26761729a837 100644 --- a/src/test/ui/parser/issues/issue-88276-unary-plus.stderr +++ b/src/test/ui/parser/issues/issue-88276-unary-plus.stderr @@ -10,20 +10,16 @@ LL - let _ = +1; LL + let _ = 1; | -error: Rust has no postfix increment operator - --> $DIR/issue-88276-unary-plus.rs:5:18 +error: leading `+` is not supported + --> $DIR/issue-88276-unary-plus.rs:5:20 | LL | let _ = (1.0 + +2.0) * +3.0; - | ^^^ not a valid postfix operator - | -help: use `+= 1` instead + | ^ unexpected `+` | -LL | let _ = ({ let tmp = 1.0 ; 1.0 += 1; tmp }2.0) * +3.0; - | +++++++++++ ~~~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this +help: try removing the `+` | LL - let _ = (1.0 + +2.0) * +3.0; -LL + let _ = (1.0 += 12.0) * +3.0; +LL + let _ = (1.0 + 2.0) * +3.0; | error: leading `+` is not supported From 95960b7d54ecd05b03734cadea071cd5cc29fa35 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 18 Feb 2022 15:27:58 -0800 Subject: [PATCH 12/16] Make `standalone` an enum --- .../rustc_parse/src/parser/diagnostics.rs | 38 ++++++++++++------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 4f2a308f2e9b7..162982ebbe208 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -161,15 +161,24 @@ impl AttemptLocalParseRecovery { #[derive(Debug, Copy, Clone)] struct IncDecRecovery { /// Is this increment/decrement its own statement? - /// - /// This is `None` when we are unsure. - standalone: Option, + standalone: IsStandalone, /// Is this an increment or decrement? op: IncOrDec, /// Is this pre- or postfix? fixity: UnaryFixity, } +/// Is an increment or decrement expression its own statement? +#[derive(Debug, Copy, Clone)] +enum IsStandalone { + /// It's standalone, i.e., its own statement. + Standalone, + /// It's a subexpression, i.e., *not* standalone. + Subexpr, + /// It's maybe standalone; we're not sure. + Maybe, +} + #[derive(Debug, Copy, Clone, PartialEq, Eq)] enum IncOrDec { Inc, @@ -1226,11 +1235,9 @@ impl<'a> Parser<'a> { op_span: Span, prev_is_semi: bool, ) -> PResult<'a, P> { - let kind = IncDecRecovery { - standalone: Some(prev_is_semi), - op: IncOrDec::Inc, - fixity: UnaryFixity::Pre, - }; + let standalone = + if prev_is_semi { IsStandalone::Standalone } else { IsStandalone::Subexpr }; + let kind = IncDecRecovery { standalone, op: IncOrDec::Inc, fixity: UnaryFixity::Pre }; self.recover_from_inc_dec(operand_expr, kind, op_span) } @@ -1240,8 +1247,11 @@ impl<'a> Parser<'a> { operand_expr: P, op_span: Span, ) -> PResult<'a, P> { - let kind = - IncDecRecovery { standalone: None, op: IncOrDec::Inc, fixity: UnaryFixity::Post }; + let kind = IncDecRecovery { + standalone: IsStandalone::Maybe, + op: IncOrDec::Inc, + fixity: UnaryFixity::Post, + }; self.recover_from_inc_dec(operand_expr, kind, op_span) } @@ -1271,8 +1281,10 @@ impl<'a> Parser<'a> { }; match kind.standalone { - Some(true) => self.inc_dec_standalone_recovery(&mut err, kind, spans, false), - Some(false) => { + IsStandalone::Standalone => { + self.inc_dec_standalone_recovery(&mut err, kind, spans, false) + } + IsStandalone::Subexpr => { let Ok(base_src) = self.span_to_snippet(base.span) else { return help_base_case(err, base) }; match kind.fixity { @@ -1284,7 +1296,7 @@ impl<'a> Parser<'a> { } } } - None => { + IsStandalone::Maybe => { let Ok(base_src) = self.span_to_snippet(base.span) else { return help_base_case(err, base) }; match kind.fixity { From ef74796178edd2cf28f17109083cbd235808f88d Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Fri, 18 Feb 2022 15:32:46 -0800 Subject: [PATCH 13/16] Change temporary variable name if it would conflict --- compiler/rustc_parse/src/parser/diagnostics.rs | 11 ++++------- src/test/ui/parser/increment-autofix.stderr | 12 ++++++------ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 162982ebbe208..9590f26a76f1b 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1337,15 +1337,12 @@ impl<'a> Parser<'a> { kind: IncDecRecovery, (pre_span, post_span): (Span, Span), ) { - let mut msg = format!("use `{}= 1` instead", kind.op.chr()); - if base_src.trim() == "tmp" { - msg.push_str(" (rename `tmp` so it doesn't conflict with your variable)"); - } + let tmp_var = if base_src.trim() == "tmp" { "tmp_" } else { "tmp" }; err.multipart_suggestion( - &msg, + &format!("use `{}= 1` instead", kind.op.chr()), vec![ - (pre_span, "{ let tmp = ".to_string()), - (post_span, format!("; {} {}= 1; tmp }}", base_src, kind.op.chr())), + (pre_span, format!("{{ let {} = ", tmp_var)), + (post_span, format!("; {} {}= 1; {} }}", base_src, kind.op.chr(), tmp_var)), ], Applicability::HasPlaceholders, ); diff --git a/src/test/ui/parser/increment-autofix.stderr b/src/test/ui/parser/increment-autofix.stderr index 8c934c9efde35..04fd68ed85bf7 100644 --- a/src/test/ui/parser/increment-autofix.stderr +++ b/src/test/ui/parser/increment-autofix.stderr @@ -36,10 +36,10 @@ error: Rust has no postfix increment operator LL | tmp++; | ^^ not a valid postfix operator | -help: use `+= 1` instead (rename `tmp` so it doesn't conflict with your variable) +help: use `+= 1` instead | -LL | { let tmp = tmp; tmp += 1; tmp }; - | +++++++++++ ~~~~~~~~~~~~~~~~~ +LL | { let tmp_ = tmp; tmp += 1; tmp_ }; + | ++++++++++++ ~~~~~~~~~~~~~~~~~~ help: or, if you don't need to use it as an expression, change it to this | LL - tmp++; @@ -52,10 +52,10 @@ error: Rust has no postfix increment operator LL | while tmp++ < 5 { | ^^ not a valid postfix operator | -help: use `+= 1` instead (rename `tmp` so it doesn't conflict with your variable) +help: use `+= 1` instead | -LL | while { let tmp = tmp; tmp += 1; tmp } < 5 { - | +++++++++++ ~~~~~~~~~~~~~~~~~ +LL | while { let tmp_ = tmp; tmp += 1; tmp_ } < 5 { + | ++++++++++++ ~~~~~~~~~~~~~~~~~~ help: or, if you don't need to use it as an expression, change it to this | LL - while tmp++ < 5 { From 725cde42d5673d1957ad58c6b2fada28a6a4edbd Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Mon, 21 Feb 2022 18:52:47 -0800 Subject: [PATCH 14/16] Use `multipart_suggestions` This records that the suggestions are mutually-exclusive (i.e., only one should be applied). --- .../rustc_parse/src/parser/diagnostics.rs | 102 +++++++++--------- src/test/ui/parser/increment-autofix.stderr | 8 -- src/test/ui/parser/increment-notfixed.stderr | 4 - 3 files changed, 54 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 9590f26a76f1b..0e0954d6ee2fc 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -218,6 +218,27 @@ impl std::fmt::Display for UnaryFixity { } } +struct MultiSugg { + msg: String, + patches: Vec<(Span, String)>, + applicability: Applicability, +} + +impl MultiSugg { + fn emit(self, err: &mut DiagnosticBuilder<'_>) { + err.multipart_suggestion(&self.msg, self.patches, self.applicability); + } + + /// Overrides individual messages and applicabilities. + fn emit_many( + err: &mut DiagnosticBuilder<'_>, + msg: &str, + applicability: Applicability, + suggestions: impl Iterator, + ) { + err.multipart_suggestions(msg, suggestions.map(|s| s.patches), applicability); + } +} // SnapshotParser is used to create a snapshot of the parser // without causing duplicate errors being emitted when the `Parser` // is dropped. @@ -1281,33 +1302,33 @@ impl<'a> Parser<'a> { }; match kind.standalone { - IsStandalone::Standalone => { - self.inc_dec_standalone_recovery(&mut err, kind, spans, false) - } + IsStandalone::Standalone => self.inc_dec_standalone_suggest(kind, spans).emit(&mut err), IsStandalone::Subexpr => { let Ok(base_src) = self.span_to_snippet(base.span) else { return help_base_case(err, base) }; match kind.fixity { UnaryFixity::Pre => { - self.prefix_inc_dec_suggest(base_src, &mut err, kind, spans) + self.prefix_inc_dec_suggest(base_src, kind, spans).emit(&mut err) } UnaryFixity::Post => { - self.postfix_inc_dec_suggest(base_src, &mut err, kind, spans) + self.postfix_inc_dec_suggest(base_src, kind, spans).emit(&mut err) } } } IsStandalone::Maybe => { let Ok(base_src) = self.span_to_snippet(base.span) else { return help_base_case(err, base) }; - match kind.fixity { - UnaryFixity::Pre => { - self.prefix_inc_dec_suggest(base_src, &mut err, kind, spans) - } - UnaryFixity::Post => { - self.postfix_inc_dec_suggest(base_src, &mut err, kind, spans) - } - } - self.inc_dec_standalone_recovery(&mut err, kind, spans, true) + let sugg1 = match kind.fixity { + UnaryFixity::Pre => self.prefix_inc_dec_suggest(base_src, kind, spans), + UnaryFixity::Post => self.postfix_inc_dec_suggest(base_src, kind, spans), + }; + let sugg2 = self.inc_dec_standalone_suggest(kind, spans); + MultiSugg::emit_many( + &mut err, + "use `+= 1` instead", + Applicability::MaybeIncorrect, + [sugg1, sugg2].into_iter(), + ) } } Err(err) @@ -1316,61 +1337,46 @@ impl<'a> Parser<'a> { fn prefix_inc_dec_suggest( &mut self, base_src: String, - err: &mut DiagnosticBuilder<'a>, kind: IncDecRecovery, (pre_span, post_span): (Span, Span), - ) { - err.multipart_suggestion( - &format!("use `{}= 1` instead", kind.op.chr()), - vec![ + ) -> MultiSugg { + MultiSugg { + msg: format!("use `{}= 1` instead", kind.op.chr()), + patches: vec![ (pre_span, "{ ".to_string()), (post_span, format!(" {}= 1; {} }}", kind.op.chr(), base_src)), ], - Applicability::MachineApplicable, - ); + applicability: Applicability::MachineApplicable, + } } fn postfix_inc_dec_suggest( &mut self, base_src: String, - err: &mut DiagnosticBuilder<'a>, kind: IncDecRecovery, (pre_span, post_span): (Span, Span), - ) { + ) -> MultiSugg { let tmp_var = if base_src.trim() == "tmp" { "tmp_" } else { "tmp" }; - err.multipart_suggestion( - &format!("use `{}= 1` instead", kind.op.chr()), - vec![ + MultiSugg { + msg: format!("use `{}= 1` instead", kind.op.chr()), + patches: vec![ (pre_span, format!("{{ let {} = ", tmp_var)), (post_span, format!("; {} {}= 1; {} }}", base_src, kind.op.chr(), tmp_var)), ], - Applicability::HasPlaceholders, - ); + applicability: Applicability::HasPlaceholders, + } } - fn inc_dec_standalone_recovery( + fn inc_dec_standalone_suggest( &mut self, - err: &mut DiagnosticBuilder<'a>, kind: IncDecRecovery, (pre_span, post_span): (Span, Span), - maybe_not_standalone: bool, - ) { - let msg = if maybe_not_standalone { - "or, if you don't need to use it as an expression, change it to this".to_owned() - } else { - format!("use `{}= 1` instead", kind.op.chr()) - }; - let applicability = if maybe_not_standalone { - // FIXME: Unspecified isn't right, but it's the least wrong option - Applicability::Unspecified - } else { - Applicability::MachineApplicable - }; - err.multipart_suggestion( - &msg, - vec![(pre_span, String::new()), (post_span, format!(" {}= 1", kind.op.chr()))], - applicability, - ); + ) -> MultiSugg { + MultiSugg { + msg: format!("use `{}= 1` instead", kind.op.chr()), + patches: vec![(pre_span, String::new()), (post_span, format!(" {}= 1", kind.op.chr()))], + applicability: Applicability::MachineApplicable, + } } /// Tries to recover from associated item paths like `[T]::AssocItem` / `(T, U)::AssocItem`. diff --git a/src/test/ui/parser/increment-autofix.stderr b/src/test/ui/parser/increment-autofix.stderr index 04fd68ed85bf7..5bf4b2751de3d 100644 --- a/src/test/ui/parser/increment-autofix.stderr +++ b/src/test/ui/parser/increment-autofix.stderr @@ -8,8 +8,6 @@ help: use `+= 1` instead | LL | { let tmp = i; i += 1; tmp }; | +++++++++++ ~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this - | LL - i++; LL + i += 1; | @@ -24,8 +22,6 @@ help: use `+= 1` instead | LL | while { let tmp = i; i += 1; tmp } < 5 { | +++++++++++ ~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this - | LL - while i++ < 5 { LL + while i += 1 < 5 { | @@ -40,8 +36,6 @@ help: use `+= 1` instead | LL | { let tmp_ = tmp; tmp += 1; tmp_ }; | ++++++++++++ ~~~~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this - | LL - tmp++; LL + tmp += 1; | @@ -56,8 +50,6 @@ help: use `+= 1` instead | LL | while { let tmp_ = tmp; tmp += 1; tmp_ } < 5 { | ++++++++++++ ~~~~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this - | LL - while tmp++ < 5 { LL + while tmp += 1 < 5 { | diff --git a/src/test/ui/parser/increment-notfixed.stderr b/src/test/ui/parser/increment-notfixed.stderr index c4c83b5113b4d..16ff42ca8b06f 100644 --- a/src/test/ui/parser/increment-notfixed.stderr +++ b/src/test/ui/parser/increment-notfixed.stderr @@ -8,8 +8,6 @@ help: use `+= 1` instead | LL | { let tmp = foo.bar.qux; foo.bar.qux += 1; tmp }; | +++++++++++ ~~~~~~~~~~~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this - | LL - foo.bar.qux++; LL + foo.bar.qux += 1; | @@ -24,8 +22,6 @@ help: use `+= 1` instead | LL | { let tmp = s.tmp; s.tmp += 1; tmp }; | +++++++++++ ~~~~~~~~~~~~~~~~~~~ -help: or, if you don't need to use it as an expression, change it to this - | LL - s.tmp++; LL + s.tmp += 1; | From 86220d6e517225d13c145fd12acfc96f78358170 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 23 Mar 2022 17:26:59 -0700 Subject: [PATCH 15/16] Fix rustfix panic on test `run-rustfix` applies all suggestions regardless of their Applicability. There's a flag, `rustfix-only-machine-applicable`, that does what it says, but then the produced `.fixed` file would have invalid code from the suggestions that weren't applied. So, I moved the cases of postfix increment, in which case multiple suggestions are given, to the `-notfixed` test, which does not run rustfix. I also changed the Applicability to Unspecified since MaybeIncorrect requires that the code be valid, even if it's incorrect. --- .../rustc_parse/src/parser/diagnostics.rs | 2 +- src/test/ui/parser/increment-autofix.fixed | 33 ++++----- src/test/ui/parser/increment-autofix.rs | 43 +++-------- src/test/ui/parser/increment-autofix.stderr | 73 +++++-------------- src/test/ui/parser/increment-notfixed.rs | 34 +++++++-- src/test/ui/parser/increment-notfixed.stderr | 64 +++++++++++++++- 6 files changed, 135 insertions(+), 114 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index 0e0954d6ee2fc..bab4ac9d08aa5 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -1326,7 +1326,7 @@ impl<'a> Parser<'a> { MultiSugg::emit_many( &mut err, "use `+= 1` instead", - Applicability::MaybeIncorrect, + Applicability::Unspecified, [sugg1, sugg2].into_iter(), ) } diff --git a/src/test/ui/parser/increment-autofix.fixed b/src/test/ui/parser/increment-autofix.fixed index ad61c4e66d28c..24a5ac42ca6d0 100644 --- a/src/test/ui/parser/increment-autofix.fixed +++ b/src/test/ui/parser/increment-autofix.fixed @@ -1,19 +1,5 @@ // 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 @@ -28,9 +14,18 @@ fn pre_while() { } } -fn main() { - post_regular(); - post_while(); - pre_regular(); - pre_while(); +fn pre_regular_tmp() { + let mut tmp = 0; + tmp += 1; //~ ERROR Rust has no prefix increment operator + println!("{}", tmp); } + +fn pre_while_tmp() { + let mut tmp = 0; + while { tmp += 1; tmp } < 5 { + //~^ ERROR Rust has no prefix increment operator + println!("{}", tmp); + } +} + +fn main() {} diff --git a/src/test/ui/parser/increment-autofix.rs b/src/test/ui/parser/increment-autofix.rs index 909c8f8c371ea..b1cfd09e9ec84 100644 --- a/src/test/ui/parser/increment-autofix.rs +++ b/src/test/ui/parser/increment-autofix.rs @@ -1,52 +1,31 @@ // run-rustfix -fn post_regular() { +fn pre_regular() { let mut i = 0; - i++; //~ ERROR Rust has no postfix increment operator + ++i; //~ ERROR Rust has no prefix increment operator println!("{}", i); } -fn post_while() { +fn pre_while() { let mut i = 0; - while i++ < 5 { - //~^ ERROR Rust has no postfix increment operator + while ++i < 5 { + //~^ ERROR Rust has no prefix increment operator println!("{}", i); } } -fn post_regular_tmp() { +fn pre_regular_tmp() { let mut tmp = 0; - tmp++; //~ ERROR Rust has no postfix increment operator + ++tmp; //~ ERROR Rust has no prefix increment operator println!("{}", tmp); } -fn post_while_tmp() { +fn pre_while_tmp() { let mut tmp = 0; - while tmp++ < 5 { - //~^ ERROR Rust has no postfix increment operator - println!("{}", tmp); - } -} - -fn pre_regular() { - let mut i = 0; - ++i; //~ ERROR Rust has no prefix increment operator - println!("{}", i); -} - -fn pre_while() { - let mut i = 0; - while ++i < 5 { + while ++tmp < 5 { //~^ ERROR Rust has no prefix increment operator - println!("{}", i); + println!("{}", tmp); } } -fn main() { - post_regular(); - post_while(); - post_regular_tmp(); - post_while_tmp(); - pre_regular(); - pre_while(); -} +fn main() {} diff --git a/src/test/ui/parser/increment-autofix.stderr b/src/test/ui/parser/increment-autofix.stderr index 5bf4b2751de3d..a9dc70ad0d6b1 100644 --- a/src/test/ui/parser/increment-autofix.stderr +++ b/src/test/ui/parser/increment-autofix.stderr @@ -1,81 +1,48 @@ -error: Rust has no postfix increment operator - --> $DIR/increment-autofix.rs:5:6 +error: Rust has no prefix increment operator + --> $DIR/increment-autofix.rs:5:5 | -LL | i++; - | ^^ not a valid postfix operator +LL | ++i; + | ^^ not a valid prefix operator | help: use `+= 1` instead | -LL | { let tmp = i; i += 1; tmp }; - | +++++++++++ ~~~~~~~~~~~~~~~ -LL - i++; +LL - ++i; LL + i += 1; | -error: Rust has no postfix increment operator - --> $DIR/increment-autofix.rs:11:12 - | -LL | while i++ < 5 { - | ^^ not a valid postfix operator - | -help: use `+= 1` instead - | -LL | while { let tmp = i; i += 1; tmp } < 5 { - | +++++++++++ ~~~~~~~~~~~~~~~ -LL - while i++ < 5 { -LL + while i += 1 < 5 { - | - -error: Rust has no postfix increment operator - --> $DIR/increment-autofix.rs:19:8 - | -LL | tmp++; - | ^^ not a valid postfix operator - | -help: use `+= 1` instead - | -LL | { let tmp_ = tmp; tmp += 1; tmp_ }; - | ++++++++++++ ~~~~~~~~~~~~~~~~~~ -LL - tmp++; -LL + tmp += 1; - | - -error: Rust has no postfix increment operator - --> $DIR/increment-autofix.rs:25:14 +error: Rust has no prefix increment operator + --> $DIR/increment-autofix.rs:11:11 | -LL | while tmp++ < 5 { - | ^^ not a valid postfix operator +LL | while ++i < 5 { + | ^^ not a valid prefix operator | help: use `+= 1` instead | -LL | while { let tmp_ = tmp; tmp += 1; tmp_ } < 5 { - | ++++++++++++ ~~~~~~~~~~~~~~~~~~ -LL - while tmp++ < 5 { -LL + while tmp += 1 < 5 { - | +LL | while { i += 1; i } < 5 { + | ~ +++++++++ error: Rust has no prefix increment operator - --> $DIR/increment-autofix.rs:33:5 + --> $DIR/increment-autofix.rs:19:5 | -LL | ++i; +LL | ++tmp; | ^^ not a valid prefix operator | help: use `+= 1` instead | -LL - ++i; -LL + i += 1; +LL - ++tmp; +LL + tmp += 1; | error: Rust has no prefix increment operator - --> $DIR/increment-autofix.rs:39:11 + --> $DIR/increment-autofix.rs:25:11 | -LL | while ++i < 5 { +LL | while ++tmp < 5 { | ^^ not a valid prefix operator | help: use `+= 1` instead | -LL | while { i += 1; i } < 5 { - | ~ +++++++++ +LL | while { tmp += 1; tmp } < 5 { + | ~ +++++++++++ -error: aborting due to 6 previous errors +error: aborting due to 4 previous errors diff --git a/src/test/ui/parser/increment-notfixed.rs b/src/test/ui/parser/increment-notfixed.rs index 3db8a4c3326f3..543f73bf772e1 100644 --- a/src/test/ui/parser/increment-notfixed.rs +++ b/src/test/ui/parser/increment-notfixed.rs @@ -6,6 +6,34 @@ struct Bar { qux: i32, } +fn post_regular() { + let mut i = 0; + i++; //~ ERROR Rust has no postfix increment operator + println!("{}", i); +} + +fn post_while() { + let mut i = 0; + while i++ < 5 { + //~^ ERROR Rust has no postfix increment operator + println!("{}", i); + } +} + +fn post_regular_tmp() { + let mut tmp = 0; + tmp++; //~ ERROR Rust has no postfix increment operator + println!("{}", tmp); +} + +fn post_while_tmp() { + let mut tmp = 0; + while tmp++ < 5 { + //~^ ERROR Rust has no postfix increment operator + println!("{}", tmp); + } +} + fn post_field() { let foo = Foo { bar: Bar { qux: 0 } }; foo.bar.qux++; @@ -30,8 +58,4 @@ fn pre_field() { println!("{}", foo.bar.qux); } -fn main() { - post_field(); - post_field_tmp(); - pre_field(); -} +fn main() {} diff --git a/src/test/ui/parser/increment-notfixed.stderr b/src/test/ui/parser/increment-notfixed.stderr index 16ff42ca8b06f..75fce91c6efb2 100644 --- a/src/test/ui/parser/increment-notfixed.stderr +++ b/src/test/ui/parser/increment-notfixed.stderr @@ -1,5 +1,61 @@ error: Rust has no postfix increment operator - --> $DIR/increment-notfixed.rs:11:16 + --> $DIR/increment-notfixed.rs:11:6 + | +LL | i++; + | ^^ not a valid postfix operator + | +help: use `+= 1` instead + | +LL | { let tmp = i; i += 1; tmp }; + | +++++++++++ ~~~~~~~~~~~~~~~ +LL - i++; +LL + i += 1; + | + +error: Rust has no postfix increment operator + --> $DIR/increment-notfixed.rs:17:12 + | +LL | while i++ < 5 { + | ^^ not a valid postfix operator + | +help: use `+= 1` instead + | +LL | while { let tmp = i; i += 1; tmp } < 5 { + | +++++++++++ ~~~~~~~~~~~~~~~ +LL - while i++ < 5 { +LL + while i += 1 < 5 { + | + +error: Rust has no postfix increment operator + --> $DIR/increment-notfixed.rs:25:8 + | +LL | tmp++; + | ^^ not a valid postfix operator + | +help: use `+= 1` instead + | +LL | { let tmp_ = tmp; tmp += 1; tmp_ }; + | ++++++++++++ ~~~~~~~~~~~~~~~~~~ +LL - tmp++; +LL + tmp += 1; + | + +error: Rust has no postfix increment operator + --> $DIR/increment-notfixed.rs:31:14 + | +LL | while tmp++ < 5 { + | ^^ not a valid postfix operator + | +help: use `+= 1` instead + | +LL | while { let tmp_ = tmp; tmp += 1; tmp_ } < 5 { + | ++++++++++++ ~~~~~~~~~~~~~~~~~~ +LL - while tmp++ < 5 { +LL + while tmp += 1 < 5 { + | + +error: Rust has no postfix increment operator + --> $DIR/increment-notfixed.rs:39:16 | LL | foo.bar.qux++; | ^^ not a valid postfix operator @@ -13,7 +69,7 @@ LL + foo.bar.qux += 1; | error: Rust has no postfix increment operator - --> $DIR/increment-notfixed.rs:21:10 + --> $DIR/increment-notfixed.rs:49:10 | LL | s.tmp++; | ^^ not a valid postfix operator @@ -27,7 +83,7 @@ LL + s.tmp += 1; | error: Rust has no prefix increment operator - --> $DIR/increment-notfixed.rs:28:5 + --> $DIR/increment-notfixed.rs:56:5 | LL | ++foo.bar.qux; | ^^ not a valid prefix operator @@ -38,5 +94,5 @@ LL - ++foo.bar.qux; LL + foo.bar.qux += 1; | -error: aborting due to 3 previous errors +error: aborting due to 7 previous errors From 4943688e9d0d0f34f2fda188e9a9d2777f90d186 Mon Sep 17 00:00:00 2001 From: Noah Lev Date: Wed, 23 Mar 2022 19:47:45 -0700 Subject: [PATCH 16/16] Fix from rebase I changed the test functions to be `pub` rather than called from a `main` function too, for easier future modification of tests. --- compiler/rustc_parse/src/parser/diagnostics.rs | 10 +++++----- src/test/ui/parser/increment-autofix.fixed | 8 ++++---- src/test/ui/parser/increment-autofix.rs | 8 ++++---- src/test/ui/parser/increment-autofix.stderr | 8 ++++++-- src/test/ui/parser/increment-notfixed.rs | 14 +++++++------- src/test/ui/parser/increment-notfixed.stderr | 8 ++++++-- 6 files changed, 32 insertions(+), 24 deletions(-) diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index bab4ac9d08aa5..9bf672312781e 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -17,7 +17,7 @@ use rustc_ast::{ }; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashSet; -use rustc_errors::{pluralize, struct_span_err, Diagnostic, ErrorGuaranteed}; +use rustc_errors::{pluralize, struct_span_err, Diagnostic, EmissionGuarantee, ErrorGuaranteed}; use rustc_errors::{Applicability, DiagnosticBuilder, Handler, PResult}; use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, Ident}; @@ -225,13 +225,13 @@ struct MultiSugg { } impl MultiSugg { - fn emit(self, err: &mut DiagnosticBuilder<'_>) { + fn emit(self, err: &mut DiagnosticBuilder<'_, G>) { err.multipart_suggestion(&self.msg, self.patches, self.applicability); } /// Overrides individual messages and applicabilities. - fn emit_many( - err: &mut DiagnosticBuilder<'_>, + fn emit_many( + err: &mut DiagnosticBuilder<'_, G>, msg: &str, applicability: Applicability, suggestions: impl Iterator, @@ -1289,7 +1289,7 @@ impl<'a> Parser<'a> { ); err.span_label(op_span, &format!("not a valid {} operator", kind.fixity)); - let help_base_case = |mut err: DiagnosticBuilder<'_>, base| { + let help_base_case = |mut err: DiagnosticBuilder<'_, _>, base| { err.help(&format!("use `{}= 1` instead", kind.op.chr())); err.emit(); Ok(base) diff --git a/src/test/ui/parser/increment-autofix.fixed b/src/test/ui/parser/increment-autofix.fixed index 24a5ac42ca6d0..7a426badfc2d3 100644 --- a/src/test/ui/parser/increment-autofix.fixed +++ b/src/test/ui/parser/increment-autofix.fixed @@ -1,12 +1,12 @@ // run-rustfix -fn pre_regular() { +pub fn pre_regular() { let mut i = 0; i += 1; //~ ERROR Rust has no prefix increment operator println!("{}", i); } -fn pre_while() { +pub fn pre_while() { let mut i = 0; while { i += 1; i } < 5 { //~^ ERROR Rust has no prefix increment operator @@ -14,13 +14,13 @@ fn pre_while() { } } -fn pre_regular_tmp() { +pub fn pre_regular_tmp() { let mut tmp = 0; tmp += 1; //~ ERROR Rust has no prefix increment operator println!("{}", tmp); } -fn pre_while_tmp() { +pub fn pre_while_tmp() { let mut tmp = 0; while { tmp += 1; tmp } < 5 { //~^ ERROR Rust has no prefix increment operator diff --git a/src/test/ui/parser/increment-autofix.rs b/src/test/ui/parser/increment-autofix.rs index b1cfd09e9ec84..d38603697a7a6 100644 --- a/src/test/ui/parser/increment-autofix.rs +++ b/src/test/ui/parser/increment-autofix.rs @@ -1,12 +1,12 @@ // run-rustfix -fn pre_regular() { +pub fn pre_regular() { let mut i = 0; ++i; //~ ERROR Rust has no prefix increment operator println!("{}", i); } -fn pre_while() { +pub fn pre_while() { let mut i = 0; while ++i < 5 { //~^ ERROR Rust has no prefix increment operator @@ -14,13 +14,13 @@ fn pre_while() { } } -fn pre_regular_tmp() { +pub fn pre_regular_tmp() { let mut tmp = 0; ++tmp; //~ ERROR Rust has no prefix increment operator println!("{}", tmp); } -fn pre_while_tmp() { +pub fn pre_while_tmp() { let mut tmp = 0; while ++tmp < 5 { //~^ ERROR Rust has no prefix increment operator diff --git a/src/test/ui/parser/increment-autofix.stderr b/src/test/ui/parser/increment-autofix.stderr index a9dc70ad0d6b1..593592ba4ab74 100644 --- a/src/test/ui/parser/increment-autofix.stderr +++ b/src/test/ui/parser/increment-autofix.stderr @@ -14,7 +14,9 @@ error: Rust has no prefix increment operator --> $DIR/increment-autofix.rs:11:11 | LL | while ++i < 5 { - | ^^ not a valid prefix operator + | ----- ^^ not a valid prefix operator + | | + | while parsing the condition of this `while` expression | help: use `+= 1` instead | @@ -37,7 +39,9 @@ error: Rust has no prefix increment operator --> $DIR/increment-autofix.rs:25:11 | LL | while ++tmp < 5 { - | ^^ not a valid prefix operator + | ----- ^^ not a valid prefix operator + | | + | while parsing the condition of this `while` expression | help: use `+= 1` instead | diff --git a/src/test/ui/parser/increment-notfixed.rs b/src/test/ui/parser/increment-notfixed.rs index 543f73bf772e1..15f159e53d294 100644 --- a/src/test/ui/parser/increment-notfixed.rs +++ b/src/test/ui/parser/increment-notfixed.rs @@ -6,13 +6,13 @@ struct Bar { qux: i32, } -fn post_regular() { +pub fn post_regular() { let mut i = 0; i++; //~ ERROR Rust has no postfix increment operator println!("{}", i); } -fn post_while() { +pub fn post_while() { let mut i = 0; while i++ < 5 { //~^ ERROR Rust has no postfix increment operator @@ -20,13 +20,13 @@ fn post_while() { } } -fn post_regular_tmp() { +pub fn post_regular_tmp() { let mut tmp = 0; tmp++; //~ ERROR Rust has no postfix increment operator println!("{}", tmp); } -fn post_while_tmp() { +pub fn post_while_tmp() { let mut tmp = 0; while tmp++ < 5 { //~^ ERROR Rust has no postfix increment operator @@ -34,14 +34,14 @@ fn post_while_tmp() { } } -fn post_field() { +pub fn post_field() { let foo = Foo { bar: Bar { qux: 0 } }; foo.bar.qux++; //~^ ERROR Rust has no postfix increment operator println!("{}", foo.bar.qux); } -fn post_field_tmp() { +pub fn post_field_tmp() { struct S { tmp: i32 } @@ -51,7 +51,7 @@ fn post_field_tmp() { println!("{}", s.tmp); } -fn pre_field() { +pub fn pre_field() { let foo = Foo { bar: Bar { qux: 0 } }; ++foo.bar.qux; //~^ ERROR Rust has no prefix increment operator diff --git a/src/test/ui/parser/increment-notfixed.stderr b/src/test/ui/parser/increment-notfixed.stderr index 75fce91c6efb2..f23595da32ac8 100644 --- a/src/test/ui/parser/increment-notfixed.stderr +++ b/src/test/ui/parser/increment-notfixed.stderr @@ -16,7 +16,9 @@ error: Rust has no postfix increment operator --> $DIR/increment-notfixed.rs:17:12 | LL | while i++ < 5 { - | ^^ not a valid postfix operator + | ----- ^^ not a valid postfix operator + | | + | while parsing the condition of this `while` expression | help: use `+= 1` instead | @@ -44,7 +46,9 @@ error: Rust has no postfix increment operator --> $DIR/increment-notfixed.rs:31:14 | LL | while tmp++ < 5 { - | ^^ not a valid postfix operator + | ----- ^^ not a valid postfix operator + | | + | while parsing the condition of this `while` expression | help: use `+= 1` instead |