auto merge of #15212 : huonw/rust/struct-paren-lint, r=alexcrichton

rustc: update the unnecessary parens lint for struct literals.

Things like `match X { x: 1 } { ... }` now need to be written with
parentheses, so the lint should avoid warning in cases like that.

2 files changed, 74 insertions, 13 deletions

diff --git a/src/librustc/lint/builtin.rs b/src/librustc/lint/builtin.rs
index 5078ae80d75..339db67b3f1 100644
--- a/src/librustc/lint/builtin.rs
+++ b/src/librustc/lint/builtin.rs
@@ -975,14 +975,52 @@ declare_lint!(UNNECESSARY_PARENS, Warn,
 pub struct UnnecessaryParens;
 
 impl UnnecessaryParens {
-    fn check_unnecessary_parens_core(&self, cx: &Context, value: &ast::Expr, msg: &str) {
+    fn check_unnecessary_parens_core(&self, cx: &Context, value: &ast::Expr, msg: &str,
+                                     struct_lit_needs_parens: bool) {
         match value.node {
-            ast::ExprParen(_) => {
-                cx.span_lint(UNNECESSARY_PARENS, value.span,
-                    format!("unnecessary parentheses around {}", msg).as_slice())
+            ast::ExprParen(ref inner) => {
+                let necessary = struct_lit_needs_parens && contains_exterior_struct_lit(&**inner);
+                if !necessary {
+                    cx.span_lint(UNNECESSARY_PARENS, value.span,
+                                 format!("unnecessary parentheses around {}",
+                                         msg).as_slice())
+                }
             }
             _ => {}
         }
+
+        /// Expressions that syntatically contain an "exterior" struct
+        /// literal i.e. not surrounded by any parens or other
+        /// delimiters, e.g. `X { y: 1 }`, `X { y: 1 }.method()`, `foo
+        /// == X { y: 1 }` and `X { y: 1 } == foo` all do, but `(X {
+        /// y: 1 }) == foo` does not.
+        fn contains_exterior_struct_lit(value: &ast::Expr) -> bool {
+            match value.node {
+                ast::ExprStruct(..) => true,
+
+                ast::ExprAssign(ref lhs, ref rhs) |
+                ast::ExprAssignOp(_, ref lhs, ref rhs) |
+                ast::ExprBinary(_, ref lhs, ref rhs) => {
+                    // X { y: 1 } + X { y: 2 }
+                    contains_exterior_struct_lit(&**lhs) ||
+                        contains_exterior_struct_lit(&**rhs)
+                }
+                ast::ExprUnary(_, ref x) |
+                ast::ExprCast(ref x, _) |
+                ast::ExprField(ref x, _, _) |
+                ast::ExprIndex(ref x, _) => {
+                    // &X { y: 1 }, X { y: 1 }.y
+                    contains_exterior_struct_lit(&**x)
+                }
+
+                ast::ExprMethodCall(_, _, ref exprs) => {
+                    // X { y: 1 }.bar(...)
+                    contains_exterior_struct_lit(&**exprs.get(0))
+                }
+
+                _ => false
+            }
+        }
     }
 }
 
@@ -992,16 +1030,16 @@ impl LintPass for UnnecessaryParens {
     }
 
     fn check_expr(&mut self, cx: &Context, e: &ast::Expr) {
-        let (value, msg) = match e.node {
-            ast::ExprIf(cond, _, _) => (cond, "`if` condition"),
-            ast::ExprWhile(cond, _) => (cond, "`while` condition"),
-            ast::ExprMatch(head, _) => (head, "`match` head expression"),
-            ast::ExprRet(Some(value)) => (value, "`return` value"),
-            ast::ExprAssign(_, value) => (value, "assigned value"),
-            ast::ExprAssignOp(_, _, value) => (value, "assigned value"),
+        let (value, msg, struct_lit_needs_parens) = match e.node {
+            ast::ExprIf(cond, _, _) => (cond, "`if` condition", true),
+            ast::ExprWhile(cond, _) => (cond, "`while` condition", true),
+            ast::ExprMatch(head, _) => (head, "`match` head expression", true),
+            ast::ExprRet(Some(value)) => (value, "`return` value", false),
+            ast::ExprAssign(_, value) => (value, "assigned value", false),
+            ast::ExprAssignOp(_, _, value) => (value, "assigned value", false),
             _ => return
         };
-        self.check_unnecessary_parens_core(cx, &*value, msg);
+        self.check_unnecessary_parens_core(cx, &*value, msg, struct_lit_needs_parens);
     }
 
     fn check_stmt(&mut self, cx: &Context, s: &ast::Stmt) {
@@ -1015,7 +1053,7 @@ impl LintPass for UnnecessaryParens {
             },
             _ => return
         };
-        self.check_unnecessary_parens_core(cx, &*value, msg);
+        self.check_unnecessary_parens_core(cx, &*value, msg, false);
     }
 }
 
diff --git a/src/test/compile-fail/lint-unnecessary-parens.rs b/src/test/compile-fail/lint-unnecessary-parens.rs
index b2abe025794..4d9383aeda2 100644
--- a/src/test/compile-fail/lint-unnecessary-parens.rs
+++ b/src/test/compile-fail/lint-unnecessary-parens.rs
@@ -10,18 +10,41 @@
 
 #![deny(unnecessary_parens)]
 
+#[deriving(Eq, PartialEq)]
+struct X { y: bool }
+impl X {
+    fn foo(&self) -> bool { self.y }
+}
+
 fn foo() -> int {
     return (1); //~ ERROR unnecessary parentheses around `return` value
 }
+fn bar() -> X {
+    return (X { y: true }); //~ ERROR unnecessary parentheses around `return` value
+}
 
 fn main() {
     foo();
+    bar();
 
     if (true) {} //~ ERROR unnecessary parentheses around `if` condition
     while (true) {} //~ ERROR unnecessary parentheses around `while` condition
     match (true) { //~ ERROR unnecessary parentheses around `match` head expression
         _ => {}
     }
+    let v = X { y: false };
+    // struct lits needs parens, so these shouldn't warn.
+    if (v == X { y: true }) {}
+    if (X { y: true } == v) {}
+    if (X { y: false }.y) {}
+
+    while (X { y: false }.foo()) {}
+    while (true | X { y: false }.y) {}
+
+    match (X { y: false }) {
+        _ => {}
+    }
+
     let mut _a = (0); //~ ERROR unnecessary parentheses around assigned value
     _a = (0); //~ ERROR unnecessary parentheses around assigned value
     _a += (1); //~ ERROR unnecessary parentheses around assigned value