From 9982de6397197a63a093e7b79851d1915ef783d7 Mon Sep 17 00:00:00 2001 From: Eduard Bopp Date: Tue, 18 Feb 2014 13:40:25 +0100 Subject: [PATCH] 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 +- .../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 3f35b5cf2a1ff..3558166128b28 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 c47affac683f4..9714da3abf58d 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 e8e05b0979a04..8a504f07b7393 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 28436093a3573..911b6df10a67d 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 7a9d93d89f2b3..488287e1c683e 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 09c21d54c87a0..aa53ea59901eb 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 e65c21a6b5f86..b7e1e5a8da7d8 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 458e31fd86f5f..fdd81179325eb 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 6b0f2c6e516c6..82e4d08d07779 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 e8edc1a0dfc0e..c436dc018e7f8 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 9455df063f117..8cf0f128d222d 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 948e79b448105..dfb6d6b1a8825 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 19c2f80cdbd56..eba922ac7b8b9 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 5941afb7d7573..4bbe38bd213dc 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 aedcbde7f9dcd..528fc2f64b49b 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 }