Skip to content

Suggest i += 1 when we see i++ or ++i #88672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Apr 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
214 changes: 213 additions & 1 deletion compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -1167,6 +1250,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.
Expand Down
24 changes: 24 additions & 0 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}
Expand Down
31 changes: 31 additions & 0 deletions src/test/ui/parser/increment-autofix.fixed
Original file line number Diff line number Diff line change
@@ -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() {}
31 changes: 31 additions & 0 deletions src/test/ui/parser/increment-autofix.rs
Original file line number Diff line number Diff line change
@@ -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() {}
52 changes: 52 additions & 0 deletions src/test/ui/parser/increment-autofix.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading