Skip to content

Closes #54538: unused_patterns lint #54820

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 69 additions & 42 deletions src/librustc_lint/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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.,
Expand All @@ -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);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/test/run-pass/binding/pat-tuple-7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
// run-pass

fn main() {
#[allow(unused_parens)]
match 0 {
(pat) => assert_eq!(pat, 0)
}
Expand Down
38 changes: 38 additions & 0 deletions src/test/ui/lint/issue-54538-unused-parens-lint.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
&_ => {}
}
}
42 changes: 42 additions & 0 deletions src/test/ui/lint/issue-54538-unused-parens-lint.stderr
Original file line number Diff line number Diff line change
@@ -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