Skip to content

Commit 89571a1

Browse files
estebankestebank
committed
Tweak chained operators diagnostic
Use more selective spans Improve suggestion output Be more selective when displaying suggestions Silence some knock-down type errors
1 parent 3c1d9ad commit 89571a1

6 files changed

+197
-160
lines changed

‎src/librustc_parse/parser/diagnostics.rs

+98-46
Original file line numberDiff line numberDiff line change
@@ -459,9 +459,18 @@ impl<'a> Parser<'a> {
459459
err: &mut DiagnosticBuilder<'_>,
460460
inner_op: &Expr,
461461
outer_op: &Spanned<AssocOp>,
462-
) {
462+
) -> bool /* recover */ {
463463
if let ExprKind::Binary(op, ref l1, ref r1) = inner_op.kind {
464-
match (op.node, &outer_op.node) {
464+
if let ExprKind::Field(_, ident) = l1.kind {
465+
if ident.as_str().parse::<i32>().is_err() && !matches!(r1.kind, ExprKind::Lit(_)) {
466+
// The parser has encountered `foo.bar<baz`, the likelihood of the turbofish
467+
// suggestion being the only one to apply is high.
468+
return false;
469+
}
470+
}
471+
return match (op.node, &outer_op.node) {
472+
// `x == y == z`
473+
(BinOpKind::Eq, AssocOp::Equal) |
465474
// `x < y < z` and friends.
466475
(BinOpKind::Lt, AssocOp::Less) | (BinOpKind::Lt, AssocOp::LessEqual) |
467476
(BinOpKind::Le, AssocOp::LessEqual) | (BinOpKind::Le, AssocOp::Less) |
@@ -472,35 +481,65 @@ impl<'a> Parser<'a> {
472481
self.span_to_snippet(e.span)
473482
.unwrap_or_else(|_| pprust::expr_to_string(&e))
474483
};
475-
err.span_suggestion(
476-
inner_op.span.to(outer_op.span),
477-
"split the comparison into two...",
478-
format!(
479-
"{} {} {} && {} {}",
480-
expr_to_str(&l1),
481-
op.node.to_string(),
482-
expr_to_str(&r1),
483-
expr_to_str(&r1),
484-
outer_op.node.to_ast_binop().unwrap().to_string(),
485-
),
484+
err.span_suggestion_verbose(
485+
inner_op.span.shrink_to_hi(),
486+
"split the comparison into two",
487+
format!(" && {}", expr_to_str(&r1)),
486488
Applicability::MaybeIncorrect,
487489
);
488-
err.span_suggestion(
489-
inner_op.span.to(outer_op.span),
490-
"...or parenthesize one of the comparisons",
491-
format!(
492-
"({} {} {}) {}",
493-
expr_to_str(&l1),
494-
op.node.to_string(),
495-
expr_to_str(&r1),
496-
outer_op.node.to_ast_binop().unwrap().to_string(),
497-
),
490+
false // Keep the current parse behavior, where the AST is `(x < y) < z`.
491+
}
492+
// `x == y < z`
493+
(BinOpKind::Eq, AssocOp::Less) | (BinOpKind::Eq, AssocOp::LessEqual) |
494+
(BinOpKind::Eq, AssocOp::Greater) | (BinOpKind::Eq, AssocOp::GreaterEqual) => {
495+
// Consume `/`z`/outer-op-rhs.
496+
let snapshot = self.clone();
497+
match self.parse_expr() {
498+
Ok(r2) => {
499+
err.multipart_suggestion(
500+
"parenthesize the comparison",
501+
vec![
502+
(r1.span.shrink_to_lo(), "(".to_string()),
503+
(r2.span.shrink_to_hi(), ")".to_string()),
504+
],
505+
Applicability::MaybeIncorrect,
506+
);
507+
true
508+
}
509+
Err(mut expr_err) => {
510+
expr_err.cancel();
511+
mem::replace(self, snapshot);
512+
false
513+
}
514+
}
515+
}
516+
// `x > y == z`
517+
(BinOpKind::Lt, AssocOp::Equal) | (BinOpKind::Le, AssocOp::Equal) |
518+
(BinOpKind::Gt, AssocOp::Equal) | (BinOpKind::Ge, AssocOp::Equal) => {
519+
let snapshot = self.clone();
520+
err.multipart_suggestion(
521+
"parenthesize the comparison",
522+
vec![
523+
(l1.span.shrink_to_lo(), "(".to_string()),
524+
(r1.span.shrink_to_hi(), ")".to_string()),
525+
],
498526
Applicability::MaybeIncorrect,
499527
);
528+
match self.parse_expr() {
529+
Ok(_) => {
530+
true
531+
}
532+
Err(mut expr_err) => {
533+
expr_err.cancel();
534+
mem::replace(self, snapshot);
535+
false
536+
}
537+
}
500538
}
501-
_ => {}
502-
}
539+
_ => false,
540+
};
503541
}
542+
false
504543
}
505544

506545
/// Produces an error if comparison operators are chained (RFC #558).
@@ -534,31 +573,26 @@ impl<'a> Parser<'a> {
534573
|this: &Self, span| Ok(Some(this.mk_expr(span, ExprKind::Err, AttrVec::new())));
535574

536575
match inner_op.kind {
537-
ExprKind::Binary(op, _, _) if op.node.is_comparison() => {
538-
// Respan to include both operators.
539-
let op_span = op.span.to(self.prev_token.span);
540-
let mut err =
541-
self.struct_span_err(op_span, "comparison operators cannot be chained");
542-
543-
// If it looks like a genuine attempt to chain operators (as opposed to a
544-
// misformatted turbofish, for instance), suggest a correct form.
545-
self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op);
576+
ExprKind::Binary(op, ref l1, ref r1) if op.node.is_comparison() => {
577+
let mut err = self.struct_span_err(
578+
vec![op.span, self.prev_token.span],
579+
"comparison operators cannot be chained",
580+
);
546581

547582
let suggest = |err: &mut DiagnosticBuilder<'_>| {
548583
err.span_suggestion_verbose(
549-
op_span.shrink_to_lo(),
584+
op.span.shrink_to_lo(),
550585
TURBOFISH,
551586
"::".to_string(),
552587
Applicability::MaybeIncorrect,
553588
);
554589
};
555590

556-
if op.node == BinOpKind::Lt &&
557-
outer_op.node == AssocOp::Less || // Include `<` to provide this recommendation
558-
outer_op.node == AssocOp::Greater
559-
// even in a case like the following:
591+
if op.node == BinOpKind::Lt && outer_op.node == AssocOp::Less
592+
|| outer_op.node == AssocOp::Greater
560593
{
561-
// Foo<Bar<Baz<Qux, ()>>>
594+
// Include `<` to provide this recommendation
595+
// even in a case like `Foo<Bar<Baz<Qux, ()>>>`
562596
if outer_op.node == AssocOp::Less {
563597
let snapshot = self.clone();
564598
self.bump();
@@ -617,15 +651,33 @@ impl<'a> Parser<'a> {
617651
}
618652
}
619653
} else {
620-
// All we know is that this is `foo < bar >` and *nothing* else. Try to
621-
// be helpful, but don't attempt to recover.
622-
err.help(TURBOFISH);
623-
err.help("or use `(...)` if you meant to specify fn arguments");
624-
// These cases cause too many knock-down errors, bail out (#61329).
625-
Err(err)
654+
if !matches!(l1.kind, ExprKind::Lit(_))
655+
&& !matches!(r1.kind, ExprKind::Lit(_))
656+
{
657+
// All we know is that this is `foo < bar >` and *nothing* else. Try to
658+
// be helpful, but don't attempt to recover.
659+
err.help(TURBOFISH);
660+
err.help("or use `(...)` if you meant to specify fn arguments");
661+
}
662+
663+
// If it looks like a genuine attempt to chain operators (as opposed to a
664+
// misformatted turbofish, for instance), suggest a correct form.
665+
if self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op)
666+
{
667+
err.emit();
668+
mk_err_expr(self, inner_op.span.to(self.prev_token.span))
669+
} else {
670+
// These cases cause too many knock-down errors, bail out (#61329).
671+
Err(err)
672+
}
626673
};
627674
}
675+
let recover =
676+
self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op);
628677
err.emit();
678+
if recover {
679+
return mk_err_expr(self, inner_op.span.to(self.prev_token.span));
680+
}
629681
}
630682
_ => {}
631683
}

‎src/test/ui/did_you_mean/issue-40396.stderr

+3-19
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,8 @@ error: comparison operators cannot be chained
22
--> $DIR/issue-40396.rs:2:20
33
|
44
LL | (0..13).collect<Vec<i32>>();
5-
| ^^^^^
5+
| ^ ^
66
|
7-
help: split the comparison into two...
8-
|
9-
LL | (0..13).collect < Vec && Vec <i32>>();
10-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11-
help: ...or parenthesize one of the comparisons
12-
|
13-
LL | ((0..13).collect < Vec) <i32>>();
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
157
help: use `::<...>` instead of `<...>` to specify type arguments
168
|
179
LL | (0..13).collect::<Vec<i32>>();
@@ -21,7 +13,7 @@ error: comparison operators cannot be chained
2113
--> $DIR/issue-40396.rs:4:8
2214
|
2315
LL | Vec<i32>::new();
24-
| ^^^^^
16+
| ^ ^
2517
|
2618
help: use `::<...>` instead of `<...>` to specify type arguments
2719
|
@@ -32,16 +24,8 @@ error: comparison operators cannot be chained
3224
--> $DIR/issue-40396.rs:6:20
3325
|
3426
LL | (0..13).collect<Vec<i32>();
35-
| ^^^^^
36-
|
37-
help: split the comparison into two...
38-
|
39-
LL | (0..13).collect < Vec && Vec <i32>();
40-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
41-
help: ...or parenthesize one of the comparisons
27+
| ^ ^
4228
|
43-
LL | ((0..13).collect < Vec) <i32>();
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^
4529
help: use `::<...>` instead of `<...>` to specify type arguments
4630
|
4731
LL | (0..13).collect::<Vec<i32>();

‎src/test/ui/parser/chained-comparison-suggestion.rs

+13
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,17 @@ fn comp8() {
3737
//~^ ERROR mismatched types
3838
}
3939

40+
fn comp9() {
41+
1 == 2 < 3; //~ ERROR comparison operators cannot be chained
42+
}
43+
44+
fn comp10() {
45+
1 > 2 == false; //~ ERROR comparison operators cannot be chained
46+
}
47+
48+
fn comp11() {
49+
1 == 2 == 3; //~ ERROR comparison operators cannot be chained
50+
//~^ ERROR mismatched types
51+
}
52+
4053
fn main() {}

0 commit comments

Comments
 (0)