diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index 90e467713968b..39c0698aeec9f 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -398,18 +398,37 @@ impl UnusedParens { } } - fn check_unused_parens_pat(&self, - cx: &EarlyContext<'_>, - value: &ast::Pat, - msg: &str) { - if let ast::PatKind::Paren(_) = value.node { + fn check_unused_parens_pat( + &self, + cx: &EarlyContext<'_>, + value: &ast::Pat, + 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}; - // 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. - if let Paren(ref pat) = p.node { - match pat.node { - Range(..) => {} - _ => self.check_unused_parens_pat(cx, &p, "pattern") - } + use ast::{PatKind::*, Mutability}; + match &p.node { + // Do not lint on `(..)` as that will result in the other arms being useless. + Paren(_) + // The other cases do not contain sub-patterns. + | Wild | Rest | Lit(..) | Mac(..) | Range(..) | Ident(.., None) | Path(..) => return, + // These are list-like patterns; parens can always be removed. + TupleStruct(_, ps) | Tuple(ps) | Slice(ps) | Or(ps) => for p in ps { + 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, ¶m.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! { diff --git a/src/test/ui/lint/issue-54538-unused-parens-lint.rs b/src/test/ui/lint/issue-54538-unused-parens-lint.rs index eda9e2cdfaa2f..c442c39fe010e 100644 --- a/src/test/ui/lint/issue-54538-unused-parens-lint.rs +++ b/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) {} // Doesn't parse if we remove parens for now. + //~^ ERROR identifier `a` is bound more than once + + let _ = |(Ok(a) | Err(a)): Result| 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 - (y) => {} //~ WARNING: unnecessary parentheses around pattern - (ref r) => {} //~ WARNING: unnecessary parentheses around pattern - (e @ 1...2) => {} //~ WARNING: unnecessary parentheses around outer pattern - (1...2) => {} // Non ambiguous range pattern should not warn + (_) => {} //~ ERROR unnecessary parentheses around pattern + (y) => {} //~ ERROR unnecessary parentheses around pattern + (ref r) => {} //~ ERROR unnecessary parentheses around pattern + (e @ 1...2) => {} //~ ERROR unnecessary parentheses around pattern + (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 - &(_) => {} //~ WARNING: unnecessary parentheses around pattern - e @ &(1...2) => {} // Ambiguous range pattern should not warn - &(1...2) => {} // Ambiguous range pattern should not warn + (e @ &(1...2)) => {} //~ ERROR unnecessary parentheses around pattern + &(_) => {} //~ ERROR unnecessary parentheses around pattern + e @ &(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 - (y) => {} //~ WARNING: unnecessary parentheses around pattern - (ref r) => {} //~ WARNING: unnecessary parentheses around pattern - (e @ 1..=2) => {} //~ WARNING: unnecessary parentheses around outer pattern - (1..=2) => {} // Non ambiguous range pattern should not warn + (_) => {} //~ ERROR unnecessary parentheses around pattern + (y) => {} //~ ERROR unnecessary parentheses around pattern + (ref r) => {} //~ ERROR unnecessary parentheses around pattern + (e @ 1..=2) => {} //~ ERROR unnecessary parentheses around pattern + (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 - &(_) => {} //~ WARNING: unnecessary parentheses around pattern - e @ &(1..=2) => {} // Ambiguous range pattern should not warn - &(1..=2) => {} // Ambiguous range pattern should not warn + (e @ &(1..=2)) => {} //~ ERROR unnecessary parentheses around pattern + &(_) => {} //~ ERROR unnecessary parentheses around pattern + e @ &(1..=2) => {} // Ambiguous range pattern should not warn + &(1..=2) => {} // Ambiguous range pattern should not warn } match &1 { diff --git a/src/test/ui/lint/issue-54538-unused-parens-lint.stderr b/src/test/ui/lint/issue-54538-unused-parens-lint.stderr index 3b312198952a5..a3e0fb938b3c6 100644 --- a/src/test/ui/lint/issue-54538-unused-parens-lint.stderr +++ b/src/test/ui/lint/issue-54538-unused-parens-lint.stderr @@ -1,78 +1,173 @@ -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:10:9 +error[E0416]: identifier `a` is bound more than once in the same pattern + --> $DIR/issue-54538-unused-parens-lint.rs:35:25 | -LL | (_) => {} +LL | fn foo((Ok(a) | Err(a)): Result) {} // 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| 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 - --> $DIR/issue-54538-unused-parens-lint.rs:11:9 +error: unnecessary parentheses around pattern + --> $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 - --> $DIR/issue-54538-unused-parens-lint.rs:12:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:62:9 | LL | (ref r) => {} | ^^^^^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:13:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:63:9 | LL | (e @ 1...2) => {} | ^^^^^^^^^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:19:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:69:9 | LL | (e @ &(1...2)) => {} | ^^^^^^^^^^^^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:20:10 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:70:10 | LL | &(_) => {} | ^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:31:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:81:9 | LL | (_) => {} | ^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:32:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:82:9 | LL | (y) => {} | ^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:33:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:83:9 | LL | (ref r) => {} | ^^^^^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:34:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:84:9 | LL | (e @ 1..=2) => {} | ^^^^^^^^^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:40:9 +error: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:90:9 | LL | (e @ &(1..=2)) => {} | ^^^^^^^^^^^^^^ help: remove these parentheses -warning: unnecessary parentheses around pattern - --> $DIR/issue-54538-unused-parens-lint.rs:41:10 +error: unnecessary parentheses around pattern + --> $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`.