From 9982de6397197a63a093e7b79851d1915ef783d7 Mon Sep 17 00:00:00 2001 From: Eduard Bopp Date: Tue, 18 Feb 2014 13:40:25 +0100 Subject: Warn about unnecessary parentheses upon assignment Closes #12366. Parentheses around assignment statements such as let mut a = (0); a = (1); a += (2); are not necessary and therefore an unnecessary_parens warning is raised when statements like this occur. The warning mechanism was refactored along the way to allow for code reuse between the routines for checking expressions and statements. Code had to be adopted throughout the compiler and standard libraries to comply with this modification of the lint. --- src/librustc/metadata/loader.rs | 7 ++-- .../middle/borrowck/gather_loans/lifetime.rs | 5 ++- src/librustc/middle/dataflow.rs | 2 +- src/librustc/middle/lint.rs | 38 +++++++++++++++++----- src/librustc/middle/trans/monomorphize.rs | 6 ++-- src/librustc/middle/ty.rs | 4 +-- src/libserialize/ebml.rs | 2 +- src/libstd/os.rs | 4 +-- src/libsyntax/abi.rs | 2 +- src/libsyntax/ast.rs | 2 +- src/libsyntax/diagnostic.rs | 2 +- src/libterm/terminfo/parm.rs | 4 +-- src/libtest/lib.rs | 2 +- src/libuuid/lib.rs | 2 +- src/test/compile-fail/lint-unnecessary-parens.rs | 3 ++ 15 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/librustc/metadata/loader.rs b/src/librustc/metadata/loader.rs index 3f35b5cf2a1..3558166128b 100644 --- a/src/librustc/metadata/loader.rs +++ b/src/librustc/metadata/loader.rs @@ -407,11 +407,8 @@ fn get_metadata_section_imp(os: Os, filename: &Path) -> Option { debug!("checking {} bytes of metadata-version stamp", vlen); let minsz = cmp::min(vlen, csz); - let mut version_ok = false; - vec::raw::buf_as_slice(cvbuf, minsz, |buf0| { - version_ok = (buf0 == - encoder::metadata_encoding_version); - }); + let version_ok = vec::raw::buf_as_slice(cvbuf, minsz, + |buf0| buf0 == encoder::metadata_encoding_version); if !version_ok { return None; } let cvbuf1 = cvbuf.offset(vlen as int); diff --git a/src/librustc/middle/borrowck/gather_loans/lifetime.rs b/src/librustc/middle/borrowck/gather_loans/lifetime.rs index c47affac683..9714da3abf5 100644 --- a/src/librustc/middle/borrowck/gather_loans/lifetime.rs +++ b/src/librustc/middle/borrowck/gather_loans/lifetime.rs @@ -95,11 +95,10 @@ impl<'a> GuaranteeLifetimeContext<'a> { let base_scope = self.scope(base); // L-Deref-Managed-Imm-User-Root - let omit_root = ( + let omit_root = self.bccx.is_subregion_of(self.loan_region, base_scope) && self.is_rvalue_or_immutable(base) && - !self.is_moved(base) - ); + !self.is_moved(base); if !omit_root { // L-Deref-Managed-Imm-Compiler-Root diff --git a/src/librustc/middle/dataflow.rs b/src/librustc/middle/dataflow.rs index e8e05b0979a..8a504f07b73 100644 --- a/src/librustc/middle/dataflow.rs +++ b/src/librustc/middle/dataflow.rs @@ -891,7 +891,7 @@ fn bitwise(out_vec: &mut [uint], in_vec: &[uint], op: |uint, uint| -> uint) let old_val = *out_elt; let new_val = op(old_val, *in_elt); *out_elt = new_val; - changed |= (old_val != new_val); + changed |= old_val != new_val; } changed } diff --git a/src/librustc/middle/lint.rs b/src/librustc/middle/lint.rs index 28436093a35..911b6df10a6 100644 --- a/src/librustc/middle/lint.rs +++ b/src/librustc/middle/lint.rs @@ -1167,22 +1167,41 @@ fn check_pat_non_uppercase_statics(cx: &Context, p: &ast::Pat) { } } -fn check_unnecessary_parens(cx: &Context, e: &ast::Expr) { +fn check_unnecessary_parens_core(cx: &Context, value: &ast::Expr, msg: &str) { + match value.node { + ast::ExprParen(_) => { + cx.span_lint(UnnecessaryParens, value.span, + format!("unnecessary parentheses around {}", msg)) + } + _ => {} + } +} + +fn check_unnecessary_parens_expr(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"), _ => return }; + check_unnecessary_parens_core(cx, value, msg); +} - match value.node { - ast::ExprParen(_) => { - cx.span_lint(UnnecessaryParens, value.span, - format!("unnecessary parentheses around {}", msg)) - } - _ => {} - } +fn check_unnecessary_parens_stmt(cx: &Context, s: &ast::Stmt) { + let (value, msg) = match s.node { + ast::StmtDecl(decl, _) => match decl.node { + ast::DeclLocal(local) => match local.init { + Some(value) => (value, "assigned value"), + None => return + }, + _ => return + }, + _ => return + }; + check_unnecessary_parens_core(cx, value, msg); } fn check_unused_unsafe(cx: &Context, e: &ast::Expr) { @@ -1534,7 +1553,7 @@ impl<'a> Visitor<()> for Context<'a> { check_while_true_expr(self, e); check_stability(self, e); - check_unnecessary_parens(self, e); + check_unnecessary_parens_expr(self, e); check_unused_unsafe(self, e); check_unsafe_block(self, e); check_unnecessary_allocation(self, e); @@ -1549,6 +1568,7 @@ impl<'a> Visitor<()> for Context<'a> { fn visit_stmt(&mut self, s: &ast::Stmt, _: ()) { check_path_statement(self, s); check_unused_result(self, s); + check_unnecessary_parens_stmt(self, s); visit::walk_stmt(self, s, ()); } diff --git a/src/librustc/middle/trans/monomorphize.rs b/src/librustc/middle/trans/monomorphize.rs index 7a9d93d89f2..488287e1c68 100644 --- a/src/librustc/middle/trans/monomorphize.rs +++ b/src/librustc/middle/trans/monomorphize.rs @@ -143,10 +143,8 @@ pub fn monomorphic_fn(ccx: @CrateContext, // This is a bit unfortunate. let idx = psubsts.tys.len() - num_method_ty_params; - let substs = - (psubsts.tys.slice(0, idx) + - &[psubsts.self_ty.unwrap()] + - psubsts.tys.tailn(idx)); + let substs = psubsts.tys.slice(0, idx) + + &[psubsts.self_ty.unwrap()] + psubsts.tys.tailn(idx); debug!("static default: changed substitution to {}", substs.repr(ccx.tcx)); diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 09c21d54c87..aa53ea59901 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -2293,8 +2293,8 @@ pub fn type_contents(cx: ctxt, ty: t) -> TypeContents { bounds: BuiltinBounds) -> TypeContents { // These are the type contents of the (opaque) interior - let contents = (TC::ReachesMutable.when(mutbl == ast::MutMutable) | - kind_bounds_to_contents(cx, bounds, [])); + let contents = TC::ReachesMutable.when(mutbl == ast::MutMutable) | + kind_bounds_to_contents(cx, bounds, []); match store { UniqTraitStore => { diff --git a/src/libserialize/ebml.rs b/src/libserialize/ebml.rs index e65c21a6b5f..b7e1e5a8da7 100644 --- a/src/libserialize/ebml.rs +++ b/src/libserialize/ebml.rs @@ -674,7 +674,7 @@ pub mod writer { let last_size_pos = self.size_positions.pop().unwrap(); let cur_pos = try!(self.writer.tell()); try!(self.writer.seek(last_size_pos as i64, io::SeekSet)); - let size = (cur_pos as uint - last_size_pos - 4); + let size = cur_pos as uint - last_size_pos - 4; write_sized_vuint(self.writer, size, 4u); try!(self.writer.seek(cur_pos as i64, io::SeekSet)); diff --git a/src/libstd/os.rs b/src/libstd/os.rs index 458e31fd86f..fdd81179325 100644 --- a/src/libstd/os.rs +++ b/src/libstd/os.rs @@ -119,7 +119,7 @@ pub mod win32 { } else if k == n && libc::GetLastError() == libc::ERROR_INSUFFICIENT_BUFFER as DWORD { - n *= (2 as DWORD); + n *= 2 as DWORD; } else if k >= n { n = k; } else { @@ -225,7 +225,7 @@ pub fn env_as_bytes() -> ~[(~[u8],~[u8])] { for p in input.iter() { let vs: ~[&[u8]] = p.splitn(1, |b| *b == '=' as u8).collect(); let key = vs[0].to_owned(); - let val = (if vs.len() < 2 { ~[] } else { vs[1].to_owned() }); + let val = if vs.len() < 2 { ~[] } else { vs[1].to_owned() }; pairs.push((key, val)); } pairs diff --git a/src/libsyntax/abi.rs b/src/libsyntax/abi.rs index 6b0f2c6e516..82e4d08d077 100644 --- a/src/libsyntax/abi.rs +++ b/src/libsyntax/abi.rs @@ -202,7 +202,7 @@ impl AbiSet { } pub fn add(&mut self, abi: Abi) { - self.bits |= (1 << abi.index()); + self.bits |= 1 << abi.index(); } pub fn each(&self, op: |abi: Abi| -> bool) -> bool { diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index e8edc1a0dfc..c436dc018e7 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -1224,6 +1224,6 @@ mod test { }, }; // doesn't matter which encoder we use.... - let _f = (&e as &serialize::Encodable); + let _f = &e as &serialize::Encodable; } } diff --git a/src/libsyntax/diagnostic.rs b/src/libsyntax/diagnostic.rs index 9455df063f1..8cf0f128d22 100644 --- a/src/libsyntax/diagnostic.rs +++ b/src/libsyntax/diagnostic.rs @@ -329,7 +329,7 @@ fn highlight_lines(cm: &codemap::CodeMap, for _ in range(0, skip) { s.push_char(' '); } let orig = fm.get_line(lines.lines[0] as int); for pos in range(0u, left-skip) { - let curChar = (orig[pos] as char); + let curChar = orig[pos] as char; // Whenever a tab occurs on the previous line, we insert one on // the error-point-squiggly-line as well (instead of a space). // That way the squiggly line will usually appear in the correct diff --git a/src/libterm/terminfo/parm.rs b/src/libterm/terminfo/parm.rs index 948e79b4481..dfb6d6b1a88 100644 --- a/src/libterm/terminfo/parm.rs +++ b/src/libterm/terminfo/parm.rs @@ -259,7 +259,7 @@ pub fn expand(cap: &[u8], params: &[Param], vars: &mut Variables) ' ' => flags.space = true, '.' => fstate = FormatStatePrecision, '0'..'9' => { - flags.width = (cur as uint - '0' as uint); + flags.width = cur as uint - '0' as uint; fstate = FormatStateWidth; } _ => unreachable!() @@ -359,7 +359,7 @@ pub fn expand(cap: &[u8], params: &[Param], vars: &mut Variables) flags.space = true; } (FormatStateFlags,'0'..'9') => { - flags.width = (cur as uint - '0' as uint); + flags.width = cur as uint - '0' as uint; *fstate = FormatStateWidth; } (FormatStateFlags,'.') => { diff --git a/src/libtest/lib.rs b/src/libtest/lib.rs index 19c2f80cdbd..eba922ac7b8 100644 --- a/src/libtest/lib.rs +++ b/src/libtest/lib.rs @@ -1047,7 +1047,7 @@ impl MetricMap { let r = match selfmap.find(k) { None => MetricRemoved, Some(v) => { - let delta = (v.value - vold.value); + let delta = v.value - vold.value; let noise = match noise_pct { None => f64::max(vold.noise.abs(), v.noise.abs()), Some(pct) => vold.value * pct / 100.0 diff --git a/src/libuuid/lib.rs b/src/libuuid/lib.rs index 5941afb7d75..4bbe38bd213 100644 --- a/src/libuuid/lib.rs +++ b/src/libuuid/lib.rs @@ -296,7 +296,7 @@ impl Uuid { /// /// This represents the algorithm used to generate the contents pub fn get_version(&self) -> Option { - let v = (self.bytes[6] >> 4); + let v = self.bytes[6] >> 4; match v { 1 => Some(Version1Mac), 2 => Some(Version2Dce), diff --git a/src/test/compile-fail/lint-unnecessary-parens.rs b/src/test/compile-fail/lint-unnecessary-parens.rs index aedcbde7f9d..528fc2f64b4 100644 --- a/src/test/compile-fail/lint-unnecessary-parens.rs +++ b/src/test/compile-fail/lint-unnecessary-parens.rs @@ -22,4 +22,7 @@ fn main() { match (true) { //~ ERROR unnecessary parentheses around `match` head expression _ => {} } + 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 } -- cgit 1.4.1-3-g733a5