Auto merge of #88672 - camelid:inc-parser-sugg, r=davidtwco

Suggest `i += 1` when we see `i++` or `++i`

Closes #83502 (for `i++` and `++i`; `--i` should be covered by #82987, and `i--`
is tricky to handle).

This is a continuation of #83536.

r? `@estebank`

Author: bors

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};
@@ -156,6 +156,89 @@ impl AttemptLocalParseRecovery {
     }
 }
 
+/// Information for emitting suggestions and recovering from
+/// C-style `i++`, `--i`, etc.
+#[derive(Debug, Copy, Clone)]
+struct IncDecRecovery {
+    /// Is this increment/decrement its own statement?
+    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,
+    // 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"),
+        }
+    }
+}
+
+struct MultiSugg {
+    msg: String,
+    patches: Vec<(Span, String)>,
+    applicability: Applicability,
+}
+
+impl MultiSugg {
+    fn emit<G: EmissionGuarantee>(self, err: &mut DiagnosticBuilder<'_, G>) {
+        err.multipart_suggestion(&self.msg, self.patches, self.applicability);
+    }
+
+    /// Overrides individual messages and applicabilities.
+    fn emit_many<G: EmissionGuarantee>(
+        err: &mut DiagnosticBuilder<'_, G>,
+        msg: &str,
+        applicability: Applicability,
+        suggestions: impl Iterator<Item = Self>,
+    ) {
+        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.
@@ -1171,6 +1254,135 @@ impl<'a> Parser<'a> {
         Ok(())
     }
 
+    pub(super) fn recover_from_prefix_increment(
+        &mut self,
+        operand_expr: P<Expr>,
+        op_span: Span,
+        prev_is_semi: bool,
+    ) -> PResult<'a, P<Expr>> {
+        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)
+    }
+
+    pub(super) fn recover_from_postfix_increment(
+        &mut self,
+        operand_expr: P<Expr>,
+        op_span: Span,
+    ) -> PResult<'a, P<Expr>> {
+        let kind = IncDecRecovery {
+            standalone: IsStandalone::Maybe,
+            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<Expr>,
+        kind: IncDecRecovery,
+        op_span: Span,
+    ) -> PResult<'a, P<Expr>> {
+        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));
+
+        let help_base_case = |mut err: DiagnosticBuilder<'_, _>, base| {
+            err.help(&format!("use `{}= 1` instead", kind.op.chr()));
+            err.emit();
+            Ok(base)
+        };
+
+        // (pre, post)
+        let spans = match kind.fixity {
+            UnaryFixity::Pre => (op_span, base.span.shrink_to_hi()),
+            UnaryFixity::Post => (base.span.shrink_to_lo(), op_span),
+        };
+
+        match kind.standalone {
+            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, kind, spans).emit(&mut err)
+                    }
+                    UnaryFixity::Post => {
+                        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) };
+                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::Unspecified,
+                    [sugg1, sugg2].into_iter(),
+                )
+            }
+        }
+        Err(err)
+    }
+
+    fn prefix_inc_dec_suggest(
+        &mut self,
+        base_src: String,
+        kind: IncDecRecovery,
+        (pre_span, post_span): (Span, Span),
+    ) -> 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: Applicability::MachineApplicable,
+        }
+    }
+
+    fn postfix_inc_dec_suggest(
+        &mut self,
+        base_src: String,
+        kind: IncDecRecovery,
+        (pre_span, post_span): (Span, Span),
+    ) -> MultiSugg {
+        let tmp_var = if base_src.trim() == "tmp" { "tmp_" } else { "tmp" };
+        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: Applicability::HasPlaceholders,
+        }
+    }
+
+    fn inc_dec_standalone_suggest(
+        &mut self,
+        kind: IncDecRecovery,
+        (pre_span, post_span): (Span, Span),
+    ) -> 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`.
     /// Attempts to convert the base expression/pattern/type into a type, parses the `::AssocItem`
     /// tail, and combines them into a `<Ty>::AssocItem` expression/pattern/type.

compiler/rustc_parse/src/parser/expr.rs

@@ -267,6 +267,17 @@ impl<'a> Parser<'a> {
                 self.bump();
             }
 
+            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 `+`
+                self.bump();
+                lhs = self.recover_from_postfix_increment(lhs, op_span)?;
+                continue;
+            }
+
             let op = op.node;
             // Special cases:
             if op == AssocOp::As {
@@ -580,6 +591,19 @@ impl<'a> Parser<'a> {
                 this.bump();
                 this.parse_prefix_expr(None)
             } // `+expr`
+            // 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_dot_or_call_expr(Default::default())?;
+                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))
             }

src/test/ui/parser/increment-autofix.fixed

@@ -0,0 +1,31 @@
+// run-rustfix
+
+pub fn pre_regular() {
+    let mut i = 0;
+    i += 1; //~ ERROR Rust has no prefix increment operator
+    println!("{}", i);
+}
+
+pub fn pre_while() {
+    let mut i = 0;
+    while { i += 1; i } < 5 {
+        //~^ ERROR Rust has no prefix increment operator
+        println!("{}", i);
+    }
+}
+
+pub fn pre_regular_tmp() {
+    let mut tmp = 0;
+    tmp += 1; //~ ERROR Rust has no prefix increment operator
+    println!("{}", tmp);
+}
+
+pub fn pre_while_tmp() {
+    let mut tmp = 0;
+    while { tmp += 1; tmp } < 5 {
+        //~^ ERROR Rust has no prefix increment operator
+        println!("{}", tmp);
+    }
+}
+
+fn main() {}

src/test/ui/parser/increment-autofix.rs

@@ -0,0 +1,31 @@
+// run-rustfix
+
+pub fn pre_regular() {
+    let mut i = 0;
+    ++i; //~ ERROR Rust has no prefix increment operator
+    println!("{}", i);
+}
+
+pub fn pre_while() {
+    let mut i = 0;
+    while ++i < 5 {
+        //~^ ERROR Rust has no prefix increment operator
+        println!("{}", i);
+    }
+}
+
+pub fn pre_regular_tmp() {
+    let mut tmp = 0;
+    ++tmp; //~ ERROR Rust has no prefix increment operator
+    println!("{}", tmp);
+}
+
+pub fn pre_while_tmp() {
+    let mut tmp = 0;
+    while ++tmp < 5 {
+        //~^ ERROR Rust has no prefix increment operator
+        println!("{}", tmp);
+    }
+}
+
+fn main() {}

src/test/ui/parser/increment-autofix.stderr

@@ -0,0 +1,52 @@
+error: Rust has no prefix increment operator
+  --> $DIR/increment-autofix.rs:5:5
+   |
+LL |     ++i;
+   |     ^^ not a valid prefix operator
+   |
+help: use `+= 1` instead
+   |
+LL -     ++i;
+LL +     i += 1;
+   | 
+
+error: Rust has no prefix increment operator
+  --> $DIR/increment-autofix.rs:11:11
+   |
+LL |     while ++i < 5 {
+   |     ----- ^^ not a valid prefix operator
+   |     |
+   |     while parsing the condition of this `while` expression
+   |
+help: use `+= 1` instead
+   |
+LL |     while { i += 1; i } < 5 {
+   |           ~   +++++++++
+
+error: Rust has no prefix increment operator
+  --> $DIR/increment-autofix.rs:19:5
+   |
+LL |     ++tmp;
+   |     ^^ not a valid prefix operator
+   |
+help: use `+= 1` instead
+   |
+LL -     ++tmp;
+LL +     tmp += 1;
+   | 
+
+error: Rust has no prefix increment operator
+  --> $DIR/increment-autofix.rs:25:11
+   |
+LL |     while ++tmp < 5 {
+   |     ----- ^^ not a valid prefix operator
+   |     |
+   |     while parsing the condition of this `while` expression
+   |
+help: use `+= 1` instead
+   |
+LL |     while { tmp += 1; tmp } < 5 {
+   |           ~     +++++++++++
+
+error: aborting due to 4 previous errors
+