Produce a better error for irrefutable if let patterns

Modify ast::ExprMatch to include a new value of type ast::MatchSource,
making it easy to tell whether the match was written literally or
produced via desugaring. This allows us to customize error messages
appropriately.

src/librustc/diagnostics.rs

@@ -19,6 +19,11 @@ register_diagnostic!(E0001, r##"
     one is too specific or the ordering is incorrect.
 "##)
 
+register_diagnostic!(E0162, r##"
+    This error is produced by an `if let` expression where the pattern is irrefutable.
+    An `if let` that can never fail is considered an error.
+"##)
+
 register_diagnostics!(
     E0002,
     E0003,

src/librustc/lint/builtin.rs

@@ -1091,7 +1091,10 @@ impl LintPass for UnnecessaryParens {
         let (value, msg, struct_lit_needs_parens) = match e.node {
             ast::ExprIf(ref cond, _, _) => (cond, "`if` condition", true),
             ast::ExprWhile(ref cond, _, _) => (cond, "`while` condition", true),
-            ast::ExprMatch(ref head, _) => (head, "`match` head expression", true),
+            ast::ExprMatch(ref head, _, source) => match source {
+                ast::MatchNormal => (head, "`match` head expression", true),
+                ast::MatchIfLetDesugar => (head, "`if let` head expression", true)
+            },
             ast::ExprRet(Some(ref value)) => (value, "`return` value", false),
             ast::ExprAssign(_, ref value) => (value, "assigned value", false),
             ast::ExprAssignOp(_, _, ref value) => (value, "assigned value", false),
@@ -1241,7 +1244,7 @@ impl LintPass for UnusedMut {
 
     fn check_expr(&mut self, cx: &Context, e: &ast::Expr) {
         match e.node {
-            ast::ExprMatch(_, ref arms) => {
+            ast::ExprMatch(_, ref arms, _) => {
                 for a in arms.iter() {
                     self.check_unused_mut_pat(cx, a.pats.as_slice())
                 }

src/librustc/middle/cfg/construct.rs

@@ -324,7 +324,7 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> {
                 expr_exit
             }
 
-            ast::ExprMatch(ref discr, ref arms) => {
+            ast::ExprMatch(ref discr, ref arms, _) => {
                 //
                 //     [pred]
                 //       |

src/librustc/middle/check_match.rs

@@ -147,7 +147,7 @@ pub fn check_crate(tcx: &ty::ctxt) {
 fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
     visit::walk_expr(cx, ex);
     match ex.node {
-        ExprMatch(ref scrut, ref arms) => {
+        ExprMatch(ref scrut, ref arms, source) => {
             // First, check legality of move bindings.
             for arm in arms.iter() {
                 check_legality_of_move_bindings(cx,
@@ -184,7 +184,7 @@ fn check_expr(cx: &mut MatchCheckCtxt, ex: &Expr) {
             }
 
             // Fourth, check for unreachable arms.
-            check_arms(cx, inlined_arms.as_slice());
+            check_arms(cx, inlined_arms.as_slice(), source);
 
             // Finally, check if the whole match expression is exhaustive.
             // Check for empty enum, because is_useful only works on inhabited types.
@@ -252,13 +252,31 @@ fn check_for_static_nan(cx: &MatchCheckCtxt, pats: &[P<Pat>]) {
 }
 
 // Check for unreachable patterns
-fn check_arms(cx: &MatchCheckCtxt, arms: &[(Vec<P<Pat>>, Option<&Expr>)]) {
+fn check_arms(cx: &MatchCheckCtxt, arms: &[(Vec<P<Pat>>, Option<&Expr>)], source: MatchSource) {
     let mut seen = Matrix(vec![]);
+    let mut printed_if_let_err = false;
     for &(ref pats, guard) in arms.iter() {
         for pat in pats.iter() {
             let v = vec![&**pat];
+
             match is_useful(cx, &seen, v.as_slice(), LeaveOutWitness) {
-                NotUseful => span_err!(cx.tcx.sess, pat.span, E0001, "unreachable pattern"),
+                NotUseful => {
+                    if source == MatchIfLetDesugar {
+                        if printed_if_let_err {
+                            // we already printed an irrefutable if-let pattern error.
+                            // We don't want two, that's just confusing.
+                        } else {
+                            // find the first arm pattern so we can use its span
+                            let &(ref first_arm_pats, _) = &arms[0]; // we know there's at least 1 arm
+                            let first_pat = first_arm_pats.get(0); // and it's safe to assume 1 pat
+                            let span = first_pat.span;
+                            span_err!(cx.tcx.sess, span, E0162, "irrefutable if-let pattern");
+                            printed_if_let_err = true;
+                        }
+                    } else {
+                        span_err!(cx.tcx.sess, pat.span, E0001, "unreachable pattern");
+                    }
+                }
                 Useful => (),
                 UsefulWithWitness(_) => unreachable!()
             }

src/librustc/middle/expr_use_visitor.rs

@@ -376,7 +376,7 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,TYPER> {
 
             ast::ExprIfLet(..) => fail!("non-desugared ExprIfLet"),
 
-            ast::ExprMatch(ref discr, ref arms) => {
+            ast::ExprMatch(ref discr, ref arms, _) => {
                 let discr_cmt = return_if_err!(self.mc.cat_expr(&**discr));
                 self.borrow_expr(&**discr, ty::ReEmpty, ty::ImmBorrow, MatchDiscriminant);
 

src/librustc/middle/liveness.rs

@@ -1029,7 +1029,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
             self.propagate_through_loop(expr, LoopLoop, &**blk, succ)
           }
 
-          ExprMatch(ref e, ref arms) => {
+          ExprMatch(ref e, ref arms, _) => {
             //
             //      (e)
             //       |

src/librustc/middle/trans/debuginfo.rs

@@ -3659,7 +3659,7 @@ fn populate_scope_map(cx: &CrateContext,
                 }
             }
 
-            ast::ExprMatch(ref discriminant_exp, ref arms) => {
+            ast::ExprMatch(ref discriminant_exp, ref arms, _) => {
                 walk_expr(cx, &**discriminant_exp, scope_stack, scope_map);
 
                 // For each arm we have to first walk the pattern as these might

src/librustc/middle/trans/expr.rs

@@ -1013,7 +1013,7 @@ fn trans_rvalue_dps_unadjusted<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
         ast::ExprIf(ref cond, ref thn, ref els) => {
             controlflow::trans_if(bcx, expr.id, &**cond, &**thn, els.as_ref().map(|e| &**e), dest)
         }
-        ast::ExprMatch(ref discr, ref arms) => {
+        ast::ExprMatch(ref discr, ref arms, _) => {
             _match::trans_match(bcx, expr, &**discr, arms.as_slice(), dest)
         }
         ast::ExprBlock(ref blk) => {

src/librustc/middle/typeck/check/mod.rs

@@ -4144,7 +4144,7 @@ fn check_expr_with_unifier(fcx: &FnCtxt,
             fcx.write_nil(id);
         }
       }
-      ast::ExprMatch(ref discrim, ref arms) => {
+      ast::ExprMatch(ref discrim, ref arms, _) => {
         _match::check_match(fcx, expr, &**discrim, arms.as_slice());
       }
       ast::ExprFnBlock(_, ref decl, ref body) => {

src/librustc/middle/typeck/check/regionck.rs

@@ -725,7 +725,7 @@ fn visit_expr(rcx: &mut Rcx, expr: &ast::Expr) {
             visit::walk_expr(rcx, expr);
         }
 
-        ast::ExprMatch(ref discr, ref arms) => {
+        ast::ExprMatch(ref discr, ref arms, _) => {
             link_match(rcx, &**discr, arms.as_slice());
 
             visit::walk_expr(rcx, expr);

src/librustc/util/ppaux.rs

@@ -92,6 +92,7 @@ pub fn explain_region_and_span(cx: &ctxt, region: ty::Region)
               ast::ExprMethodCall(..) => {
                 explain_span(cx, "method call", expr.span)
               },
+              ast::ExprMatch(_, _, ast::MatchIfLetDesugar) => explain_span(cx, "if let", expr.span),
               ast::ExprMatch(..) => explain_span(cx, "match", expr.span),
               _ => explain_span(cx, "expression", expr.span)
             }

src/libsyntax/ast.rs

@@ -529,7 +529,7 @@ pub enum Expr_ {
     // Conditionless loop (can be exited with break, cont, or ret)
     // FIXME #6993: change to Option<Name> ... or not, if these are hygienic.
     ExprLoop(P<Block>, Option<Ident>),
-    ExprMatch(P<Expr>, Vec<Arm>),
+    ExprMatch(P<Expr>, Vec<Arm>, MatchSource),
     ExprFnBlock(CaptureClause, P<FnDecl>, P<Block>),
     ExprProc(P<FnDecl>, P<Block>),
     ExprUnboxedFn(CaptureClause, UnboxedClosureKind, P<FnDecl>, P<Block>),
@@ -577,6 +577,12 @@ pub struct QPath {
     pub item_name: Ident,
 }
 
+#[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash, Show)]
+pub enum MatchSource {
+    MatchNormal,
+    MatchIfLetDesugar
+}
+
 #[deriving(Clone, PartialEq, Eq, Encodable, Decodable, Hash, Show)]
 pub enum CaptureClause {
     CaptureByValue,

src/libsyntax/config.rs

@@ -210,10 +210,10 @@ fn fold_expr(cx: &mut Context, expr: P<ast::Expr>) -> P<ast::Expr> {
         fold::noop_fold_expr(ast::Expr {
             id: id,
             node: match node {
-                ast::ExprMatch(m, arms) => {
+                ast::ExprMatch(m, arms, source) => {
                     ast::ExprMatch(m, arms.into_iter()
                                         .filter(|a| (cx.in_cfg)(a.attrs.as_slice()))
-                                        .collect())
+                                        .collect(), source)
                 }
                 _ => node
             },

src/libsyntax/ext/build.rs

@@ -845,7 +845,7 @@ impl<'a> AstBuilder for ExtCtxt<'a> {
     }
 
     fn expr_match(&self, span: Span, arg: P<ast::Expr>, arms: Vec<ast::Arm>) -> P<Expr> {
-        self.expr(span, ast::ExprMatch(arg, arms))
+        self.expr(span, ast::ExprMatch(arg, arms, ast::MatchNormal))
     }
 
     fn expr_if(&self, span: Span, cond: P<ast::Expr>,

src/libsyntax/ext/expand.rs

@@ -127,7 +127,7 @@ pub fn expand_expr(e: P<ast::Expr>, fld: &mut MacroExpander) -> P<ast::Expr> {
             arms.push_all_move(else_if_arms);
             arms.push(else_arm);
 
-            let match_expr = fld.cx.expr_match(span, expr, arms);
+            let match_expr = fld.cx.expr(span, ast::ExprMatch(expr, arms, ast::MatchIfLetDesugar));
             fld.fold_expr(match_expr)
         }
 

src/libsyntax/fold.rs

@@ -1226,9 +1226,10 @@ pub fn noop_fold_expr<T: Folder>(Expr {id, node, span}: Expr, folder: &mut T) ->
                 ExprLoop(folder.fold_block(body),
                         opt_ident.map(|i| folder.fold_ident(i)))
             }
-            ExprMatch(expr, arms) => {
+            ExprMatch(expr, arms, source) => {
                 ExprMatch(folder.fold_expr(expr),
-                        arms.move_map(|x| folder.fold_arm(x)))
+                        arms.move_map(|x| folder.fold_arm(x)),
+                        source)
             }
             ExprFnBlock(capture_clause, decl, body) => {
                 ExprFnBlock(capture_clause,

src/libsyntax/parse/parser.rs

@@ -38,7 +38,7 @@ use ast::{ItemMac, ItemMod, ItemStruct, ItemTrait, ItemTy};
 use ast::{LifetimeDef, Lit, Lit_};
 use ast::{LitBool, LitChar, LitByte, LitBinary};
 use ast::{LitNil, LitStr, LitInt, Local, LocalLet};
-use ast::{MutImmutable, MutMutable, Mac_, MacInvocTT, Matcher, MatchNonterminal};
+use ast::{MutImmutable, MutMutable, Mac_, MacInvocTT, Matcher, MatchNonterminal, MatchNormal};
 use ast::{MatchSeq, MatchTok, Method, MutTy, BiMul, Mutability};
 use ast::{MethodImplItem, NamedField, UnNeg, NoReturn, UnNot};
 use ast::{Pat, PatEnum, PatIdent, PatLit, PatRange, PatRegion, PatStruct};
@@ -2973,7 +2973,7 @@ impl<'a> Parser<'a> {
         }
         let hi = self.span.hi;
         self.bump();
-        return self.mk_expr(lo, hi, ExprMatch(discriminant, arms));
+        return self.mk_expr(lo, hi, ExprMatch(discriminant, arms, MatchNormal));
     }
 
     pub fn parse_arm(&mut self) -> Arm {

src/libsyntax/print/pprust.rs

@@ -1535,7 +1535,7 @@ impl<'a> State<'a> {
                 try!(space(&mut self.s));
                 try!(self.print_block(&**blk));
             }
-            ast::ExprMatch(ref expr, ref arms) => {
+            ast::ExprMatch(ref expr, ref arms, _) => {
                 try!(self.cbox(indent_unit));
                 try!(self.ibox(4));
                 try!(self.word_nbsp("match"));

src/test/compile-fail/if-let.rs

@@ -0,0 +1,57 @@
+// 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.
+
+#![feature(macro_rules)]
+
+fn macros() {
+    macro_rules! foo{
+        ($p:pat, $e:expr, $b:block) => {{
+            if let $p = $e $b
+        }}
+    }
+    macro_rules! bar{
+        ($p:pat, $e:expr, $b:block) => {{
+            foo!($p, $e, $b)
+        }}
+    }
+
+    foo!(a, 1i, { //~ ERROR irrefutable if-let
+        println!("irrefutable pattern");
+    });
+    bar!(a, 1i, { //~ ERROR irrefutable if-let
+        println!("irrefutable pattern");
+    });
+}
+
+pub fn main() {
+    if let a = 1i { //~ ERROR irrefutable if-let
+        println!("irrefutable pattern");
+    }
+
+    if let a = 1i { //~ ERROR irrefutable if-let
+        println!("irrefutable pattern");
+    } else if true {
+        println!("else-if in irrefutable if-let");
+    } else {
+        println!("else in irrefutable if-let");
+    }
+
+    if let 1i = 2i {
+        println!("refutable pattern");
+    } else if let a = 1i { //~ ERROR irrefutable if-let
+        println!("irrefutable pattern");
+    }
+
+    if true {
+        println!("if");
+    } else if let a = 1i { //~ ERROR irrefutable if-let
+        println!("irrefutable pattern");
+    }
+}

src/test/compile-fail/lint-unnecessary-parens.rs

@@ -32,6 +32,7 @@ fn main() {
     match (true) { //~ ERROR unnecessary parentheses around `match` head expression
         _ => {}
     }
+    if let 1i = (1i) {} //~ ERROR unnecessary parentheses around `if let` head expression
     let v = X { y: false };
     // struct lits needs parens, so these shouldn't warn.
     if (v == X { y: true }) {}