Rollup merge of #64128 - Centril:unused-parens-pat, r=davidtwco

unused_parens: account for or-patterns and `&(mut x)`

Fixes https://github.com/rust-lang/rust/issues/55342.
Fixes https://github.com/rust-lang/rust/issues/64106.

cc https://github.com/rust-lang/rust/issues/54883
cc https://github.com/rust-lang/rust/pull/64111

r? @oli-obk

src/librustc_lint/unused.rs

@@ -398,18 +398,37 @@ impl UnusedParens {
         }
     }
 
-    fn check_unused_parens_pat(&self,
+    fn check_unused_parens_pat(
-                                cx: &EarlyContext<'_>,
+        &self,
-                                value: &ast::Pat,
+        cx: &EarlyContext<'_>,
-                                msg: &str) {
+        value: &ast::Pat,
-        if let ast::PatKind::Paren(_) = value.node {
+        avoid_or: bool,
+        avoid_mut: bool,
+    ) {
+        use ast::{PatKind, BindingMode::ByValue, Mutability::Mutable};
+
+        if let PatKind::Paren(inner) = &value.node {
+            match inner.node {
+                // The lint visitor will visit each subpattern of `p`. We do not want to lint
+                // any range pattern no matter where it occurs in the pattern. For something like
+                // `&(a..=b)`, there is a recursive `check_pat` on `a` and `b`, but we will assume
+                // that if there are unnecessary parens they serve a purpose of readability.
+                PatKind::Range(..) => return,
+                // Avoid `p0 | .. | pn` if we should.
+                PatKind::Or(..) if avoid_or => return,
+                // Avoid `mut x` and `mut x @ p` if we should:
+                PatKind::Ident(ByValue(Mutable), ..) if avoid_mut => return,
+                // Otherwise proceed with linting.
+                _ => {}
+            }
+
             let pattern_text = if let Ok(snippet) = cx.sess().source_map()
                 .span_to_snippet(value.span) {
                     snippet
                 } else {
                     pprust::pat_to_string(value)
                 };
-            Self::remove_outer_parens(cx, value.span, &pattern_text, msg, (false, false));
+            Self::remove_outer_parens(cx, value.span, &pattern_text, "pattern", (false, false));
         }
     }
 
@@ -474,6 +493,13 @@ impl EarlyLintPass for UnusedParens {
     fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
         use syntax::ast::ExprKind::*;
         let (value, msg, followed_by_block, left_pos, right_pos) = match e.node {
+            Let(ref pats, ..) => {
+                for p in pats {
+                    self.check_unused_parens_pat(cx, p, false, false);
+                }
+                return;
+            }
+
             If(ref cond, ref block, ..) => {
                 let left = e.span.lo() + syntax_pos::BytePos(2);
                 let right = block.span.lo();
@@ -486,7 +512,8 @@ impl EarlyLintPass for UnusedParens {
                 (cond, "`while` condition", true, Some(left), Some(right))
             },
 
-            ForLoop(_, ref cond, ref block, ..) => {
+            ForLoop(ref pat, ref cond, ref block, ..) => {
+                self.check_unused_parens_pat(cx, pat, false, false);
                 (cond, "`for` head expression", true, None, Some(block.span.lo()))
             }
 
@@ -531,26 +558,46 @@ impl EarlyLintPass for UnusedParens {
     }
 
     fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) {
-        use ast::PatKind::{Paren, Range};
+        use ast::{PatKind::*, Mutability};
-        // The lint visitor will visit each subpattern of `p`. We do not want to lint any range
+        match &p.node {
-        // pattern no matter where it occurs in the pattern. For something like `&(a..=b)`, there
+            // Do not lint on `(..)` as that will result in the other arms being useless.
-        // is a recursive `check_pat` on `a` and `b`, but we will assume that if there are
+            Paren(_)
-        // unnecessary parens they serve a purpose of readability.
+            // The other cases do not contain sub-patterns.
-        if let Paren(ref pat) = p.node {
+            | Wild | Rest | Lit(..) | Mac(..) | Range(..) | Ident(.., None) | Path(..) => return,
-            match pat.node {
+            // These are list-like patterns; parens can always be removed.
-                Range(..) => {}
+            TupleStruct(_, ps) | Tuple(ps) | Slice(ps) | Or(ps) => for p in ps {
-                _ => self.check_unused_parens_pat(cx, &p, "pattern")
+                self.check_unused_parens_pat(cx, p, false, false);
-            }
+            },
+            Struct(_, fps, _) => for f in fps {
+                self.check_unused_parens_pat(cx, &f.pat, false, false);
+            },
+            // Avoid linting on `i @ (p0 | .. | pn)` and `box (p0 | .. | pn)`, #64106.
+            Ident(.., Some(p)) | Box(p) => self.check_unused_parens_pat(cx, p, true, false),
+            // Avoid linting on `&(mut x)` as `&mut x` has a different meaning, #55342.
+            // Also avoid linting on `& mut? (p0 | .. | pn)`, #64106.
+            Ref(p, m) => self.check_unused_parens_pat(cx, p, true, *m == Mutability::Immutable),
         }
     }
 
     fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) {
         if let ast::StmtKind::Local(ref local) = s.node {
+            self.check_unused_parens_pat(cx, &local.pat, false, false);
+
             if let Some(ref value) = local.init {
                 self.check_unused_parens_expr(cx, &value, "assigned value", false, None, None);
             }
         }
     }
+
+    fn check_param(&mut self, cx: &EarlyContext<'_>, param: &ast::Param) {
+        self.check_unused_parens_pat(cx, &param.pat, true, false);
+    }
+
+    fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
+        for p in &arm.pats {
+            self.check_unused_parens_pat(cx, p, false, false);
+        }
+    }
 }
 
 declare_lint! {

src/test/ui/lint/issue-54538-unused-parens-lint.rs

@@ -1,25 +1,75 @@
-// build-pass (FIXME(62277): could be check-pass?)
+#![feature(box_patterns)]
+
+#![feature(or_patterns)]
+//~^ WARN the feature `or_patterns` is incomplete
 
 #![allow(ellipsis_inclusive_range_patterns)]
 #![allow(unreachable_patterns)]
 #![allow(unused_variables)]
-#![warn(unused_parens)]
+#![deny(unused_parens)]
+
+fn lint_on_top_level() {
+    let (a) = 0; //~ ERROR unnecessary parentheses around pattern
+    for (a) in 0..1 {} //~ ERROR unnecessary parentheses around pattern
+    if let (a) = 0 {} //~ ERROR unnecessary parentheses around pattern
+    while let (a) = 0 {} //~ ERROR unnecessary parentheses around pattern
+    fn foo((a): u8) {} //~ ERROR unnecessary parentheses around pattern
+    let _ = |(a): u8| 0; //~ ERROR unnecessary parentheses around pattern
+}
+
+// Don't lint in these cases (#64106).
+fn or_patterns_no_lint() {
+    match Box::new(0) {
+        box (0 | 1) => {} // Should not lint as `box 0 | 1` binds as `(box 0) | 1`.
+        _ => {}
+    }
+
+    match 0 {
+        x @ (0 | 1) => {} // Should not lint as `x @ 0 | 1` binds as `(x @ 0) | 1`.
+        _ => {}
+    }
+
+    if let &(0 | 1) = &0 {} // Should also not lint.
+    if let &mut (0 | 1) = &mut 0 {} // Same.
+
+    fn foo((Ok(a) | Err(a)): Result<u8, u8>) {} // Doesn't parse if we remove parens for now.
+    //~^ ERROR identifier `a` is bound more than once
+
+    let _ = |(Ok(a) | Err(a)): Result<u8, u8>| 1; // `|Ok(a) | Err(a)| 1` parses as bit-or.
+    //~^ ERROR identifier `a` is bound more than once
+}
+
+fn or_patterns_will_lint() {
+    if let (0 | 1) = 0 {} //~ ERROR unnecessary parentheses around pattern
+    if let ((0 | 1),) = (0,) {} //~ ERROR unnecessary parentheses around pattern
+    if let [(0 | 1)] = [0] {} //~ ERROR unnecessary parentheses around pattern
+    if let 0 | (1 | 2) = 0 {} //~ ERROR unnecessary parentheses around pattern
+    struct TS(u8);
+    if let TS((0 | 1)) = TS(0) {} //~ ERROR unnecessary parentheses around pattern
+    struct NS { f: u8 }
+    if let NS { f: (0 | 1) } = (NS { f: 0 }) {} //~ ERROR unnecessary parentheses around pattern
+}
+
+// Don't lint on `&(mut x)` because `&mut x` means something else (#55342).
+fn deref_mut_binding_no_lint() {
+    let &(mut x) = &0;
+}
 
 fn main() {
     match 1 {
-        (_) => {}         //~ WARNING: unnecessary parentheses around pattern
+        (_) => {} //~ ERROR unnecessary parentheses around pattern
-        (y) => {}         //~ WARNING: unnecessary parentheses around pattern
+        (y) => {} //~ ERROR unnecessary parentheses around pattern
-        (ref r) => {}     //~ WARNING: unnecessary parentheses around pattern
+        (ref r) => {} //~ ERROR unnecessary parentheses around pattern
-        (e @ 1...2) => {} //~ WARNING: unnecessary parentheses around outer pattern
+        (e @ 1...2) => {} //~ ERROR unnecessary parentheses around pattern
-        (1...2) => {}     // Non ambiguous range pattern should not warn
+        (1...2) => {} // Non ambiguous range pattern should not warn
         e @ (3...4) => {} // Non ambiguous range pattern should not warn
     }
 
     match &1 {
-        (e @ &(1...2)) => {} //~ WARNING: unnecessary parentheses around outer pattern
+        (e @ &(1...2)) => {} //~ ERROR unnecessary parentheses around pattern
-        &(_) => {}           //~ WARNING: unnecessary parentheses around pattern
+        &(_) => {} //~ ERROR unnecessary parentheses around pattern
-        e @ &(1...2) => {}   // Ambiguous range pattern should not warn
+        e @ &(1...2) => {} // Ambiguous range pattern should not warn
-        &(1...2) => {}       // Ambiguous range pattern should not warn
+        &(1...2) => {} // Ambiguous range pattern should not warn
     }
 
     match &1 {
@@ -28,19 +78,19 @@ fn main() {
     }
 
     match 1 {
-        (_) => {}         //~ WARNING: unnecessary parentheses around pattern
+        (_) => {} //~ ERROR unnecessary parentheses around pattern
-        (y) => {}         //~ WARNING: unnecessary parentheses around pattern
+        (y) => {} //~ ERROR unnecessary parentheses around pattern
-        (ref r) => {}     //~ WARNING: unnecessary parentheses around pattern
+        (ref r) => {} //~ ERROR unnecessary parentheses around pattern
-        (e @ 1..=2) => {} //~ WARNING: unnecessary parentheses around outer pattern
+        (e @ 1..=2) => {} //~ ERROR unnecessary parentheses around pattern
-        (1..=2) => {}     // Non ambiguous range pattern should not warn
+        (1..=2) => {} // Non ambiguous range pattern should not warn
         e @ (3..=4) => {} // Non ambiguous range pattern should not warn
     }
 
     match &1 {
-        (e @ &(1..=2)) => {} //~ WARNING: unnecessary parentheses around outer pattern
+        (e @ &(1..=2)) => {} //~ ERROR unnecessary parentheses around pattern
-        &(_) => {}           //~ WARNING: unnecessary parentheses around pattern
+        &(_) => {} //~ ERROR unnecessary parentheses around pattern
-        e @ &(1..=2) => {}   // Ambiguous range pattern should not warn
+        e @ &(1..=2) => {} // Ambiguous range pattern should not warn
-        &(1..=2) => {}       // Ambiguous range pattern should not warn
+        &(1..=2) => {} // Ambiguous range pattern should not warn
     }
 
     match &1 {

src/test/ui/lint/issue-54538-unused-parens-lint.stderr

@@ -1,78 +1,173 @@
-warning: unnecessary parentheses around pattern
+error[E0416]: identifier `a` is bound more than once in the same pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:10:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:35:25
    |
-LL |         (_) => {}
+LL |     fn foo((Ok(a) | Err(a)): Result<u8, u8>) {} // Doesn't parse if we remove parens for now.
+   |                         ^ used in a pattern more than once
+
+error[E0416]: identifier `a` is bound more than once in the same pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:38:27
+   |
+LL |     let _ = |(Ok(a) | Err(a)): Result<u8, u8>| 1; // `|Ok(a) | Err(a)| 1` parses as bit-or.
+   |                           ^ used in a pattern more than once
+
+warning: the feature `or_patterns` is incomplete and may cause the compiler to crash
+  --> $DIR/issue-54538-unused-parens-lint.rs:3:12
+   |
+LL | #![feature(or_patterns)]
+   |            ^^^^^^^^^^^
+   |
+   = note: `#[warn(incomplete_features)]` on by default
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:12:9
+   |
+LL |     let (a) = 0;
    |         ^^^ help: remove these parentheses
    |
 note: lint level defined here
-  --> $DIR/issue-54538-unused-parens-lint.rs:6:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:9:9
    |
-LL | #![warn(unused_parens)]
+LL | #![deny(unused_parens)]
    |         ^^^^^^^^^^^^^
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:11:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:13:9
+   |
+LL |     for (a) in 0..1 {}
+   |         ^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:14:12
+   |
+LL |     if let (a) = 0 {}
+   |            ^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:15:15
+   |
+LL |     while let (a) = 0 {}
+   |               ^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:16:12
+   |
+LL |     fn foo((a): u8) {}
+   |            ^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:17:14
+   |
+LL |     let _ = |(a): u8| 0;
+   |              ^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:43:12
+   |
+LL |     if let (0 | 1) = 0 {}
+   |            ^^^^^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:44:13
+   |
+LL |     if let ((0 | 1),) = (0,) {}
+   |             ^^^^^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:45:13
+   |
+LL |     if let [(0 | 1)] = [0] {}
+   |             ^^^^^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:46:16
+   |
+LL |     if let 0 | (1 | 2) = 0 {}
+   |                ^^^^^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:48:15
+   |
+LL |     if let TS((0 | 1)) = TS(0) {}
+   |               ^^^^^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:50:20
+   |
+LL |     if let NS { f: (0 | 1) } = (NS { f: 0 }) {}
+   |                    ^^^^^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:60:9
+   |
+LL |         (_) => {}
+   |         ^^^ help: remove these parentheses
+
+error: unnecessary parentheses around pattern
+  --> $DIR/issue-54538-unused-parens-lint.rs:61:9
    |
 LL |         (y) => {}
    |         ^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:12:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:62:9
    |
 LL |         (ref r) => {}
    |         ^^^^^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:13:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:63:9
    |
 LL |         (e @ 1...2) => {}
    |         ^^^^^^^^^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:19:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:69:9
    |
 LL |         (e @ &(1...2)) => {}
    |         ^^^^^^^^^^^^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:20:10
+  --> $DIR/issue-54538-unused-parens-lint.rs:70:10
    |
 LL |         &(_) => {}
    |          ^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:31:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:81:9
    |
 LL |         (_) => {}
    |         ^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:32:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:82:9
    |
 LL |         (y) => {}
    |         ^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:33:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:83:9
    |
 LL |         (ref r) => {}
    |         ^^^^^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:34:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:84:9
    |
 LL |         (e @ 1..=2) => {}
    |         ^^^^^^^^^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:40:9
+  --> $DIR/issue-54538-unused-parens-lint.rs:90:9
    |
 LL |         (e @ &(1..=2)) => {}
    |         ^^^^^^^^^^^^^^ help: remove these parentheses
 
-warning: unnecessary parentheses around pattern
+error: unnecessary parentheses around pattern
-  --> $DIR/issue-54538-unused-parens-lint.rs:41:10
+  --> $DIR/issue-54538-unused-parens-lint.rs:91:10
    |
 LL |         &(_) => {}
    |          ^^^ help: remove these parentheses
 
+error: aborting due to 26 previous errors
+
+For more information about this error, try `rustc --explain E0416`.