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