Skip to content

Commit 64019e7

Browse files
committed
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.
1 parent 8fe47bc commit 64019e7

File tree

2 files changed

+74
-13
lines changed

2 files changed

+74
-13
lines changed

src/librustc/lint/builtin.rs

+51-13
Original file line numberDiff line numberDiff line change
@@ -975,14 +975,52 @@ declare_lint!(UNNECESSARY_PARENS, Warn,
975975
pub struct UnnecessaryParens;
976976

977977
impl UnnecessaryParens {
978-
fn check_unnecessary_parens_core(&self, cx: &Context, value: &ast::Expr, msg: &str) {
978+
fn check_unnecessary_parens_core(&self, cx: &Context, value: &ast::Expr, msg: &str,
979+
struct_lit_needs_parens: bool) {
979980
match value.node {
980-
ast::ExprParen(_) => {
981-
cx.span_lint(UNNECESSARY_PARENS, value.span,
982-
format!("unnecessary parentheses around {}", msg).as_slice())
981+
ast::ExprParen(ref inner) => {
982+
let necessary = struct_lit_needs_parens && contains_exterior_struct_lit(&**inner);
983+
if !necessary {
984+
cx.span_lint(UNNECESSARY_PARENS, value.span,
985+
format!("unnecessary parentheses around {}",
986+
msg).as_slice())
987+
}
983988
}
984989
_ => {}
985990
}
991+
992+
/// Expressions that syntatically contain an "exterior" struct
993+
/// literal i.e. not surrounded by any parens or other
994+
/// delimiters, e.g. `X { y: 1 }`, `X { y: 1 }.method()`, `foo
995+
/// == X { y: 1 }` and `X { y: 1 } == foo` all do, but `(X {
996+
/// y: 1 }) == foo` does not.
997+
fn contains_exterior_struct_lit(value: &ast::Expr) -> bool {
998+
match value.node {
999+
ast::ExprStruct(..) => true,
1000+
1001+
ast::ExprAssign(ref lhs, ref rhs) |
1002+
ast::ExprAssignOp(_, ref lhs, ref rhs) |
1003+
ast::ExprBinary(_, ref lhs, ref rhs) => {
1004+
// X { y: 1 } + X { y: 2 }
1005+
contains_exterior_struct_lit(&**lhs) ||
1006+
contains_exterior_struct_lit(&**rhs)
1007+
}
1008+
ast::ExprUnary(_, ref x) |
1009+
ast::ExprCast(ref x, _) |
1010+
ast::ExprField(ref x, _, _) |
1011+
ast::ExprIndex(ref x, _) => {
1012+
// &X { y: 1 }, X { y: 1 }.y
1013+
contains_exterior_struct_lit(&**x)
1014+
}
1015+
1016+
ast::ExprMethodCall(_, _, ref exprs) => {
1017+
// X { y: 1 }.bar(...)
1018+
contains_exterior_struct_lit(&**exprs.get(0))
1019+
}
1020+
1021+
_ => false
1022+
}
1023+
}
9861024
}
9871025
}
9881026

@@ -992,16 +1030,16 @@ impl LintPass for UnnecessaryParens {
9921030
}
9931031

9941032
fn check_expr(&mut self, cx: &Context, e: &ast::Expr) {
995-
let (value, msg) = match e.node {
996-
ast::ExprIf(cond, _, _) => (cond, "`if` condition"),
997-
ast::ExprWhile(cond, _) => (cond, "`while` condition"),
998-
ast::ExprMatch(head, _) => (head, "`match` head expression"),
999-
ast::ExprRet(Some(value)) => (value, "`return` value"),
1000-
ast::ExprAssign(_, value) => (value, "assigned value"),
1001-
ast::ExprAssignOp(_, _, value) => (value, "assigned value"),
1033+
let (value, msg, struct_lit_needs_parens) = match e.node {
1034+
ast::ExprIf(cond, _, _) => (cond, "`if` condition", true),
1035+
ast::ExprWhile(cond, _) => (cond, "`while` condition", true),
1036+
ast::ExprMatch(head, _) => (head, "`match` head expression", true),
1037+
ast::ExprRet(Some(value)) => (value, "`return` value", false),
1038+
ast::ExprAssign(_, value) => (value, "assigned value", false),
1039+
ast::ExprAssignOp(_, _, value) => (value, "assigned value", false),
10021040
_ => return
10031041
};
1004-
self.check_unnecessary_parens_core(cx, &*value, msg);
1042+
self.check_unnecessary_parens_core(cx, &*value, msg, struct_lit_needs_parens);
10051043
}
10061044

10071045
fn check_stmt(&mut self, cx: &Context, s: &ast::Stmt) {
@@ -1015,7 +1053,7 @@ impl LintPass for UnnecessaryParens {
10151053
},
10161054
_ => return
10171055
};
1018-
self.check_unnecessary_parens_core(cx, &*value, msg);
1056+
self.check_unnecessary_parens_core(cx, &*value, msg, false);
10191057
}
10201058
}
10211059

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

+23
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,41 @@
1010

1111
#![deny(unnecessary_parens)]
1212

13+
#[deriving(Eq, PartialEq)]
14+
struct X { y: bool }
15+
impl X {
16+
fn foo(&self) -> bool { self.y }
17+
}
18+
1319
fn foo() -> int {
1420
return (1); //~ ERROR unnecessary parentheses around `return` value
1521
}
22+
fn bar() -> X {
23+
return (X { y: true }); //~ ERROR unnecessary parentheses around `return` value
24+
}
1625

1726
fn main() {
1827
foo();
28+
bar();
1929

2030
if (true) {} //~ ERROR unnecessary parentheses around `if` condition
2131
while (true) {} //~ ERROR unnecessary parentheses around `while` condition
2232
match (true) { //~ ERROR unnecessary parentheses around `match` head expression
2333
_ => {}
2434
}
35+
let v = X { y: false };
36+
// struct lits needs parens, so these shouldn't warn.
37+
if (v == X { y: true }) {}
38+
if (X { y: true } == v) {}
39+
if (X { y: false }.y) {}
40+
41+
while (X { y: false }.foo()) {}
42+
while (true | X { y: false }.y) {}
43+
44+
match (X { y: false }) {
45+
_ => {}
46+
}
47+
2548
let mut _a = (0); //~ ERROR unnecessary parentheses around assigned value
2649
_a = (0); //~ ERROR unnecessary parentheses around assigned value
2750
_a += (1); //~ ERROR unnecessary parentheses around assigned value

0 commit comments

Comments
 (0)