Skip to content

Commit 7e0d1db

Browse files
jakubadamwjakubadamw
committed
Suggest using a qualified path in patterns with inconsistent bindings
A program like the following one: ```rust enum E { A, B, C } fn f(x: E) -> bool { match x { A | B => false, C => true } } ``` is rejected by the compiler due to `E` variant paths not being in scope. In this case `A`, `B` are resolved as pattern bindings and consequently the pattern is considered invalid as the inner or-patterns do not bind to the same set of identifiers. This is expected but the compiler errors that follow could be surprising or confusing to some users. This commit adds a help note explaining that if the user desired to match against variants or consts, they should use a qualified path. The note is restricted to cases where the identifier starts with an upper-case sequence so as to reduce the false negatives. Since this happens during resolution, there's no clean way to check what the patterns match against. The syntactic criterium, however, is in line with the convention that's assumed by the `non-camel-case-types` lint.
1 parent 2628f57 commit 7e0d1db

File tree

3 files changed

+156
-46
lines changed

3 files changed

+156
-46
lines changed

‎src/librustc_resolve/lib.rs‎

Lines changed: 43 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ struct BindingError {
146146
name: Name,
147147
origin: BTreeSet<Span>,
148148
target: BTreeSet<Span>,
149+
could_be_variant: bool
149150
}
150151

151152
impl PartialOrd for BindingError {
@@ -331,21 +332,30 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver<'_>,
331332
err
332333
}
333334
ResolutionError::VariableNotBoundInPattern(binding_error) => {
334-
let target_sp = binding_error.target.iter().cloned().collect::<Vec<_>>();
335+
let BindingError { name, target, origin, could_be_variant } = binding_error;
336+
337+
let target_sp = target.iter().cloned().collect::<Vec<_>>();
338+
let origin_sp = origin.iter().cloned().collect::<Vec<_>>();
339+
335340
let msp = MultiSpan::from_spans(target_sp.clone());
336-
let msg = format!("variable `{}` is not bound in all patterns", binding_error.name);
341+
let msg = format!("variable `{}` is not bound in all patterns", name);
337342
let mut err = resolver.session.struct_span_err_with_code(
338343
msp,
339344
&msg,
340345
DiagnosticId::Error("E0408".into()),
341346
);
342347
for sp in target_sp {
343-
err.span_label(sp, format!("pattern doesn't bind `{}`", binding_error.name));
348+
err.span_label(sp, format!("pattern doesn't bind `{}`", name));
344349
}
345-
let origin_sp = binding_error.origin.iter().cloned();
346350
for sp in origin_sp {
347351
err.span_label(sp, "variable not in all patterns");
348352
}
353+
if *could_be_variant {
354+
let help_msg = format!(
355+
"if you meant to match on a variant or a const, consider \
356+
making the path in the pattern qualified: `?::{}`", name);
357+
err.span_help(span, &help_msg);
358+
}
349359
err
350360
}
351361
ResolutionError::VariableBoundWithDifferentMode(variable_name,
@@ -3181,61 +3191,53 @@ impl<'a> Resolver<'a> {
31813191
// Checks that all of the arms in an or-pattern have exactly the
31823192
// same set of bindings, with the same binding modes for each.
31833193
fn check_consistent_bindings(&mut self, pats: &[P<Pat>]) {
3184-
if pats.is_empty() {
3194+
if pats.len() <= 1 {
31853195
return;
31863196
}
31873197

31883198
let mut missing_vars = FxHashMap::default();
31893199
let mut inconsistent_vars = FxHashMap::default();
3190-
for (i, p) in pats.iter().enumerate() {
3200+
for p in pats.iter() {
31913201
let map_i = self.binding_mode_map(&p);
3192-
3193-
for (j, q) in pats.iter().enumerate() {
3194-
if i == j {
3202+
for q in pats.iter() {
3203+
if p.id == q.id {
31953204
continue;
31963205
}
31973206

31983207
let map_j = self.binding_mode_map(&q);
3199-
for (&key, &binding_i) in &map_i {
3200-
if map_j.is_empty() { // Account for missing bindings when
3201-
let binding_error = missing_vars // `map_j` has none.
3202-
.entry(key.name)
3203-
.or_insert(BindingError {
3204-
name: key.name,
3205-
origin: BTreeSet::new(),
3206-
target: BTreeSet::new(),
3207-
});
3208-
binding_error.origin.insert(binding_i.span);
3209-
binding_error.target.insert(q.span);
3210-
}
3211-
for (&key_j, &binding_j) in &map_j {
3212-
match map_i.get(&key_j) {
3213-
None => { // missing binding
3214-
let binding_error = missing_vars
3208+
for (&key_j, &binding_j) in map_j.iter() {
3209+
match map_i.get(&key_j) {
3210+
None => { // missing binding
3211+
let binding_error = missing_vars
3212+
.entry(key_j.name)
3213+
.or_insert(BindingError {
3214+
name: key_j.name,
3215+
origin: BTreeSet::new(),
3216+
target: BTreeSet::new(),
3217+
could_be_variant:
3218+
key_j.name.as_str().starts_with(char::is_uppercase)
3219+
});
3220+
binding_error.origin.insert(binding_j.span);
3221+
binding_error.target.insert(p.span);
3222+
}
3223+
Some(binding_i) => { // check consistent binding
3224+
if binding_i.binding_mode != binding_j.binding_mode {
3225+
inconsistent_vars
32153226
.entry(key_j.name)
3216-
.or_insert(BindingError {
3217-
name: key_j.name,
3218-
origin: BTreeSet::new(),
3219-
target: BTreeSet::new(),
3220-
});
3221-
binding_error.origin.insert(binding_j.span);
3222-
binding_error.target.insert(p.span);
3223-
}
3224-
Some(binding_i) => { // check consistent binding
3225-
if binding_i.binding_mode != binding_j.binding_mode {
3226-
inconsistent_vars
3227-
.entry(key.name)
3228-
.or_insert((binding_j.span, binding_i.span));
3229-
}
3227+
.or_insert((binding_j.span, binding_i.span));
32303228
}
32313229
}
32323230
}
32333231
}
32343232
}
32353233
}
3236-
let mut missing_vars = missing_vars.iter().collect::<Vec<_>>();
3234+
3235+
let mut missing_vars = missing_vars.iter_mut().collect::<Vec<_>>();
32373236
missing_vars.sort();
3238-
for (_, v) in missing_vars {
3237+
for (name, mut v) in missing_vars {
3238+
if inconsistent_vars.contains_key(name) {
3239+
v.could_be_variant = false;
3240+
}
32393241
resolve_error(self,
32403242
*v.origin.iter().next().unwrap(),
32413243
ResolutionError::VariableNotBoundInPattern(v));

‎src/test/ui/resolve/resolve-inconsistent-names.rs‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,36 @@
1+
#![allow(non_camel_case_types)]
2+
3+
enum E { A, B, c }
4+
5+
mod m {
6+
const CONST1: usize = 10;
7+
const Const2: usize = 20;
8+
}
9+
110
fn main() {
211
let y = 1;
312
match y {
413
a | b => {} //~ ERROR variable `a` is not bound in all patterns
5-
//~^ ERROR variable `b` is not bound in all patterns
14+
//~| ERROR variable `b` is not bound in all patterns
15+
}
16+
17+
let x = (E::A, E::B);
18+
match x {
19+
(A, B) | (ref B, c) | (c, A) => ()
20+
//~^ ERROR variable `A` is not bound in all patterns
21+
//~| ERROR variable `B` is not bound in all patterns
22+
//~| ERROR variable `B` is bound in inconsistent ways
23+
//~| ERROR mismatched types
24+
//~| ERROR variable `c` is not bound in all patterns
25+
//~| HELP consider making the path in the pattern qualified: `?::A`
26+
}
27+
28+
let z = (10, 20);
29+
match z {
30+
(CONST1, _) | (_, Const2) => ()
31+
//~^ ERROR variable `CONST1` is not bound in all patterns
32+
//~| HELP consider making the path in the pattern qualified: `?::CONST1`
33+
//~| ERROR variable `Const2` is not bound in all patterns
34+
//~| HELP consider making the path in the pattern qualified: `?::Const2`
635
}
736
}

‎src/test/ui/resolve/resolve-inconsistent-names.stderr‎

Lines changed: 83 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,98 @@
11
error[E0408]: variable `a` is not bound in all patterns
2-
--> $DIR/resolve-inconsistent-names.rs:4:12
2+
--> $DIR/resolve-inconsistent-names.rs:13:12
33
|
44
LL | a | b => {}
55
| - ^ pattern doesn't bind `a`
66
| |
77
| variable not in all patterns
88

99
error[E0408]: variable `b` is not bound in all patterns
10-
--> $DIR/resolve-inconsistent-names.rs:4:8
10+
--> $DIR/resolve-inconsistent-names.rs:13:8
1111
|
1212
LL | a | b => {}
1313
| ^ - variable not in all patterns
1414
| |
1515
| pattern doesn't bind `b`
1616

17-
error: aborting due to 2 previous errors
17+
error[E0408]: variable `A` is not bound in all patterns
18+
--> $DIR/resolve-inconsistent-names.rs:19:18
19+
|
20+
LL | (A, B) | (ref B, c) | (c, A) => ()
21+
| - ^^^^^^^^^^ - variable not in all patterns
22+
| | |
23+
| | pattern doesn't bind `A`
24+
| variable not in all patterns
25+
|
26+
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::A`
27+
--> $DIR/resolve-inconsistent-names.rs:19:10
28+
|
29+
LL | (A, B) | (ref B, c) | (c, A) => ()
30+
| ^
31+
32+
error[E0408]: variable `B` is not bound in all patterns
33+
--> $DIR/resolve-inconsistent-names.rs:19:31
34+
|
35+
LL | (A, B) | (ref B, c) | (c, A) => ()
36+
| - - ^^^^^^ pattern doesn't bind `B`
37+
| | |
38+
| | variable not in all patterns
39+
| variable not in all patterns
40+
41+
error[E0408]: variable `c` is not bound in all patterns
42+
--> $DIR/resolve-inconsistent-names.rs:19:9
43+
|
44+
LL | (A, B) | (ref B, c) | (c, A) => ()
45+
| ^^^^^^ - - variable not in all patterns
46+
| | |
47+
| | variable not in all patterns
48+
| pattern doesn't bind `c`
49+
50+
error[E0409]: variable `B` is bound in inconsistent ways within the same match arm
51+
--> $DIR/resolve-inconsistent-names.rs:19:23
52+
|
53+
LL | (A, B) | (ref B, c) | (c, A) => ()
54+
| - ^ bound in different ways
55+
| |
56+
| first binding
57+
58+
error[E0408]: variable `CONST1` is not bound in all patterns
59+
--> $DIR/resolve-inconsistent-names.rs:30:23
60+
|
61+
LL | (CONST1, _) | (_, Const2) => ()
62+
| ------ ^^^^^^^^^^^ pattern doesn't bind `CONST1`
63+
| |
64+
| variable not in all patterns
65+
|
66+
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::CONST1`
67+
--> $DIR/resolve-inconsistent-names.rs:30:10
68+
|
69+
LL | (CONST1, _) | (_, Const2) => ()
70+
| ^^^^^^
71+
72+
error[E0408]: variable `Const2` is not bound in all patterns
73+
--> $DIR/resolve-inconsistent-names.rs:30:9
74+
|
75+
LL | (CONST1, _) | (_, Const2) => ()
76+
| ^^^^^^^^^^^ ------ variable not in all patterns
77+
| |
78+
| pattern doesn't bind `Const2`
79+
|
80+
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::Const2`
81+
--> $DIR/resolve-inconsistent-names.rs:30:27
82+
|
83+
LL | (CONST1, _) | (_, Const2) => ()
84+
| ^^^^^^
85+
86+
error[E0308]: mismatched types
87+
--> $DIR/resolve-inconsistent-names.rs:19:19
88+
|
89+
LL | (A, B) | (ref B, c) | (c, A) => ()
90+
| ^^^^^ expected enum `E`, found &E
91+
|
92+
= note: expected type `E`
93+
found type `&E`
94+
95+
error: aborting due to 9 previous errors
1896

19-
For more information about this error, try `rustc --explain E0408`.
97+
Some errors have detailed explanations: E0308, E0408, E0409.
98+
For more information about an error, try `rustc --explain E0308`.

0 commit comments

Comments
 (0)