Skip to content

Commit

Permalink
rustc: update the unnecessary parens lint for struct literals.
Browse files Browse the repository at this point in the history
Things like `match X { x: 1 } { ... }` now need to be written with
parentheses, so the lint should avoid warning in cases like that.
  • Loading branch information
huonw committed Jun 27, 2014
1 parent 8fe47bc commit 64019e7
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 13 deletions.
64 changes: 51 additions & 13 deletions src/librustc/lint/builtin.rs
Expand Up @@ -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
}
}
}
}

Expand All @@ -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) {
Expand All @@ -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);
}
}

Expand Down
23 changes: 23 additions & 0 deletions src/test/compile-fail/lint-unnecessary-parens.rs
Expand Up @@ -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
Expand Down

5 comments on commit 64019e7

@bors
Copy link
Contributor

@bors bors commented on 64019e7 Jun 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from alexcrichton
at huonw@64019e7

@bors
Copy link
Contributor

@bors bors commented on 64019e7 Jun 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging huonw/rust/struct-paren-lint = 64019e7 into auto

@bors
Copy link
Contributor

@bors bors commented on 64019e7 Jun 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

huonw/rust/struct-paren-lint = 64019e7 merged ok, testing candidate = abdf71c

@bors
Copy link
Contributor

@bors bors commented on 64019e7 Jun 27, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = abdf71c

Please sign in to comment.