Skip to content

Commit 86085b4

Browse files
estebankestebank
committed
Avoid unnecessary suggestion in or-pattern
1 parent 3355f4b commit 86085b4

File tree

7 files changed

+36
-73
lines changed

7 files changed

+36
-73
lines changed

‎compiler/rustc_resolve/src/diagnostics.rs‎

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -678,26 +678,32 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
678678
for sp in &origin_sp {
679679
err.subdiagnostic(errors::VariableNotInAllPatterns { span: *sp });
680680
}
681-
let mut target_visitor = BindingVisitor::default();
682-
for pat in &target {
683-
target_visitor.visit_pat(pat);
684-
}
685-
target_visitor.identifiers.sort();
686-
target_visitor.identifiers.dedup();
687-
let mut origin_visitor = BindingVisitor::default();
688-
for (_, pat) in &origin {
689-
origin_visitor.visit_pat(pat);
690-
}
691-
origin_visitor.identifiers.sort();
692-
origin_visitor.identifiers.dedup();
693-
// Find if the binding could have been a typo
694681
let mut suggested_typo = false;
695-
if let Some(typo) =
696-
find_best_match_for_name(&target_visitor.identifiers, name.name, None)
697-
&& !origin_visitor.identifiers.contains(&typo)
682+
if !target.iter().all(|pat| matches!(pat.kind, ast::PatKind::Ident(..)))
683+
&& !origin.iter().all(|(_, pat)| matches!(pat.kind, ast::PatKind::Ident(..)))
698684
{
699-
err.subdiagnostic(errors::PatternBindingTypo { spans: origin_sp, typo });
700-
suggested_typo = true;
685+
// The check above is so that when we encounter `match foo { (a | b) => {} }`,
686+
// we don't suggest `(a | a) => {}`, which would never be what the user wants.
687+
let mut target_visitor = BindingVisitor::default();
688+
for pat in &target {
689+
target_visitor.visit_pat(pat);
690+
}
691+
target_visitor.identifiers.sort();
692+
target_visitor.identifiers.dedup();
693+
let mut origin_visitor = BindingVisitor::default();
694+
for (_, pat) in &origin {
695+
origin_visitor.visit_pat(pat);
696+
}
697+
origin_visitor.identifiers.sort();
698+
origin_visitor.identifiers.dedup();
699+
// Find if the binding could have been a typo
700+
if let Some(typo) =
701+
find_best_match_for_name(&target_visitor.identifiers, name.name, None)
702+
&& !origin_visitor.identifiers.contains(&typo)
703+
{
704+
err.subdiagnostic(errors::PatternBindingTypo { spans: origin_sp, typo });
705+
suggested_typo = true;
706+
}
701707
}
702708
if could_be_path {
703709
let import_suggestions = self.lookup_import_candidates(

‎tests/ui/or-patterns/binding-typo-2.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ fn foo(x: (Lol, Lol)) {
4747
//~| ERROR: variable `Non` is not bound in all patterns [E0408]
4848
//~| NOTE: pattern doesn't bind `Non`
4949
//~| NOTE: variable not in all patterns
50-
//~| HELP: you might have meant to use the similarly named previously used binding `None`
50+
//~| HELP: you might have meant to use the similarly named unit variant `None`
5151
//~| HELP: you might have meant to pattern match on the similarly named
5252
}
5353
match Some(42) {

‎tests/ui/or-patterns/binding-typo-2.stderr‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,11 @@ LL | Non | None => {}
3434
| |
3535
| variable not in all patterns
3636
|
37-
help: you might have meant to use the similarly named previously used binding `None`
37+
help: you might have meant to use the similarly named unit variant `None`
38+
|
39+
LL - Non | None => {}
40+
LL + core::option::Option::None | None => {}
3841
|
39-
LL | None | None => {}
40-
| +
4142

4243
error[E0408]: variable `Non` is not bound in all patterns
4344
--> $DIR/binding-typo-2.rs:54:15

‎tests/ui/or-patterns/mismatched-bindings-async-fn.stderr‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ LL | async fn a((x | s): String) {}
55
| ^ - variable not in all patterns
66
| |
77
| pattern doesn't bind `s`
8-
|
9-
help: you might have meant to use the similarly named previously used binding `x`
10-
|
11-
LL - async fn a((x | s): String) {}
12-
LL + async fn a((x | x): String) {}
13-
|
148

159
error[E0408]: variable `x` is not bound in all patterns
1610
--> $DIR/mismatched-bindings-async-fn.rs:4:17
@@ -19,12 +13,6 @@ LL | async fn a((x | s): String) {}
1913
| - ^ pattern doesn't bind `x`
2014
| |
2115
| variable not in all patterns
22-
|
23-
help: you might have meant to use the similarly named previously used binding `s`
24-
|
25-
LL - async fn a((x | s): String) {}
26-
LL + async fn a((s | s): String) {}
27-
|
2816

2917
error[E0408]: variable `s` is not bound in all patterns
3018
--> $DIR/mismatched-bindings-async-fn.rs:9:10
@@ -33,12 +21,6 @@ LL | let (x | s) = String::new();
3321
| ^ - variable not in all patterns
3422
| |
3523
| pattern doesn't bind `s`
36-
|
37-
help: you might have meant to use the similarly named previously used binding `x`
38-
|
39-
LL - let (x | s) = String::new();
40-
LL + let (x | x) = String::new();
41-
|
4224

4325
error[E0408]: variable `x` is not bound in all patterns
4426
--> $DIR/mismatched-bindings-async-fn.rs:9:14
@@ -47,12 +29,6 @@ LL | let (x | s) = String::new();
4729
| - ^ pattern doesn't bind `x`
4830
| |
4931
| variable not in all patterns
50-
|
51-
help: you might have meant to use the similarly named previously used binding `s`
52-
|
53-
LL - let (x | s) = String::new();
54-
LL + let (s | s) = String::new();
55-
|
5632

5733
error: aborting due to 4 previous errors
5834

‎tests/ui/or-patterns/nested-undelimited-precedence.stderr‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,12 +60,6 @@ LL | let b @ A | B: E = A;
6060
| - ^ pattern doesn't bind `b`
6161
| |
6262
| variable not in all patterns
63-
|
64-
help: you might have meant to use the similarly named previously used binding `B`
65-
|
66-
LL - let b @ A | B: E = A;
67-
LL + let B @ A | B: E = A;
68-
|
6963

7064
error[E0308]: mismatched types
7165
--> $DIR/nested-undelimited-precedence.rs:34:9

‎tests/ui/resolve/resolve-inconsistent-names.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ fn main() {
1212
match y {
1313
a | b => {} //~ ERROR variable `a` is not bound in all patterns
1414
//~| ERROR variable `b` is not bound in all patterns
15-
//~| HELP you might have meant to use the similarly named previously used binding `a`
16-
//~| HELP you might have meant to use the similarly named previously used binding `b`
1715
}
1816

1917
let x = (E::A, E::B);

‎tests/ui/resolve/resolve-inconsistent-names.stderr‎

Lines changed: 7 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ LL | a | b => {}
55
| ^ - variable not in all patterns
66
| |
77
| pattern doesn't bind `b`
8-
|
9-
help: you might have meant to use the similarly named previously used binding `a`
10-
|
11-
LL - a | b => {}
12-
LL + a | a => {}
13-
|
148

159
error[E0408]: variable `a` is not bound in all patterns
1610
--> $DIR/resolve-inconsistent-names.rs:13:13
@@ -19,15 +13,9 @@ LL | a | b => {}
1913
| - ^ pattern doesn't bind `a`
2014
| |
2115
| variable not in all patterns
22-
|
23-
help: you might have meant to use the similarly named previously used binding `b`
24-
|
25-
LL - a | b => {}
26-
LL + b | b => {}
27-
|
2816

2917
error[E0408]: variable `c` is not bound in all patterns
30-
--> $DIR/resolve-inconsistent-names.rs:21:9
18+
--> $DIR/resolve-inconsistent-names.rs:19:9
3119
|
3220
LL | (A, B) | (ref B, c) | (c, A) => ()
3321
| ^^^^^^ - - variable not in all patterns
@@ -36,7 +24,7 @@ LL | (A, B) | (ref B, c) | (c, A) => ()
3624
| pattern doesn't bind `c`
3725

3826
error[E0408]: variable `A` is not bound in all patterns
39-
--> $DIR/resolve-inconsistent-names.rs:21:18
27+
--> $DIR/resolve-inconsistent-names.rs:19:18
4028
|
4129
LL | (A, B) | (ref B, c) | (c, A) => ()
4230
| - ^^^^^^^^^^ - variable not in all patterns
@@ -50,7 +38,7 @@ LL | (E::A, B) | (ref B, c) | (c, A) => ()
5038
| +++
5139

5240
error[E0408]: variable `B` is not bound in all patterns
53-
--> $DIR/resolve-inconsistent-names.rs:21:31
41+
--> $DIR/resolve-inconsistent-names.rs:19:31
5442
|
5543
LL | (A, B) | (ref B, c) | (c, A) => ()
5644
| - - ^^^^^^ pattern doesn't bind `B`
@@ -59,15 +47,15 @@ LL | (A, B) | (ref B, c) | (c, A) => ()
5947
| variable not in all patterns
6048

6149
error[E0409]: variable `B` is bound inconsistently across alternatives separated by `|`
62-
--> $DIR/resolve-inconsistent-names.rs:21:23
50+
--> $DIR/resolve-inconsistent-names.rs:19:23
6351
|
6452
LL | (A, B) | (ref B, c) | (c, A) => ()
6553
| - ^ bound in different ways
6654
| |
6755
| first binding
6856

6957
error[E0408]: variable `Const2` is not bound in all patterns
70-
--> $DIR/resolve-inconsistent-names.rs:33:9
58+
--> $DIR/resolve-inconsistent-names.rs:31:9
7159
|
7260
LL | (CONST1, _) | (_, Const2) => ()
7361
| ^^^^^^^^^^^ ------ variable not in all patterns
@@ -80,7 +68,7 @@ LL | (CONST1, _) | (_, m::Const2) => ()
8068
| +++
8169

8270
error[E0408]: variable `CONST1` is not bound in all patterns
83-
--> $DIR/resolve-inconsistent-names.rs:33:23
71+
--> $DIR/resolve-inconsistent-names.rs:31:23
8472
|
8573
LL | (CONST1, _) | (_, Const2) => ()
8674
| ------ ^^^^^^^^^^^ pattern doesn't bind `CONST1`
@@ -94,7 +82,7 @@ LL | const CONST1: usize = 10;
9482
| ^^^^^^^^^^^^^^^^^^^^^^^^^ not accessible
9583

9684
error[E0308]: mismatched types
97-
--> $DIR/resolve-inconsistent-names.rs:21:19
85+
--> $DIR/resolve-inconsistent-names.rs:19:19
9886
|
9987
LL | match x {
10088
| - this expression has type `(E, E)`

0 commit comments

Comments
 (0)