Skip to content

Commit 8214bf0

Browse files
committed
match_single_binding: Fix invalid suggestion when match scrutinee has side effects
1 parent 08e36d7 commit 8214bf0

File tree

7 files changed

+100
-35
lines changed

7 files changed

+100
-35
lines changed

clippy_lints/src/matches.rs

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1479,15 +1479,34 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A
14791479
);
14801480
},
14811481
PatKind::Wild => {
1482-
span_lint_and_sugg(
1483-
cx,
1484-
MATCH_SINGLE_BINDING,
1485-
expr.span,
1486-
"this match could be replaced by its body itself",
1487-
"consider using the match body instead",
1488-
snippet_body,
1489-
Applicability::MachineApplicable,
1490-
);
1482+
if ex.can_have_side_effects() {
1483+
let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
1484+
let sugg = format!(
1485+
"{};\n{}{}",
1486+
snippet_with_applicability(cx, ex.span, "..", &mut applicability),
1487+
indent,
1488+
snippet_body
1489+
);
1490+
span_lint_and_sugg(
1491+
cx,
1492+
MATCH_SINGLE_BINDING,
1493+
expr.span,
1494+
"this match could be replaced by its scrutinee and body",
1495+
"consider using the scrutinee and body instead",
1496+
sugg,
1497+
applicability,
1498+
)
1499+
} else {
1500+
span_lint_and_sugg(
1501+
cx,
1502+
MATCH_SINGLE_BINDING,
1503+
expr.span,
1504+
"this match could be replaced by its body itself",
1505+
"consider using the match body instead",
1506+
snippet_body,
1507+
Applicability::MachineApplicable,
1508+
);
1509+
}
14911510
},
14921511
_ => (),
14931512
}

tests/ui/match_single_binding.fixed

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,10 +94,7 @@ fn main() {
9494
0 => println!("Disabled branch"),
9595
_ => println!("Enabled branch"),
9696
}
97-
// Lint
98-
let x = 1;
99-
let y = 1;
100-
println!("Single branch");
97+
10198
// Ok
10299
let x = 1;
103100
let y = 1;

tests/ui/match_single_binding.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -106,15 +106,7 @@ fn main() {
106106
0 => println!("Disabled branch"),
107107
_ => println!("Enabled branch"),
108108
}
109-
// Lint
110-
let x = 1;
111-
let y = 1;
112-
match match y {
113-
0 => 1,
114-
_ => 2,
115-
} {
116-
_ => println!("Single branch"),
117-
}
109+
118110
// Ok
119111
let x = 1;
120112
let y = 1;

tests/ui/match_single_binding.stderr

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -167,16 +167,5 @@ LL | unwrapped
167167
LL | })
168168
|
169169

170-
error: this match could be replaced by its body itself
171-
--> $DIR/match_single_binding.rs:112:5
172-
|
173-
LL | / match match y {
174-
LL | | 0 => 1,
175-
LL | | _ => 2,
176-
LL | | } {
177-
LL | | _ => println!("Single branch"),
178-
LL | | }
179-
| |_____^ help: consider using the match body instead: `println!("Single branch");`
180-
181-
error: aborting due to 12 previous errors
170+
error: aborting due to 11 previous errors
182171

tests/ui/match_single_binding2.fixed

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,20 @@ fn main() {
3434
},
3535
None => println!("nothing"),
3636
}
37+
38+
fn side_effects() {}
39+
40+
// Lint (scrutinee has side effects)
41+
// issue #7094
42+
side_effects();
43+
println!("Side effects");
44+
45+
// Lint (scrutinee has side effects)
46+
// issue #7094
47+
let x = 1;
48+
match x {
49+
0 => 1,
50+
_ => 2,
51+
};
52+
println!("Single branch");
3753
}

tests/ui/match_single_binding2.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,22 @@ fn main() {
3434
},
3535
None => println!("nothing"),
3636
}
37+
38+
fn side_effects() {}
39+
40+
// Lint (scrutinee has side effects)
41+
// issue #7094
42+
match side_effects() {
43+
_ => println!("Side effects"),
44+
}
45+
46+
// Lint (scrutinee has side effects)
47+
// issue #7094
48+
let x = 1;
49+
match match x {
50+
0 => 1,
51+
_ => 2,
52+
} {
53+
_ => println!("Single branch"),
54+
}
3755
}

tests/ui/match_single_binding2.stderr

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,5 +30,39 @@ LL | let (a, b) = get_tup();
3030
LL | println!("a {:?} and b {:?}", a, b);
3131
|
3232

33-
error: aborting due to 2 previous errors
33+
error: this match could be replaced by its scrutinee and body
34+
--> $DIR/match_single_binding2.rs:42:5
35+
|
36+
LL | / match side_effects() {
37+
LL | | _ => println!("Side effects"),
38+
LL | | }
39+
| |_____^
40+
|
41+
help: consider using the scrutinee and body instead
42+
|
43+
LL | side_effects();
44+
LL | println!("Side effects");
45+
|
46+
47+
error: this match could be replaced by its scrutinee and body
48+
--> $DIR/match_single_binding2.rs:49:5
49+
|
50+
LL | / match match x {
51+
LL | | 0 => 1,
52+
LL | | _ => 2,
53+
LL | | } {
54+
LL | | _ => println!("Single branch"),
55+
LL | | }
56+
| |_____^
57+
|
58+
help: consider using the scrutinee and body instead
59+
|
60+
LL | match x {
61+
LL | 0 => 1,
62+
LL | _ => 2,
63+
LL | };
64+
LL | println!("Single branch");
65+
|
66+
67+
error: aborting due to 4 previous errors
3468

0 commit comments

Comments
 (0)