about summary refs log tree commit diff
diff options
context:
space:
mode:
authorJakub Bukaj <jakub@jakub.cc>2014-11-20 16:57:36 +0100
committerJakub Bukaj <jakub@jakub.cc>2014-11-26 22:21:52 +0000
commit5804a306868aee5ff0a3d7829db7924978317f0e (patch)
treef269799695b88cd0d00df02549ad77bfa7faf5b1
parent6faff24ec85744de092a7d7c2378370f65d623bb (diff)
downloadrust-5804a306868aee5ff0a3d7829db7924978317f0e.tar.gz
rust-5804a306868aee5ff0a3d7829db7924978317f0e.zip
Warn on pattern bindings that have the same name as a variant
...of the type being matched.

This change will result in a better diagnostic for code like the following:

```rust
enum Enum {
    Foo,
    Bar
}

fn f(x: Enum) {
    match x {
        Foo => (),
        Bar => ()
    }
}
```

which would currently simply fail with an unreachable pattern error
on the 2nd arm.

The user is advised to either use a qualified path in the patterns
or import the variants explicitly into the scope.
-rw-r--r--src/librustc/diagnostics.rs3
-rw-r--r--src/librustc/middle/check_match.rs84
-rw-r--r--src/test/compile-fail/issue-12116.rs3
-rw-r--r--src/test/compile-fail/issue-14221.rs2
-rw-r--r--src/test/compile-fail/lint-uppercase-variables.rs3
-rw-r--r--src/test/run-pass/issue-19100.rs34
6 files changed, 100 insertions, 29 deletions
diff --git a/src/librustc/diagnostics.rs b/src/librustc/diagnostics.rs
index afbb18faa0b..28504210a3f 100644
--- a/src/librustc/diagnostics.rs
+++ b/src/librustc/diagnostics.rs
@@ -145,5 +145,6 @@ register_diagnostics!(
     E0166,
     E0167,
     E0168,
-    E0169
+    E0169,
+    E0170
 )
diff --git a/src/librustc/middle/check_match.rs b/src/librustc/middle/check_match.rs
index bcfc003480f..ed119081f78 100644
--- a/src/librustc/middle/check_match.rs
+++ b/src/librustc/middle/check_match.rs
@@ -153,19 +153,14 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) {
     visit::walk_expr(cx, ex);
     match ex.node {
         ast::ExprMatch(ref scrut, ref arms, source) => {
-            // First, check legality of move bindings.
             for arm in arms.iter() {
+                // First, check legality of move bindings.
                 check_legality_of_move_bindings(cx,
                                                 arm.guard.is_some(),
                                                 arm.pats.as_slice());
-                for pat in arm.pats.iter() {
-                    check_legality_of_bindings_in_at_patterns(cx, &**pat);
-                }
-            }
 
-            // Second, if there is a guard on each arm, make sure it isn't
-            // assigning or borrowing anything mutably.
-            for arm in arms.iter() {
+                // Second, if there is a guard on each arm, make sure it isn't
+                // assigning or borrowing anything mutably.
                 match arm.guard {
                     Some(ref guard) => check_for_mutation_in_guard(cx, &**guard),
                     None => {}
@@ -179,13 +174,23 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &ast::Expr) {
                 }).collect(), arm.guard.as_ref().map(|e| &**e))
             }).collect::<Vec<(Vec<P<Pat>>, Option<&ast::Expr>)>>();
 
+            // Bail out early if inlining failed.
             if static_inliner.failed {
                 return;
             }
 
-            // Third, check if there are any references to NaN that we should warn about.
-            for &(ref pats, _) in inlined_arms.iter() {
-                check_for_static_nan(cx, pats.as_slice());
+            for pat in inlined_arms
+                .iter()
+                .flat_map(|&(ref pats, _)| pats.iter()) {
+                // Third, check legality of move bindings.
+                check_legality_of_bindings_in_at_patterns(cx, &**pat);
+
+                // Fourth, check if there are any references to NaN that we should warn about.
+                check_for_static_nan(cx, &**pat);
+
+                // Fifth, check if for any of the patterns that match an enumerated type
+                // are bindings with the same name as one of the variants of said type.
+                check_for_bindings_named_the_same_as_variants(cx, &**pat);
             }
 
             // Fourth, check for unreachable arms.
@@ -239,21 +244,49 @@ fn is_expr_const_nan(tcx: &ty::ctxt, expr: &ast::Expr) -> bool {
     }
 }
 
-// Check that we do not match against a static NaN (#6804)
-fn check_for_static_nan(cx: &MatchCheckCtxt, pats: &[P<Pat>]) {
-    for pat in pats.iter() {
-        walk_pat(&**pat, |p| {
-            match p.node {
-                ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => {
-                    span_warn!(cx.tcx.sess, p.span, E0003,
-                        "unmatchable NaN in pattern, \
-                            use the is_nan method in a guard instead");
+fn check_for_bindings_named_the_same_as_variants(cx: &MatchCheckCtxt, pat: &Pat) {
+    walk_pat(pat, |p| {
+        match p.node {
+            ast::PatIdent(ast::BindByValue(ast::MutImmutable), ident, None) => {
+                let pat_ty = ty::pat_ty(cx.tcx, p);
+                if let ty::ty_enum(def_id, _) = pat_ty.sty {
+                    let def = cx.tcx.def_map.borrow().get(&p.id).cloned();
+                    if let Some(DefLocal(_)) = def {
+                        if ty::enum_variants(cx.tcx, def_id).iter().any(|variant|
+                            token::get_name(variant.name) == token::get_name(ident.node.name)
+                                && variant.args.len() == 0
+                        ) {
+                            span_warn!(cx.tcx.sess, p.span, E0170,
+                                "pattern binding `{}` is named the same as one \
+                                 of the variants of the type `{}`",
+                                token::get_ident(ident.node).get(), ty_to_string(cx.tcx, pat_ty));
+                            span_help!(cx.tcx.sess, p.span,
+                                "if you meant to match on a variant, \
+                                 consider making the path in the pattern qualified: `{}::{}`",
+                                ty_to_string(cx.tcx, pat_ty), token::get_ident(ident.node).get());
+                        }
+                    }
                 }
-                _ => ()
             }
-            true
-        });
-    }
+            _ => ()
+        }
+        true
+    });
+}
+
+// Check that we do not match against a static NaN (#6804)
+fn check_for_static_nan(cx: &MatchCheckCtxt, pat: &Pat) {
+    walk_pat(pat, |p| {
+        match p.node {
+            ast::PatLit(ref expr) if is_expr_const_nan(cx.tcx, &**expr) => {
+                span_warn!(cx.tcx.sess, p.span, E0003,
+                    "unmatchable NaN in pattern, \
+                        use the is_nan method in a guard instead");
+            }
+            _ => ()
+        }
+        true
+    });
 }
 
 // Check for unreachable patterns
@@ -414,8 +447,7 @@ fn construct_witness(cx: &MatchCheckCtxt, ctor: &Constructor,
                 &Variant(vid) =>
                     (vid, ty::enum_variant_with_id(cx.tcx, cid, vid).arg_names.is_some()),
                 _ =>
-                    (cid, ty::lookup_struct_fields(cx.tcx, cid).iter()
-                        .any(|field| field.name != token::special_idents::unnamed_field.name))
+                    (cid, !ty::is_tuple_struct(cx.tcx, cid))
             };
             if is_structure {
                 let fields = ty::lookup_struct_fields(cx.tcx, vid);
diff --git a/src/test/compile-fail/issue-12116.rs b/src/test/compile-fail/issue-12116.rs
index 59e29516a74..47907ac2be9 100644
--- a/src/test/compile-fail/issue-12116.rs
+++ b/src/test/compile-fail/issue-12116.rs
@@ -17,7 +17,8 @@ fn tail(source_list: &IntList) -> IntList {
     match source_list {
         &IntList::Cons(val, box ref next_list) => tail(next_list),
         &IntList::Cons(val, box Nil)           => IntList::Cons(val, box Nil),
-        //~^ ERROR: unreachable pattern
+//~^ ERROR unreachable pattern
+//~^^ WARN pattern binding `Nil` is named the same as one of the variants of the type `IntList`
         _                          => panic!()
     }
 }
diff --git a/src/test/compile-fail/issue-14221.rs b/src/test/compile-fail/issue-14221.rs
index c58012dc84c..e79be99a346 100644
--- a/src/test/compile-fail/issue-14221.rs
+++ b/src/test/compile-fail/issue-14221.rs
@@ -17,7 +17,9 @@ pub mod b {
     pub fn key(e: ::E) -> &'static str {
         match e {
             A => "A",
+//~^ WARN pattern binding `A` is named the same as one of the variants of the type `E`
             B => "B", //~ ERROR: unreachable pattern
+//~^ WARN pattern binding `B` is named the same as one of the variants of the type `E`
         }
     }
 }
diff --git a/src/test/compile-fail/lint-uppercase-variables.rs b/src/test/compile-fail/lint-uppercase-variables.rs
index 2f840ee0761..eb5c475e7ef 100644
--- a/src/test/compile-fail/lint-uppercase-variables.rs
+++ b/src/test/compile-fail/lint-uppercase-variables.rs
@@ -33,7 +33,8 @@ fn main() {
     match f.read(&mut buff) {
         Ok(cnt) => println!("read this many bytes: {}", cnt),
         Err(IoError{ kind: EndOfFile, .. }) => println!("Got end of file: {}", EndOfFile.to_string()),
-        //~^ ERROR variable `EndOfFile` should have a snake case name such as `end_of_file`
+//~^ ERROR variable `EndOfFile` should have a snake case name such as `end_of_file`
+//~^^ WARN `EndOfFile` is named the same as one of the variants of the type `std::io::IoErrorKind`
     }
 
     test(1);
diff --git a/src/test/run-pass/issue-19100.rs b/src/test/run-pass/issue-19100.rs
new file mode 100644
index 00000000000..cee5c808f99
--- /dev/null
+++ b/src/test/run-pass/issue-19100.rs
@@ -0,0 +1,34 @@
+// Copyright 2014 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+enum Foo {
+    Bar,
+    Baz
+}
+
+impl Foo {
+    fn foo(&self) {
+        match self {
+            &
+Bar if true
+//~^ WARN pattern binding `Bar` is named the same as one of the variants of the type `Foo`
+//~^^ HELP to match on a variant, consider making the path in the pattern qualified: `Foo::Bar`
+=> println!("bar"),
+            &
+Baz if false
+//~^ WARN pattern binding `Baz` is named the same as one of the variants of the type `Foo`
+//~^^ HELP to match on a variant, consider making the path in the pattern qualified: `Foo::Baz`
+=> println!("baz"),
+_ => ()
+        }
+    }
+}
+
+fn main() {}