Skip to content

Commit 83e1e1f

Browse files
CentrilCentril
authored
Rollup merge of #63406 - jakubadamw:resolve-inconsistent-names-suggest-qualified-path, r=petrochenkov
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 help 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 it is the patterns match against. The syntactic criterium, however, is in line with the convention that's assumed by the `non-camel-case-types` lint. Fixes #50831.
2 parents f049636 + 30db4eb commit 83e1e1f

File tree

5 files changed

+168
-58
lines changed

5 files changed

+168
-58
lines changed

‎src/librustc_resolve/diagnostics.rs

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ use syntax_pos::{BytePos, Span, MultiSpan};
2020

2121
use crate::resolve_imports::{ImportDirective, ImportDirectiveSubclass, ImportResolver};
2222
use crate::{path_names_to_string, KNOWN_TOOLS};
23-
use crate::{CrateLint, LegacyScope, Module, ModuleOrUniformRoot};
23+
use crate::{BindingError, CrateLint, LegacyScope, Module, ModuleOrUniformRoot};
2424
use crate::{PathResult, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Segment};
2525

2626
type Res = def::Res<ast::NodeId>;
@@ -207,21 +207,32 @@ impl<'a> Resolver<'a> {
207207
err
208208
}
209209
ResolutionError::VariableNotBoundInPattern(binding_error) => {
210-
let target_sp = binding_error.target.iter().cloned().collect::<Vec<_>>();
210+
let BindingError { name, target, origin, could_be_path } = binding_error;
211+
212+
let target_sp = target.iter().copied().collect::<Vec<_>>();
213+
let origin_sp = origin.iter().copied().collect::<Vec<_>>();
214+
211215
let msp = MultiSpan::from_spans(target_sp.clone());
212-
let msg = format!("variable `{}` is not bound in all patterns", binding_error.name);
216+
let msg = format!("variable `{}` is not bound in all patterns", name);
213217
let mut err = self.session.struct_span_err_with_code(
214218
msp,
215219
&msg,
216220
DiagnosticId::Error("E0408".into()),
217221
);
218222
for sp in target_sp {
219-
err.span_label(sp, format!("pattern doesn't bind `{}`", binding_error.name));
223+
err.span_label(sp, format!("pattern doesn't bind `{}`", name));
220224
}
221-
let origin_sp = binding_error.origin.iter().cloned();
222225
for sp in origin_sp {
223226
err.span_label(sp, "variable not in all patterns");
224227
}
228+
if *could_be_path {
229+
let help_msg = format!(
230+
"if you meant to match on a variant or a `const` item, consider \
231+
making the path in the pattern qualified: `?::{}`",
232+
name,
233+
);
234+
err.span_help(span, &help_msg);
235+
}
225236
err
226237
}
227238
ResolutionError::VariableBoundWithDifferentMode(variable_name,

‎src/librustc_resolve/late.rs

Lines changed: 38 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,65 +1136,53 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
11361136
// Checks that all of the arms in an or-pattern have exactly the
11371137
// same set of bindings, with the same binding modes for each.
11381138
fn check_consistent_bindings(&mut self, pats: &[P<Pat>]) {
1139-
if pats.is_empty() {
1140-
return;
1141-
}
1142-
11431139
let mut missing_vars = FxHashMap::default();
11441140
let mut inconsistent_vars = FxHashMap::default();
1145-
for (i, p) in pats.iter().enumerate() {
1146-
let map_i = self.binding_mode_map(&p);
1147-
1148-
for (j, q) in pats.iter().enumerate() {
1149-
if i == j {
1150-
continue;
1151-
}
11521141

1153-
let map_j = self.binding_mode_map(&q);
1154-
for (&key, &binding_i) in &map_i {
1155-
if map_j.is_empty() { // Account for missing bindings when
1156-
let binding_error = missing_vars // `map_j` has none.
1157-
.entry(key.name)
1158-
.or_insert(BindingError {
1159-
name: key.name,
1160-
origin: BTreeSet::new(),
1161-
target: BTreeSet::new(),
1162-
});
1163-
binding_error.origin.insert(binding_i.span);
1164-
binding_error.target.insert(q.span);
1165-
}
1166-
for (&key_j, &binding_j) in &map_j {
1167-
match map_i.get(&key_j) {
1168-
None => { // missing binding
1169-
let binding_error = missing_vars
1170-
.entry(key_j.name)
1171-
.or_insert(BindingError {
1172-
name: key_j.name,
1173-
origin: BTreeSet::new(),
1174-
target: BTreeSet::new(),
1175-
});
1176-
binding_error.origin.insert(binding_j.span);
1177-
binding_error.target.insert(p.span);
1178-
}
1179-
Some(binding_i) => { // check consistent binding
1180-
if binding_i.binding_mode != binding_j.binding_mode {
1181-
inconsistent_vars
1182-
.entry(key.name)
1183-
.or_insert((binding_j.span, binding_i.span));
1184-
}
1142+
for pat_outer in pats.iter() {
1143+
let map_outer = self.binding_mode_map(&pat_outer);
1144+
1145+
for pat_inner in pats.iter().filter(|pat| pat.id != pat_outer.id) {
1146+
let map_inner = self.binding_mode_map(&pat_inner);
1147+
1148+
for (&key_inner, &binding_inner) in map_inner.iter() {
1149+
match map_outer.get(&key_inner) {
1150+
None => { // missing binding
1151+
let binding_error = missing_vars
1152+
.entry(key_inner.name)
1153+
.or_insert(BindingError {
1154+
name: key_inner.name,
1155+
origin: BTreeSet::new(),
1156+
target: BTreeSet::new(),
1157+
could_be_path:
1158+
key_inner.name.as_str().starts_with(char::is_uppercase)
1159+
});
1160+
binding_error.origin.insert(binding_inner.span);
1161+
binding_error.target.insert(pat_outer.span);
1162+
}
1163+
Some(binding_outer) => { // check consistent binding
1164+
if binding_outer.binding_mode != binding_inner.binding_mode {
1165+
inconsistent_vars
1166+
.entry(key_inner.name)
1167+
.or_insert((binding_inner.span, binding_outer.span));
11851168
}
11861169
}
11871170
}
11881171
}
11891172
}
11901173
}
1191-
let mut missing_vars = missing_vars.iter().collect::<Vec<_>>();
1174+
1175+
let mut missing_vars = missing_vars.iter_mut().collect::<Vec<_>>();
11921176
missing_vars.sort();
1193-
for (_, v) in missing_vars {
1177+
for (name, mut v) in missing_vars {
1178+
if inconsistent_vars.contains_key(name) {
1179+
v.could_be_path = false;
1180+
}
11941181
self.r.report_error(
1195-
*v.origin.iter().next().unwrap(), ResolutionError::VariableNotBoundInPattern(v)
1196-
);
1182+
*v.origin.iter().next().unwrap(),
1183+
ResolutionError::VariableNotBoundInPattern(v));
11971184
}
1185+
11981186
let mut inconsistent_vars = inconsistent_vars.iter().collect::<Vec<_>>();
11991187
inconsistent_vars.sort();
12001188
for (name, v) in inconsistent_vars {
@@ -1222,7 +1210,9 @@ impl<'a, 'b> LateResolutionVisitor<'a, '_> {
12221210
self.resolve_pattern(pat, source, &mut bindings_list);
12231211
}
12241212
// This has to happen *after* we determine which pat_idents are variants
1225-
self.check_consistent_bindings(pats);
1213+
if pats.len() > 1 {
1214+
self.check_consistent_bindings(pats);
1215+
}
12261216
}
12271217

12281218
fn resolve_block(&mut self, block: &Block) {

‎src/librustc_resolve/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ struct BindingError {
135135
name: Name,
136136
origin: BTreeSet<Span>,
137137
target: BTreeSet<Span>,
138+
could_be_path: bool
138139
}
139140

140141
impl PartialOrd for BindingError {

‎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` item, 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` item, 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` item, 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)