diff --git a/src/librustc_lint/unused.rs b/src/librustc_lint/unused.rs index ae178888b6a1c..297b60d57289f 100644 --- a/src/librustc_lint/unused.rs +++ b/src/librustc_lint/unused.rs @@ -264,7 +264,7 @@ declare_lint! { pub struct UnusedParens; impl UnusedParens { - fn check_unused_parens_core(&self, + fn check_unused_parens_expr(&self, cx: &EarlyContext, value: &ast::Expr, msg: &str, @@ -273,46 +273,57 @@ impl UnusedParens { let necessary = struct_lit_needs_parens && parser::contains_exterior_struct_lit(&inner); if !necessary { - let span_msg = format!("unnecessary parentheses around {}", msg); - let mut err = cx.struct_span_lint(UNUSED_PARENS, - value.span, - &span_msg); - // Remove exactly one pair of parentheses (rather than naïvely - // stripping all paren characters) - let mut ate_left_paren = false; - let mut ate_right_paren = false; - let parens_removed = pprust::expr_to_string(value) - .trim_matches(|c| { - match c { - '(' => { - if ate_left_paren { - false - } else { - ate_left_paren = true; - true - } - }, - ')' => { - if ate_right_paren { - false - } else { - ate_right_paren = true; - true - } - }, - _ => false, - } - }).to_owned(); - err.span_suggestion_short_with_applicability( - value.span, - "remove these parentheses", - parens_removed, - Applicability::MachineApplicable - ); - err.emit(); + let pattern = pprust::expr_to_string(value); + Self::remove_outer_parens(cx, value.span, &pattern, msg); } } } + + fn check_unused_parens_pat(&self, + cx: &EarlyContext, + value: &ast::Pat, + msg: &str) { + if let ast::PatKind::Paren(_) = value.node { + let pattern = pprust::pat_to_string(value); + Self::remove_outer_parens(cx, value.span, &pattern, msg); + } + } + + fn remove_outer_parens(cx: &EarlyContext, span: Span, pattern: &str, msg: &str) { + let span_msg = format!("unnecessary parentheses around {}", msg); + let mut err = cx.struct_span_lint(UNUSED_PARENS, span, &span_msg); + let mut ate_left_paren = false; + let mut ate_right_paren = false; + let parens_removed = pattern + .trim_matches(|c| { + match c { + '(' => { + if ate_left_paren { + false + } else { + ate_left_paren = true; + true + } + }, + ')' => { + if ate_right_paren { + false + } else { + ate_right_paren = true; + true + } + }, + _ => false, + } + }).to_owned(); + err.span_suggestion_short_with_applicability( + span, + "remove these parentheses", + parens_removed, + Applicability::MachineApplicable + ); + err.emit(); + } } impl LintPass for UnusedParens { @@ -341,7 +352,9 @@ impl EarlyLintPass for UnusedParens { // first "argument" is self (which sometimes needs parens) MethodCall(_, ref args) => (&args[1..], "method"), // actual catch-all arm - _ => { return; } + _ => { + return; + } }; // Don't lint if this is a nested macro expansion: otherwise, the lint could // trigger in situations that macro authors shouldn't have to care about, e.g., @@ -354,18 +367,32 @@ impl EarlyLintPass for UnusedParens { } let msg = format!("{} argument", call_kind); for arg in args_to_check { - self.check_unused_parens_core(cx, arg, &msg, false); + self.check_unused_parens_expr(cx, arg, &msg, false); } return; } }; - self.check_unused_parens_core(cx, &value, msg, struct_lit_needs_parens); + self.check_unused_parens_expr(cx, &value, msg, struct_lit_needs_parens); + } + + 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 + // unnecessry 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") + } + } } fn check_stmt(&mut self, cx: &EarlyContext, s: &ast::Stmt) { if let ast::StmtKind::Local(ref local) = s.node { if let Some(ref value) = local.init { - self.check_unused_parens_core(cx, &value, "assigned value", false); + self.check_unused_parens_expr(cx, &value, "assigned value", false); } } } diff --git a/src/test/run-pass/binding/pat-tuple-7.rs b/src/test/run-pass/binding/pat-tuple-7.rs index c32b52eac33d1..06bd14d188e34 100644 --- a/src/test/run-pass/binding/pat-tuple-7.rs +++ b/src/test/run-pass/binding/pat-tuple-7.rs @@ -11,6 +11,7 @@ // run-pass fn main() { + #[allow(unused_parens)] match 0 { (pat) => assert_eq!(pat, 0) } diff --git a/src/test/ui/lint/issue-54538-unused-parens-lint.rs b/src/test/ui/lint/issue-54538-unused-parens-lint.rs new file mode 100644 index 0000000000000..97a2dd59a6209 --- /dev/null +++ b/src/test/ui/lint/issue-54538-unused-parens-lint.rs @@ -0,0 +1,38 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-pass + +#![allow(unreachable_patterns)] +#![allow(unused_variables)] +#![warn(unused_parens)] + +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 + 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 + } + + match &1 { + e @ &(1...2) | e @ &(3..=4) => {} // Complex ambiguous pattern should not warn + &_ => {} + } +} diff --git a/src/test/ui/lint/issue-54538-unused-parens-lint.stderr b/src/test/ui/lint/issue-54538-unused-parens-lint.stderr new file mode 100644 index 0000000000000..b76b969fd2b1a --- /dev/null +++ b/src/test/ui/lint/issue-54538-unused-parens-lint.stderr @@ -0,0 +1,42 @@ +warning: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:19:9 + | +LL | (_) => {} //~ WARNING: unnecessary parentheses around pattern + | ^^^ help: remove these parentheses + | +note: lint level defined here + --> $DIR/issue-54538-unused-parens-lint.rs:15:9 + | +LL | #![warn(unused_parens)] + | ^^^^^^^^^^^^^ + +warning: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:20:9 + | +LL | (y) => {} //~ WARNING: unnecessary parentheses around pattern + | ^^^ help: remove these parentheses + +warning: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:21:9 + | +LL | (ref r) => {} //~ WARNING: unnecessary parentheses around pattern + | ^^^^^^^ help: remove these parentheses + +warning: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:22:9 + | +LL | (e @ 1..=2) => {} //~ WARNING: unnecessary parentheses around outer pattern + | ^^^^^^^^^^^ help: remove these parentheses + +warning: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:28:9 + | +LL | (e @ &(1...2)) => {} //~ WARNING: unnecessary parentheses around outer pattern + | ^^^^^^^^^^^^^^ help: remove these parentheses + +warning: unnecessary parentheses around pattern + --> $DIR/issue-54538-unused-parens-lint.rs:29:10 + | +LL | &(_) => {} //~ WARNING: unnecessary parentheses around pattern + | ^^^ help: remove these parentheses +