Skip to content

Commit

Permalink
Auto merge of #63406 - jakubadamw:resolve-inconsistent-names-suggest-…
Browse files Browse the repository at this point in the history 
…qualified-path, r=<try>

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.
  • Loading branch information
@bors
bors committed Aug 9, 2019
2 parents d8f8be4 + 7e0d1db commit 8261083
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 46 deletions.
84 changes: 43 additions & 41 deletions src/librustc_resolve/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ struct BindingError {
name: Name,
origin: BTreeSet<Span>,
target: BTreeSet<Span>,
could_be_variant: bool
}

impl PartialOrd for BindingError {
Expand Down Expand Up @@ -331,21 +332,30 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver<'_>,
err
}
ResolutionError::VariableNotBoundInPattern(binding_error) => {
let target_sp = binding_error.target.iter().cloned().collect::<Vec<_>>();
let BindingError { name, target, origin, could_be_variant } = binding_error;

let target_sp = target.iter().cloned().collect::<Vec<_>>();
let origin_sp = origin.iter().cloned().collect::<Vec<_>>();

let msp = MultiSpan::from_spans(target_sp.clone());
let msg = format!("variable `{}` is not bound in all patterns", binding_error.name);
let msg = format!("variable `{}` is not bound in all patterns", name);
let mut err = resolver.session.struct_span_err_with_code(
msp,
&msg,
DiagnosticId::Error("E0408".into()),
);
for sp in target_sp {
err.span_label(sp, format!("pattern doesn't bind `{}`", binding_error.name));
err.span_label(sp, format!("pattern doesn't bind `{}`", name));
}
let origin_sp = binding_error.origin.iter().cloned();
for sp in origin_sp {
err.span_label(sp, "variable not in all patterns");
}
if *could_be_variant {
let help_msg = format!(
"if you meant to match on a variant or a const, consider \
making the path in the pattern qualified: `?::{}`", name);
err.span_help(span, &help_msg);
}
err
}
ResolutionError::VariableBoundWithDifferentMode(variable_name,
Expand Down Expand Up @@ -3181,61 +3191,53 @@ impl<'a> Resolver<'a> {
// Checks that all of the arms in an or-pattern have exactly the
// same set of bindings, with the same binding modes for each.
fn check_consistent_bindings(&mut self, pats: &[P<Pat>]) {
if pats.is_empty() {
if pats.len() <= 1 {
return;
}

let mut missing_vars = FxHashMap::default();
let mut inconsistent_vars = FxHashMap::default();
for (i, p) in pats.iter().enumerate() {
for p in pats.iter() {
let map_i = self.binding_mode_map(&p);

for (j, q) in pats.iter().enumerate() {
if i == j {
for q in pats.iter() {
if p.id == q.id {
continue;
}

let map_j = self.binding_mode_map(&q);
for (&key, &binding_i) in &map_i {
if map_j.is_empty() { // Account for missing bindings when
let binding_error = missing_vars // `map_j` has none.
.entry(key.name)
.or_insert(BindingError {
name: key.name,
origin: BTreeSet::new(),
target: BTreeSet::new(),
});
binding_error.origin.insert(binding_i.span);
binding_error.target.insert(q.span);
}
for (&key_j, &binding_j) in &map_j {
match map_i.get(&key_j) {
None => { // missing binding
let binding_error = missing_vars
for (&key_j, &binding_j) in map_j.iter() {
match map_i.get(&key_j) {
None => { // missing binding
let binding_error = missing_vars
.entry(key_j.name)
.or_insert(BindingError {
name: key_j.name,
origin: BTreeSet::new(),
target: BTreeSet::new(),
could_be_variant:
key_j.name.as_str().starts_with(char::is_uppercase)
});
binding_error.origin.insert(binding_j.span);
binding_error.target.insert(p.span);
}
Some(binding_i) => { // check consistent binding
if binding_i.binding_mode != binding_j.binding_mode {
inconsistent_vars
.entry(key_j.name)
.or_insert(BindingError {
name: key_j.name,
origin: BTreeSet::new(),
target: BTreeSet::new(),
});
binding_error.origin.insert(binding_j.span);
binding_error.target.insert(p.span);
}
Some(binding_i) => { // check consistent binding
if binding_i.binding_mode != binding_j.binding_mode {
inconsistent_vars
.entry(key.name)
.or_insert((binding_j.span, binding_i.span));
}
.or_insert((binding_j.span, binding_i.span));
}
}
}
}
}
}
let mut missing_vars = missing_vars.iter().collect::<Vec<_>>();

let mut missing_vars = missing_vars.iter_mut().collect::<Vec<_>>();
missing_vars.sort();
for (_, v) in missing_vars {
for (name, mut v) in missing_vars {
if inconsistent_vars.contains_key(name) {
v.could_be_variant = false;
}
resolve_error(self,
*v.origin.iter().next().unwrap(),
ResolutionError::VariableNotBoundInPattern(v));
Expand Down
31 changes: 30 additions & 1 deletion src/test/ui/resolve/resolve-inconsistent-names.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,36 @@
#![allow(non_camel_case_types)]

enum E { A, B, c }

mod m {
const CONST1: usize = 10;
const Const2: usize = 20;
}

fn main() {
let y = 1;
match y {
a | b => {} //~ ERROR variable `a` is not bound in all patterns
//~^ ERROR variable `b` is not bound in all patterns
//~| ERROR variable `b` is not bound in all patterns
}

let x = (E::A, E::B);
match x {
(A, B) | (ref B, c) | (c, A) => ()
//~^ ERROR variable `A` is not bound in all patterns
//~| ERROR variable `B` is not bound in all patterns
//~| ERROR variable `B` is bound in inconsistent ways
//~| ERROR mismatched types
//~| ERROR variable `c` is not bound in all patterns
//~| HELP consider making the path in the pattern qualified: `?::A`
}

let z = (10, 20);
match z {
(CONST1, _) | (_, Const2) => ()
//~^ ERROR variable `CONST1` is not bound in all patterns
//~| HELP consider making the path in the pattern qualified: `?::CONST1`
//~| ERROR variable `Const2` is not bound in all patterns
//~| HELP consider making the path in the pattern qualified: `?::Const2`
}
}
87 changes: 83 additions & 4 deletions src/test/ui/resolve/resolve-inconsistent-names.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,19 +1,98 @@
error[E0408]: variable `a` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:4:12
--> $DIR/resolve-inconsistent-names.rs:13:12
|
LL | a | b => {}
| - ^ pattern doesn't bind `a`
| |
| variable not in all patterns

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

error: aborting due to 2 previous errors
error[E0408]: variable `A` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:19:18
|
LL | (A, B) | (ref B, c) | (c, A) => ()
| - ^^^^^^^^^^ - variable not in all patterns
| | |
| | pattern doesn't bind `A`
| variable not in all patterns
|
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::A`
--> $DIR/resolve-inconsistent-names.rs:19:10
|
LL | (A, B) | (ref B, c) | (c, A) => ()
| ^

error[E0408]: variable `B` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:19:31
|
LL | (A, B) | (ref B, c) | (c, A) => ()
| - - ^^^^^^ pattern doesn't bind `B`
| | |
| | variable not in all patterns
| variable not in all patterns

error[E0408]: variable `c` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:19:9
|
LL | (A, B) | (ref B, c) | (c, A) => ()
| ^^^^^^ - - variable not in all patterns
| | |
| | variable not in all patterns
| pattern doesn't bind `c`

error[E0409]: variable `B` is bound in inconsistent ways within the same match arm
--> $DIR/resolve-inconsistent-names.rs:19:23
|
LL | (A, B) | (ref B, c) | (c, A) => ()
| - ^ bound in different ways
| |
| first binding

error[E0408]: variable `CONST1` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:30:23
|
LL | (CONST1, _) | (_, Const2) => ()
| ------ ^^^^^^^^^^^ pattern doesn't bind `CONST1`
| |
| variable not in all patterns
|
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::CONST1`
--> $DIR/resolve-inconsistent-names.rs:30:10
|
LL | (CONST1, _) | (_, Const2) => ()
| ^^^^^^

error[E0408]: variable `Const2` is not bound in all patterns
--> $DIR/resolve-inconsistent-names.rs:30:9
|
LL | (CONST1, _) | (_, Const2) => ()
| ^^^^^^^^^^^ ------ variable not in all patterns
| |
| pattern doesn't bind `Const2`
|
help: if you meant to match on a variant or a const, consider making the path in the pattern qualified: `?::Const2`
--> $DIR/resolve-inconsistent-names.rs:30:27
|
LL | (CONST1, _) | (_, Const2) => ()
| ^^^^^^

error[E0308]: mismatched types
--> $DIR/resolve-inconsistent-names.rs:19:19
|
LL | (A, B) | (ref B, c) | (c, A) => ()
| ^^^^^ expected enum `E`, found &E
|
= note: expected type `E`
found type `&E`

error: aborting due to 9 previous errors

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

0 comments on commit 8261083

Please sign in to comment.