Rollup merge of #69878 - estebank:chained-ops, r=Centril

Tweak chained operators diagnostic

Use more selective spans
Improve suggestion output
Be more selective when displaying suggestions
Silence some knock-down type errors

r? @Centril
This commit is contained in:
Mazdak Farrokhzad 2020-03-26 03:21:27 +01:00 committed by GitHub
commit 9fa4953aa4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 202 additions and 166 deletions

View File

@ -17,7 +17,6 @@ use rustc_span::symbol::kw;
use rustc_span::{MultiSpan, Span, SpanSnippetError, DUMMY_SP};
use log::{debug, trace};
use std::mem;
const TURBOFISH: &str = "use `::<...>` instead of `<...>` to specify type arguments";
@ -459,9 +458,28 @@ impl<'a> Parser<'a> {
err: &mut DiagnosticBuilder<'_>,
inner_op: &Expr,
outer_op: &Spanned<AssocOp>,
) {
) -> bool /* advanced the cursor */ {
if let ExprKind::Binary(op, ref l1, ref r1) = inner_op.kind {
match (op.node, &outer_op.node) {
if let ExprKind::Field(_, ident) = l1.kind {
if ident.as_str().parse::<i32>().is_err() && !matches!(r1.kind, ExprKind::Lit(_)) {
// The parser has encountered `foo.bar<baz`, the likelihood of the turbofish
// suggestion being the only one to apply is high.
return false;
}
}
let mut enclose = |left: Span, right: Span| {
err.multipart_suggestion(
"parenthesize the comparison",
vec![
(left.shrink_to_lo(), "(".to_string()),
(right.shrink_to_hi(), ")".to_string()),
],
Applicability::MaybeIncorrect,
);
};
return match (op.node, &outer_op.node) {
// `x == y == z`
(BinOpKind::Eq, AssocOp::Equal) |
// `x < y < z` and friends.
(BinOpKind::Lt, AssocOp::Less) | (BinOpKind::Lt, AssocOp::LessEqual) |
(BinOpKind::Le, AssocOp::LessEqual) | (BinOpKind::Le, AssocOp::Less) |
@ -472,35 +490,55 @@ impl<'a> Parser<'a> {
self.span_to_snippet(e.span)
.unwrap_or_else(|_| pprust::expr_to_string(&e))
};
err.span_suggestion(
inner_op.span.to(outer_op.span),
"split the comparison into two...",
format!(
"{} {} {} && {} {}",
expr_to_str(&l1),
op.node.to_string(),
expr_to_str(&r1),
expr_to_str(&r1),
outer_op.node.to_ast_binop().unwrap().to_string(),
),
Applicability::MaybeIncorrect,
);
err.span_suggestion(
inner_op.span.to(outer_op.span),
"...or parenthesize one of the comparisons",
format!(
"({} {} {}) {}",
expr_to_str(&l1),
op.node.to_string(),
expr_to_str(&r1),
outer_op.node.to_ast_binop().unwrap().to_string(),
),
err.span_suggestion_verbose(
inner_op.span.shrink_to_hi(),
"split the comparison into two",
format!(" && {}", expr_to_str(&r1)),
Applicability::MaybeIncorrect,
);
false // Keep the current parse behavior, where the AST is `(x < y) < z`.
}
_ => {}
}
// `x == y < z`
(BinOpKind::Eq, AssocOp::Less) | (BinOpKind::Eq, AssocOp::LessEqual) |
(BinOpKind::Eq, AssocOp::Greater) | (BinOpKind::Eq, AssocOp::GreaterEqual) => {
// Consume `z`/outer-op-rhs.
let snapshot = self.clone();
match self.parse_expr() {
Ok(r2) => {
// We are sure that outer-op-rhs could be consumed, the suggestion is
// likely correct.
enclose(r1.span, r2.span);
true
}
Err(mut expr_err) => {
expr_err.cancel();
*self = snapshot;
false
}
}
}
// `x > y == z`
(BinOpKind::Lt, AssocOp::Equal) | (BinOpKind::Le, AssocOp::Equal) |
(BinOpKind::Gt, AssocOp::Equal) | (BinOpKind::Ge, AssocOp::Equal) => {
let snapshot = self.clone();
// At this point it is always valid to enclose the lhs in parentheses, no
// further checks are necessary.
match self.parse_expr() {
Ok(_) => {
enclose(l1.span, r1.span);
true
}
Err(mut expr_err) => {
expr_err.cancel();
*self = snapshot;
false
}
}
}
_ => false,
};
}
false
}
/// Produces an error if comparison operators are chained (RFC #558).
@ -534,31 +572,26 @@ impl<'a> Parser<'a> {
|this: &Self, span| Ok(Some(this.mk_expr(span, ExprKind::Err, AttrVec::new())));
match inner_op.kind {
ExprKind::Binary(op, _, _) if op.node.is_comparison() => {
// Respan to include both operators.
let op_span = op.span.to(self.prev_token.span);
let mut err =
self.struct_span_err(op_span, "comparison operators cannot be chained");
// If it looks like a genuine attempt to chain operators (as opposed to a
// misformatted turbofish, for instance), suggest a correct form.
self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op);
ExprKind::Binary(op, ref l1, ref r1) if op.node.is_comparison() => {
let mut err = self.struct_span_err(
vec![op.span, self.prev_token.span],
"comparison operators cannot be chained",
);
let suggest = |err: &mut DiagnosticBuilder<'_>| {
err.span_suggestion_verbose(
op_span.shrink_to_lo(),
op.span.shrink_to_lo(),
TURBOFISH,
"::".to_string(),
Applicability::MaybeIncorrect,
);
};
if op.node == BinOpKind::Lt &&
outer_op.node == AssocOp::Less || // Include `<` to provide this recommendation
outer_op.node == AssocOp::Greater
// even in a case like the following:
// Include `<` to provide this recommendation even in a case like
// `Foo<Bar<Baz<Qux, ()>>>`
if op.node == BinOpKind::Lt && outer_op.node == AssocOp::Less
|| outer_op.node == AssocOp::Greater
{
// Foo<Bar<Baz<Qux, ()>>>
if outer_op.node == AssocOp::Less {
let snapshot = self.clone();
self.bump();
@ -572,7 +605,7 @@ impl<'a> Parser<'a> {
{
// We don't have `foo< bar >(` or `foo< bar >::`, so we rewind the
// parser and bail out.
mem::replace(self, snapshot.clone());
*self = snapshot.clone();
}
}
return if token::ModSep == self.token.kind {
@ -597,7 +630,7 @@ impl<'a> Parser<'a> {
expr_err.cancel();
// Not entirely sure now, but we bubble the error up with the
// suggestion.
mem::replace(self, snapshot);
*self = snapshot;
Err(err)
}
}
@ -617,15 +650,33 @@ impl<'a> Parser<'a> {
}
}
} else {
// All we know is that this is `foo < bar >` and *nothing* else. Try to
// be helpful, but don't attempt to recover.
err.help(TURBOFISH);
err.help("or use `(...)` if you meant to specify fn arguments");
// These cases cause too many knock-down errors, bail out (#61329).
Err(err)
if !matches!(l1.kind, ExprKind::Lit(_))
&& !matches!(r1.kind, ExprKind::Lit(_))
{
// All we know is that this is `foo < bar >` and *nothing* else. Try to
// be helpful, but don't attempt to recover.
err.help(TURBOFISH);
err.help("or use `(...)` if you meant to specify fn arguments");
}
// If it looks like a genuine attempt to chain operators (as opposed to a
// misformatted turbofish, for instance), suggest a correct form.
if self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op)
{
err.emit();
mk_err_expr(self, inner_op.span.to(self.prev_token.span))
} else {
// These cases cause too many knock-down errors, bail out (#61329).
Err(err)
}
};
}
let recover =
self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op);
err.emit();
if recover {
return mk_err_expr(self, inner_op.span.to(self.prev_token.span));
}
}
_ => {}
}
@ -643,7 +694,7 @@ impl<'a> Parser<'a> {
if self.token.kind == token::Eof {
// Not entirely sure that what we consumed were fn arguments, rollback.
mem::replace(self, snapshot);
*self = snapshot;
Err(())
} else {
// 99% certain that the suggestion is correct, continue parsing.

View File

@ -2,16 +2,8 @@ error: comparison operators cannot be chained
--> $DIR/issue-40396.rs:2:20
|
LL | (0..13).collect<Vec<i32>>();
| ^^^^^
| ^ ^
|
help: split the comparison into two...
|
LL | (0..13).collect < Vec && Vec <i32>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | ((0..13).collect < Vec) <i32>>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | (0..13).collect::<Vec<i32>>();
@ -21,7 +13,7 @@ error: comparison operators cannot be chained
--> $DIR/issue-40396.rs:4:8
|
LL | Vec<i32>::new();
| ^^^^^
| ^ ^
|
help: use `::<...>` instead of `<...>` to specify type arguments
|
@ -32,16 +24,8 @@ error: comparison operators cannot be chained
--> $DIR/issue-40396.rs:6:20
|
LL | (0..13).collect<Vec<i32>();
| ^^^^^
| ^ ^
|
help: split the comparison into two...
|
LL | (0..13).collect < Vec && Vec <i32>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | ((0..13).collect < Vec) <i32>();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | (0..13).collect::<Vec<i32>();

View File

@ -37,4 +37,17 @@ fn comp8() {
//~^ ERROR mismatched types
}
fn comp9() {
1 == 2 < 3; //~ ERROR comparison operators cannot be chained
}
fn comp10() {
1 > 2 == false; //~ ERROR comparison operators cannot be chained
}
fn comp11() {
1 == 2 == 3; //~ ERROR comparison operators cannot be chained
//~^ ERROR mismatched types
}
fn main() {}

View File

@ -2,127 +2,122 @@ error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:4:7
|
LL | 1 < 2 <= 3;
| ^^^^^^
| ^ ^^
|
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 < 2 && 2 <= 3;
| ^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (1 < 2) <= 3;
| ^^^^^^^^^^
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:9:7
|
LL | 1 < 2 < 3;
| ^^^^^
| ^ ^
|
= help: use `::<...>` instead of `<...>` to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 < 2 && 2 < 3;
| ^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (1 < 2) < 3;
| ^^^^^^^^^
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:13:7
|
LL | 1 <= 2 < 3;
| ^^^^^^
| ^^ ^
|
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 <= 2 && 2 < 3;
| ^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (1 <= 2) < 3;
| ^^^^^^^^^^
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:18:7
|
LL | 1 <= 2 <= 3;
| ^^^^^^^
| ^^ ^^
|
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 <= 2 && 2 <= 3;
| ^^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (1 <= 2) <= 3;
| ^^^^^^^^^^^
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:23:7
|
LL | 1 > 2 >= 3;
| ^^^^^^
| ^ ^^
|
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 > 2 && 2 >= 3;
| ^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (1 > 2) >= 3;
| ^^^^^^^^^^
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:28:7
|
LL | 1 > 2 > 3;
| ^^^^^
| ^ ^
|
= help: use `::<...>` instead of `<...>` to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 > 2 && 2 > 3;
| ^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (1 > 2) > 3;
| ^^^^^^^^^
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:32:7
|
LL | 1 >= 2 > 3;
| ^^^^^^
| ^^ ^
|
= help: use `::<...>` instead of `<...>` to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 >= 2 && 2 > 3;
| ^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (1 >= 2) > 3;
| ^^^^^^^^^^
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:36:7
|
LL | 1 >= 2 >= 3;
| ^^^^^^^
| ^^ ^^
|
help: split the comparison into two...
help: split the comparison into two
|
LL | 1 >= 2 && 2 >= 3;
| ^^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
| ^^^^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:41:7
|
LL | (1 >= 2) >= 3;
| ^^^^^^^^^^^
LL | 1 == 2 < 3;
| ^^ ^
|
help: parenthesize the comparison
|
LL | 1 == (2 < 3);
| ^ ^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:45:7
|
LL | 1 > 2 == false;
| ^ ^^
|
help: parenthesize the comparison
|
LL | (1 > 2) == false;
| ^ ^
error: comparison operators cannot be chained
--> $DIR/chained-comparison-suggestion.rs:49:7
|
LL | 1 == 2 == 3;
| ^^ ^^
|
help: split the comparison into two
|
LL | 1 == 2 && 2 == 3;
| ^^^^
error[E0308]: mismatched types
--> $DIR/chained-comparison-suggestion.rs:4:14
@ -154,6 +149,12 @@ error[E0308]: mismatched types
LL | 1 >= 2 >= 3;
| ^ expected `bool`, found integer
error: aborting due to 13 previous errors
error[E0308]: mismatched types
--> $DIR/chained-comparison-suggestion.rs:49:15
|
LL | 1 == 2 == 3;
| ^ expected `bool`, found integer
error: aborting due to 17 previous errors
For more information about this error, try `rustc --explain E0308`.

View File

@ -4,11 +4,11 @@ struct X;
fn main() {
false == false == false;
//~^ ERROR comparison operators cannot be chained
//~| HELP split the comparison into two
false == 0 < 2;
//~^ ERROR comparison operators cannot be chained
//~| ERROR mismatched types
//~| ERROR mismatched types
//~| HELP parenthesize the comparison
f<X>();
//~^ ERROR comparison operators cannot be chained
@ -16,8 +16,6 @@ fn main() {
f<Result<Option<X>, Option<Option<X>>>(1, 2);
//~^ ERROR comparison operators cannot be chained
//~| HELP split the comparison into two...
//~| ...or parenthesize one of the comparisons
//~| HELP use `::<...>` instead of `<...>` to specify type arguments
use std::convert::identity;

View File

@ -2,19 +2,29 @@ error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:5:11
|
LL | false == false == false;
| ^^^^^^^^^^^
| ^^ ^^
|
help: split the comparison into two
|
LL | false == false && false == false;
| ^^^^^^^^
error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:8:11
--> $DIR/require-parens-for-chained-comparison.rs:9:11
|
LL | false == 0 < 2;
| ^^^^^^
| ^^ ^
|
help: parenthesize the comparison
|
LL | false == (0 < 2);
| ^ ^
error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:13:6
|
LL | f<X>();
| ^^^
| ^ ^
|
help: use `::<...>` instead of `<...>` to specify type arguments
|
@ -25,42 +35,21 @@ error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:17:6
|
LL | f<Result<Option<X>, Option<Option<X>>>(1, 2);
| ^^^^^^^^
| ^ ^
|
help: split the comparison into two...
|
LL | f < Result && Result <Option<X>, Option<Option<X>>>(1, 2);
| ^^^^^^^^^^^^^^^^^^^^^^
help: ...or parenthesize one of the comparisons
|
LL | (f < Result) <Option<X>, Option<Option<X>>>(1, 2);
| ^^^^^^^^^^^^^^
help: use `::<...>` instead of `<...>` to specify type arguments
|
LL | f::<Result<Option<X>, Option<Option<X>>>(1, 2);
| ^^
error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:24:21
--> $DIR/require-parens-for-chained-comparison.rs:22:21
|
LL | let _ = identity<u8>;
| ^^^^
| ^ ^
|
= help: use `::<...>` instead of `<...>` to specify type arguments
= help: or use `(...)` if you meant to specify fn arguments
error[E0308]: mismatched types
--> $DIR/require-parens-for-chained-comparison.rs:8:14
|
LL | false == 0 < 2;
| ^ expected `bool`, found integer
error: aborting due to 5 previous errors
error[E0308]: mismatched types
--> $DIR/require-parens-for-chained-comparison.rs:8:18
|
LL | false == 0 < 2;
| ^ expected `bool`, found integer
error: aborting due to 7 previous errors
For more information about this error, try `rustc --explain E0308`.