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

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 @@ fn check(&self, cmt: mc::cmt, discr_scope: Option<ast::NodeId>) -> R {
                 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,15 +1167,7 @@ fn check_pat_non_uppercase_statics(cx: &Context, p: &ast::Pat) {
     }
 }
 
-fn check_unnecessary_parens(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"),
-        _ => return
-    };
-
+fn check_unnecessary_parens_core(cx: &Context, value: &ast::Expr, msg: &str) {
     match value.node {
         ast::ExprParen(_) => {
             cx.span_lint(UnnecessaryParens, value.span,
@@ -1185,6 +1177,33 @@ fn check_unnecessary_parens(cx: &Context, e: &ast::Expr) {
     }
 }
 
+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);
+}
+
+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) {
     match e.node {
         // Don't warn about generated blocks, that'll just pollute the output.
@@ -1534,7 +1553,7 @@ fn visit_expr(&mut self, e: &ast::Expr, _: ()) {
 
         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 @@ fn visit_expr(&mut self, e: &ast::Expr, _: ()) {
     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 @@ fn object_contents(cx: ctxt,
                        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 fn end_tag(&mut self) {
             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 fn fill_utf16_buf_and_decode(f: |*mut u16, DWORD| -> DWORD)
                 } 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 @@ fn env_convert(input: ~[~[u8]]) -> ~[(~[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 @@ pub fn subset_of(&self, other_abi_set: AbiSet) -> bool {
     }
 
     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 @@ fn check_asts_encodable() {
             },
         };
         // doesn't matter which encoder we use....
-        let _f = (&e as &serialize::Encodable<json::Encoder>);
+        let _f = &e as &serialize::Encodable<json::Encoder>;
     }
 }

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

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,'.') => {

src/libtest/lib.rs

@@ -1047,7 +1047,7 @@ pub fn compare_to_old(&self, old: &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

src/libuuid/lib.rs

@@ -296,7 +296,7 @@ pub fn get_version_num(&self) -> uint {
     ///
     /// This represents the algorithm used to generate the contents
     pub fn get_version(&self) -> Option<UuidVersion> {
-        let v = (self.bytes[6] >> 4);
+        let v = self.bytes[6] >> 4;
         match v {
             1 => Some(Version1Mac),
             2 => Some(Version2Dce),

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
 }