Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit e85b181

Browse files
CentrilCentril
committedSep 3, 2019
unused_parens: fix for or-patterns + &(mut x)
1 parent b505208 commit e85b181

File tree

3 files changed

+256
-64
lines changed

3 files changed

+256
-64
lines changed
 

‎src/librustc_lint/unused.rs

Lines changed: 64 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -398,18 +398,37 @@ impl UnusedParens {
398398
}
399399
}
400400

401-
fn check_unused_parens_pat(&self,
402-
cx: &EarlyContext<'_>,
403-
value: &ast::Pat,
404-
msg: &str) {
405-
if let ast::PatKind::Paren(_) = value.node {
401+
fn check_unused_parens_pat(
402+
&self,
403+
cx: &EarlyContext<'_>,
404+
value: &ast::Pat,
405+
avoid_or: bool,
406+
avoid_mut: bool,
407+
) {
408+
use ast::{PatKind, BindingMode::ByValue, Mutability::Mutable};
409+
410+
if let PatKind::Paren(inner) = &value.node {
411+
match inner.node {
412+
// The lint visitor will visit each subpattern of `p`. We do not want to lint
413+
// any range pattern no matter where it occurs in the pattern. For something like
414+
// `&(a..=b)`, there is a recursive `check_pat` on `a` and `b`, but we will assume
415+
// that if there are unnecessary parens they serve a purpose of readability.
416+
PatKind::Range(..) => return,
417+
// Avoid `p0 | .. | pn` if we should.
418+
PatKind::Or(..) if avoid_or => return,
419+
// Avoid `mut x` and `mut x @ p` if we should:
420+
PatKind::Ident(ByValue(Mutable), ..) if avoid_mut => return,
421+
// Otherwise proceed with linting.
422+
_ => {}
423+
}
424+
406425
let pattern_text = if let Ok(snippet) = cx.sess().source_map()
407426
.span_to_snippet(value.span) {
408427
snippet
409428
} else {
410429
pprust::pat_to_string(value)
411430
};
412-
Self::remove_outer_parens(cx, value.span, &pattern_text, msg, (false, false));
431+
Self::remove_outer_parens(cx, value.span, &pattern_text, "pattern", (false, false));
413432
}
414433
}
415434

@@ -474,6 +493,13 @@ impl EarlyLintPass for UnusedParens {
474493
fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) {
475494
use syntax::ast::ExprKind::*;
476495
let (value, msg, followed_by_block, left_pos, right_pos) = match e.node {
496+
Let(ref pats, ..) => {
497+
for p in pats {
498+
self.check_unused_parens_pat(cx, p, false, false);
499+
}
500+
return;
501+
}
502+
477503
If(ref cond, ref block, ..) => {
478504
let left = e.span.lo() + syntax_pos::BytePos(2);
479505
let right = block.span.lo();
@@ -486,7 +512,8 @@ impl EarlyLintPass for UnusedParens {
486512
(cond, "`while` condition", true, Some(left), Some(right))
487513
},
488514

489-
ForLoop(_, ref cond, ref block, ..) => {
515+
ForLoop(ref pat, ref cond, ref block, ..) => {
516+
self.check_unused_parens_pat(cx, pat, false, false);
490517
(cond, "`for` head expression", true, None, Some(block.span.lo()))
491518
}
492519

@@ -531,26 +558,46 @@ impl EarlyLintPass for UnusedParens {
531558
}
532559

533560
fn check_pat(&mut self, cx: &EarlyContext<'_>, p: &ast::Pat) {
534-
use ast::PatKind::{Paren, Range};
535-
// The lint visitor will visit each subpattern of `p`. We do not want to lint any range
536-
// pattern no matter where it occurs in the pattern. For something like `&(a..=b)`, there
537-
// is a recursive `check_pat` on `a` and `b`, but we will assume that if there are
538-
// unnecessary parens they serve a purpose of readability.
539-
if let Paren(ref pat) = p.node {
540-
match pat.node {
541-
Range(..) => {}
542-
_ => self.check_unused_parens_pat(cx, &p, "pattern")
543-
}
561+
use ast::{PatKind::*, Mutability};
562+
match &p.node {
563+
// Do not lint on `(..)` as that will result in the other arms being useless.
564+
Paren(_)
565+
// The other cases do not contain sub-patterns.
566+
| Wild | Rest | Lit(..) | Mac(..) | Range(..) | Ident(.., None) | Path(..) => return,
567+
// These are list-like patterns; parens can always be removed.
568+
TupleStruct(_, ps) | Tuple(ps) | Slice(ps) | Or(ps) => for p in ps {
569+
self.check_unused_parens_pat(cx, p, false, false);
570+
},
571+
Struct(_, fps, _) => for f in fps {
572+
self.check_unused_parens_pat(cx, &f.pat, false, false);
573+
},
574+
// Avoid linting on `i @ (p0 | .. | pn)` and `box (p0 | .. | pn)`, #64106.
575+
Ident(.., Some(p)) | Box(p) => self.check_unused_parens_pat(cx, p, true, false),
576+
// Avoid linting on `&(mut x)` as `&mut x` has a different meaning, #55342.
577+
// Also avoid linting on `& mut? (p0 | .. | pn)`, #64106.
578+
Ref(p, m) => self.check_unused_parens_pat(cx, p, true, *m == Mutability::Immutable),
544579
}
545580
}
546581

547582
fn check_stmt(&mut self, cx: &EarlyContext<'_>, s: &ast::Stmt) {
548583
if let ast::StmtKind::Local(ref local) = s.node {
584+
self.check_unused_parens_pat(cx, &local.pat, false, false);
585+
549586
if let Some(ref value) = local.init {
550587
self.check_unused_parens_expr(cx, &value, "assigned value", false, None, None);
551588
}
552589
}
553590
}
591+
592+
fn check_param(&mut self, cx: &EarlyContext<'_>, param: &ast::Param) {
593+
self.check_unused_parens_pat(cx, &param.pat, true, false);
594+
}
595+
596+
fn check_arm(&mut self, cx: &EarlyContext<'_>, arm: &ast::Arm) {
597+
for p in &arm.pats {
598+
self.check_unused_parens_pat(cx, p, false, false);
599+
}
600+
}
554601
}
555602

556603
declare_lint! {

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

Lines changed: 70 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,75 @@
1-
// build-pass (FIXME(62277): could be check-pass?)
1+
#![feature(box_patterns)]
2+
3+
#![feature(or_patterns)]
4+
//~^ WARN the feature `or_patterns` is incomplete
25

36
#![allow(ellipsis_inclusive_range_patterns)]
47
#![allow(unreachable_patterns)]
58
#![allow(unused_variables)]
6-
#![warn(unused_parens)]
9+
#![deny(unused_parens)]
10+
11+
fn lint_on_top_level() {
12+
let (a) = 0; //~ ERROR unnecessary parentheses around pattern
13+
for (a) in 0..1 {} //~ ERROR unnecessary parentheses around pattern
14+
if let (a) = 0 {} //~ ERROR unnecessary parentheses around pattern
15+
while let (a) = 0 {} //~ ERROR unnecessary parentheses around pattern
16+
fn foo((a): u8) {} //~ ERROR unnecessary parentheses around pattern
17+
let _ = |(a): u8| 0; //~ ERROR unnecessary parentheses around pattern
18+
}
19+
20+
// Don't lint in these cases (#64106).
21+
fn or_patterns_no_lint() {
22+
match Box::new(0) {
23+
box (0 | 1) => {} // Should not lint as `box 0 | 1` binds as `(box 0) | 1`.
24+
_ => {}
25+
}
26+
27+
match 0 {
28+
x @ (0 | 1) => {} // Should not lint as `x @ 0 | 1` binds as `(x @ 0) | 1`.
29+
_ => {}
30+
}
31+
32+
if let &(0 | 1) = &0 {} // Should also not lint.
33+
if let &mut (0 | 1) = &mut 0 {} // Same.
34+
35+
fn foo((Ok(a) | Err(a)): Result<u8, u8>) {} // Doesn't parse if we remove parens for now.
36+
//~^ ERROR identifier `a` is bound more than once
37+
38+
let _ = |(Ok(a) | Err(a)): Result<u8, u8>| 1; // `|Ok(a) | Err(a)| 1` parses as bit-or.
39+
//~^ ERROR identifier `a` is bound more than once
40+
}
41+
42+
fn or_patterns_will_lint() {
43+
if let (0 | 1) = 0 {} //~ ERROR unnecessary parentheses around pattern
44+
if let ((0 | 1),) = (0,) {} //~ ERROR unnecessary parentheses around pattern
45+
if let [(0 | 1)] = [0] {} //~ ERROR unnecessary parentheses around pattern
46+
if let 0 | (1 | 2) = 0 {} //~ ERROR unnecessary parentheses around pattern
47+
struct TS(u8);
48+
if let TS((0 | 1)) = TS(0) {} //~ ERROR unnecessary parentheses around pattern
49+
struct NS { f: u8 }
50+
if let NS { f: (0 | 1) } = (NS { f: 0 }) {} //~ ERROR unnecessary parentheses around pattern
51+
}
52+
53+
// Don't lint on `&(mut x)` because `&mut x` means something else (#55342).
54+
fn deref_mut_binding_no_lint() {
55+
let &(mut x) = &0;
56+
}
757

858
fn main() {
959
match 1 {
10-
(_) => {} //~ WARNING: unnecessary parentheses around pattern
11-
(y) => {} //~ WARNING: unnecessary parentheses around pattern
12-
(ref r) => {} //~ WARNING: unnecessary parentheses around pattern
13-
(e @ 1...2) => {} //~ WARNING: unnecessary parentheses around outer pattern
14-
(1...2) => {} // Non ambiguous range pattern should not warn
60+
(_) => {} //~ ERROR unnecessary parentheses around pattern
61+
(y) => {} //~ ERROR unnecessary parentheses around pattern
62+
(ref r) => {} //~ ERROR unnecessary parentheses around pattern
63+
(e @ 1...2) => {} //~ ERROR unnecessary parentheses around pattern
64+
(1...2) => {} // Non ambiguous range pattern should not warn
1565
e @ (3...4) => {} // Non ambiguous range pattern should not warn
1666
}
1767

1868
match &1 {
19-
(e @ &(1...2)) => {} //~ WARNING: unnecessary parentheses around outer pattern
20-
&(_) => {} //~ WARNING: unnecessary parentheses around pattern
21-
e @ &(1...2) => {} // Ambiguous range pattern should not warn
22-
&(1...2) => {} // Ambiguous range pattern should not warn
69+
(e @ &(1...2)) => {} //~ ERROR unnecessary parentheses around pattern
70+
&(_) => {} //~ ERROR unnecessary parentheses around pattern
71+
e @ &(1...2) => {} // Ambiguous range pattern should not warn
72+
&(1...2) => {} // Ambiguous range pattern should not warn
2373
}
2474

2575
match &1 {
@@ -28,19 +78,19 @@ fn main() {
2878
}
2979

3080
match 1 {
31-
(_) => {} //~ WARNING: unnecessary parentheses around pattern
32-
(y) => {} //~ WARNING: unnecessary parentheses around pattern
33-
(ref r) => {} //~ WARNING: unnecessary parentheses around pattern
34-
(e @ 1..=2) => {} //~ WARNING: unnecessary parentheses around outer pattern
35-
(1..=2) => {} // Non ambiguous range pattern should not warn
81+
(_) => {} //~ ERROR unnecessary parentheses around pattern
82+
(y) => {} //~ ERROR unnecessary parentheses around pattern
83+
(ref r) => {} //~ ERROR unnecessary parentheses around pattern
84+
(e @ 1..=2) => {} //~ ERROR unnecessary parentheses around pattern
85+
(1..=2) => {} // Non ambiguous range pattern should not warn
3686
e @ (3..=4) => {} // Non ambiguous range pattern should not warn
3787
}
3888

3989
match &1 {
40-
(e @ &(1..=2)) => {} //~ WARNING: unnecessary parentheses around outer pattern
41-
&(_) => {} //~ WARNING: unnecessary parentheses around pattern
42-
e @ &(1..=2) => {} // Ambiguous range pattern should not warn
43-
&(1..=2) => {} // Ambiguous range pattern should not warn
90+
(e @ &(1..=2)) => {} //~ ERROR unnecessary parentheses around pattern
91+
&(_) => {} //~ ERROR unnecessary parentheses around pattern
92+
e @ &(1..=2) => {} // Ambiguous range pattern should not warn
93+
&(1..=2) => {} // Ambiguous range pattern should not warn
4494
}
4595

4696
match &1 {

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

Lines changed: 122 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,78 +1,173 @@
1-
warning: unnecessary parentheses around pattern
2-
--> $DIR/issue-54538-unused-parens-lint.rs:10:9
1+
error[E0416]: identifier `a` is bound more than once in the same pattern
2+
--> $DIR/issue-54538-unused-parens-lint.rs:35:25
33
|
4-
LL | (_) => {}
4+
LL | fn foo((Ok(a) | Err(a)): Result<u8, u8>) {} // Doesn't parse if we remove parens for now.
5+
| ^ used in a pattern more than once
6+
7+
error[E0416]: identifier `a` is bound more than once in the same pattern
8+
--> $DIR/issue-54538-unused-parens-lint.rs:38:27
9+
|
10+
LL | let _ = |(Ok(a) | Err(a)): Result<u8, u8>| 1; // `|Ok(a) | Err(a)| 1` parses as bit-or.
11+
| ^ used in a pattern more than once
12+
13+
warning: the feature `or_patterns` is incomplete and may cause the compiler to crash
14+
--> $DIR/issue-54538-unused-parens-lint.rs:3:12
15+
|
16+
LL | #![feature(or_patterns)]
17+
| ^^^^^^^^^^^
18+
|
19+
= note: `#[warn(incomplete_features)]` on by default
20+
21+
error: unnecessary parentheses around pattern
22+
--> $DIR/issue-54538-unused-parens-lint.rs:12:9
23+
|
24+
LL | let (a) = 0;
525
| ^^^ help: remove these parentheses
626
|
727
note: lint level defined here
8-
--> $DIR/issue-54538-unused-parens-lint.rs:6:9
28+
--> $DIR/issue-54538-unused-parens-lint.rs:9:9
929
|
10-
LL | #![warn(unused_parens)]
30+
LL | #![deny(unused_parens)]
1131
| ^^^^^^^^^^^^^
1232

13-
warning: unnecessary parentheses around pattern
14-
--> $DIR/issue-54538-unused-parens-lint.rs:11:9
33+
error: unnecessary parentheses around pattern
34+
--> $DIR/issue-54538-unused-parens-lint.rs:13:9
35+
|
36+
LL | for (a) in 0..1 {}
37+
| ^^^ help: remove these parentheses
38+
39+
error: unnecessary parentheses around pattern
40+
--> $DIR/issue-54538-unused-parens-lint.rs:14:12
41+
|
42+
LL | if let (a) = 0 {}
43+
| ^^^ help: remove these parentheses
44+
45+
error: unnecessary parentheses around pattern
46+
--> $DIR/issue-54538-unused-parens-lint.rs:15:15
47+
|
48+
LL | while let (a) = 0 {}
49+
| ^^^ help: remove these parentheses
50+
51+
error: unnecessary parentheses around pattern
52+
--> $DIR/issue-54538-unused-parens-lint.rs:16:12
53+
|
54+
LL | fn foo((a): u8) {}
55+
| ^^^ help: remove these parentheses
56+
57+
error: unnecessary parentheses around pattern
58+
--> $DIR/issue-54538-unused-parens-lint.rs:17:14
59+
|
60+
LL | let _ = |(a): u8| 0;
61+
| ^^^ help: remove these parentheses
62+
63+
error: unnecessary parentheses around pattern
64+
--> $DIR/issue-54538-unused-parens-lint.rs:43:12
65+
|
66+
LL | if let (0 | 1) = 0 {}
67+
| ^^^^^^^ help: remove these parentheses
68+
69+
error: unnecessary parentheses around pattern
70+
--> $DIR/issue-54538-unused-parens-lint.rs:44:13
71+
|
72+
LL | if let ((0 | 1),) = (0,) {}
73+
| ^^^^^^^ help: remove these parentheses
74+
75+
error: unnecessary parentheses around pattern
76+
--> $DIR/issue-54538-unused-parens-lint.rs:45:13
77+
|
78+
LL | if let [(0 | 1)] = [0] {}
79+
| ^^^^^^^ help: remove these parentheses
80+
81+
error: unnecessary parentheses around pattern
82+
--> $DIR/issue-54538-unused-parens-lint.rs:46:16
83+
|
84+
LL | if let 0 | (1 | 2) = 0 {}
85+
| ^^^^^^^ help: remove these parentheses
86+
87+
error: unnecessary parentheses around pattern
88+
--> $DIR/issue-54538-unused-parens-lint.rs:48:15
89+
|
90+
LL | if let TS((0 | 1)) = TS(0) {}
91+
| ^^^^^^^ help: remove these parentheses
92+
93+
error: unnecessary parentheses around pattern
94+
--> $DIR/issue-54538-unused-parens-lint.rs:50:20
95+
|
96+
LL | if let NS { f: (0 | 1) } = (NS { f: 0 }) {}
97+
| ^^^^^^^ help: remove these parentheses
98+
99+
error: unnecessary parentheses around pattern
100+
--> $DIR/issue-54538-unused-parens-lint.rs:60:9
101+
|
102+
LL | (_) => {}
103+
| ^^^ help: remove these parentheses
104+
105+
error: unnecessary parentheses around pattern
106+
--> $DIR/issue-54538-unused-parens-lint.rs:61:9
15107
|
16108
LL | (y) => {}
17109
| ^^^ help: remove these parentheses
18110

19-
warning: unnecessary parentheses around pattern
20-
--> $DIR/issue-54538-unused-parens-lint.rs:12:9
111+
error: unnecessary parentheses around pattern
112+
--> $DIR/issue-54538-unused-parens-lint.rs:62:9
21113
|
22114
LL | (ref r) => {}
23115
| ^^^^^^^ help: remove these parentheses
24116

25-
warning: unnecessary parentheses around pattern
26-
--> $DIR/issue-54538-unused-parens-lint.rs:13:9
117+
error: unnecessary parentheses around pattern
118+
--> $DIR/issue-54538-unused-parens-lint.rs:63:9
27119
|
28120
LL | (e @ 1...2) => {}
29121
| ^^^^^^^^^^^ help: remove these parentheses
30122

31-
warning: unnecessary parentheses around pattern
32-
--> $DIR/issue-54538-unused-parens-lint.rs:19:9
123+
error: unnecessary parentheses around pattern
124+
--> $DIR/issue-54538-unused-parens-lint.rs:69:9
33125
|
34126
LL | (e @ &(1...2)) => {}
35127
| ^^^^^^^^^^^^^^ help: remove these parentheses
36128

37-
warning: unnecessary parentheses around pattern
38-
--> $DIR/issue-54538-unused-parens-lint.rs:20:10
129+
error: unnecessary parentheses around pattern
130+
--> $DIR/issue-54538-unused-parens-lint.rs:70:10
39131
|
40132
LL | &(_) => {}
41133
| ^^^ help: remove these parentheses
42134

43-
warning: unnecessary parentheses around pattern
44-
--> $DIR/issue-54538-unused-parens-lint.rs:31:9
135+
error: unnecessary parentheses around pattern
136+
--> $DIR/issue-54538-unused-parens-lint.rs:81:9
45137
|
46138
LL | (_) => {}
47139
| ^^^ help: remove these parentheses
48140

49-
warning: unnecessary parentheses around pattern
50-
--> $DIR/issue-54538-unused-parens-lint.rs:32:9
141+
error: unnecessary parentheses around pattern
142+
--> $DIR/issue-54538-unused-parens-lint.rs:82:9
51143
|
52144
LL | (y) => {}
53145
| ^^^ help: remove these parentheses
54146

55-
warning: unnecessary parentheses around pattern
56-
--> $DIR/issue-54538-unused-parens-lint.rs:33:9
147+
error: unnecessary parentheses around pattern
148+
--> $DIR/issue-54538-unused-parens-lint.rs:83:9
57149
|
58150
LL | (ref r) => {}
59151
| ^^^^^^^ help: remove these parentheses
60152

61-
warning: unnecessary parentheses around pattern
62-
--> $DIR/issue-54538-unused-parens-lint.rs:34:9
153+
error: unnecessary parentheses around pattern
154+
--> $DIR/issue-54538-unused-parens-lint.rs:84:9
63155
|
64156
LL | (e @ 1..=2) => {}
65157
| ^^^^^^^^^^^ help: remove these parentheses
66158

67-
warning: unnecessary parentheses around pattern
68-
--> $DIR/issue-54538-unused-parens-lint.rs:40:9
159+
error: unnecessary parentheses around pattern
160+
--> $DIR/issue-54538-unused-parens-lint.rs:90:9
69161
|
70162
LL | (e @ &(1..=2)) => {}
71163
| ^^^^^^^^^^^^^^ help: remove these parentheses
72164

73-
warning: unnecessary parentheses around pattern
74-
--> $DIR/issue-54538-unused-parens-lint.rs:41:10
165+
error: unnecessary parentheses around pattern
166+
--> $DIR/issue-54538-unused-parens-lint.rs:91:10
75167
|
76168
LL | &(_) => {}
77169
| ^^^ help: remove these parentheses
78170

171+
error: aborting due to 26 previous errors
172+
173+
For more information about this error, try `rustc --explain E0416`.

0 commit comments

Comments
 (0)
Please sign in to comment.