auto merge of #12366 : aepsil0n/rust/feature/unnecessary_parens_around_assigned_values, r=alexcrichton

Fixes #12350.

Parentheses around assignment statements such as

```rust
let mut a = (0);
a = (1);
a += (2);
```

are not necessary and therefore an unnecessary_parens warning is raised when
statements like this occur.

NOTE: In `let` declarations this does not work as intended. Is it possible that they do not count as assignment expressions (`ExprAssign`)? (edit: this is fixed by now)

Furthermore, there are some cases that I fixed in the rest of the code, where parentheses could potentially enhance readability. Compare these lines:

```rust
a = b == c;
a = (b == c);
```

Thus, after having worked on this I'm not entirely sure, whether we should go through with this patch or not. Probably a matter of debate. ;)

Author: bors

src/librustc/metadata/loader.rs

@@ -407,11 +407,8 @@ fn get_metadata_section_imp(os: Os, filename: &Path) -> Option<MetadataBlob> {
                 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);

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

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
 }

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, ());
     }

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));
 

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 => {

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));
 

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

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 {

src/libsyntax/ast.rs

@@ -1224,6 +1224,6 @@ mod test {
             },
         };
         // doesn't matter which encoder we use....
-        let _f = (&e as &serialize::Encodable<json::Encoder>);
+        let _f = &e as &serialize::Encodable<json::Encoder>;
     }
 }