Skip to content

Commit

Permalink
match_single_binding: Fix invalid suggestion when match scrutinee has…
Browse files Browse the repository at this point in the history
… side effects
  • Loading branch information
Y-Nak committed May 13, 2021
1 parent 08e36d7 commit 8214bf0
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 35 deletions.
37 changes: 28 additions & 9 deletions clippy_lints/src/matches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1479,15 +1479,34 @@ fn check_match_single_binding<'a>(cx: &LateContext<'a>, ex: &Expr<'a>, arms: &[A
);
},
PatKind::Wild => {
span_lint_and_sugg(
cx,
MATCH_SINGLE_BINDING,
expr.span,
"this match could be replaced by its body itself",
"consider using the match body instead",
snippet_body,
Applicability::MachineApplicable,
);
if ex.can_have_side_effects() {
let indent = " ".repeat(indent_of(cx, expr.span).unwrap_or(0));
let sugg = format!(
"{};\n{}{}",
snippet_with_applicability(cx, ex.span, "..", &mut applicability),
indent,
snippet_body
);
span_lint_and_sugg(
cx,
MATCH_SINGLE_BINDING,
expr.span,
"this match could be replaced by its scrutinee and body",
"consider using the scrutinee and body instead",
sugg,
applicability,
)
} else {
span_lint_and_sugg(
cx,
MATCH_SINGLE_BINDING,
expr.span,
"this match could be replaced by its body itself",
"consider using the match body instead",
snippet_body,
Applicability::MachineApplicable,
);
}
},
_ => (),
}
Expand Down
5 changes: 1 addition & 4 deletions tests/ui/match_single_binding.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,7 @@ fn main() {
0 => println!("Disabled branch"),
_ => println!("Enabled branch"),
}
// Lint
let x = 1;
let y = 1;
println!("Single branch");

// Ok
let x = 1;
let y = 1;
Expand Down
10 changes: 1 addition & 9 deletions tests/ui/match_single_binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,7 @@ fn main() {
0 => println!("Disabled branch"),
_ => println!("Enabled branch"),
}
// Lint
let x = 1;
let y = 1;
match match y {
0 => 1,
_ => 2,
} {
_ => println!("Single branch"),
}

// Ok
let x = 1;
let y = 1;
Expand Down
13 changes: 1 addition & 12 deletions tests/ui/match_single_binding.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,5 @@ LL | unwrapped
LL | })
|

error: this match could be replaced by its body itself
--> $DIR/match_single_binding.rs:112:5
|
LL | / match match y {
LL | | 0 => 1,
LL | | _ => 2,
LL | | } {
LL | | _ => println!("Single branch"),
LL | | }
| |_____^ help: consider using the match body instead: `println!("Single branch");`

error: aborting due to 12 previous errors
error: aborting due to 11 previous errors

16 changes: 16 additions & 0 deletions tests/ui/match_single_binding2.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,20 @@ fn main() {
},
None => println!("nothing"),
}

fn side_effects() {}

// Lint (scrutinee has side effects)
// issue #7094
side_effects();
println!("Side effects");

// Lint (scrutinee has side effects)
// issue #7094
let x = 1;
match x {
0 => 1,
_ => 2,
};
println!("Single branch");
}
18 changes: 18 additions & 0 deletions tests/ui/match_single_binding2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,22 @@ fn main() {
},
None => println!("nothing"),
}

fn side_effects() {}

// Lint (scrutinee has side effects)
// issue #7094
match side_effects() {
_ => println!("Side effects"),
}

// Lint (scrutinee has side effects)
// issue #7094
let x = 1;
match match x {
0 => 1,
_ => 2,
} {
_ => println!("Single branch"),
}
}
36 changes: 35 additions & 1 deletion tests/ui/match_single_binding2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,39 @@ LL | let (a, b) = get_tup();
LL | println!("a {:?} and b {:?}", a, b);
|

error: aborting due to 2 previous errors
error: this match could be replaced by its scrutinee and body
--> $DIR/match_single_binding2.rs:42:5
|
LL | / match side_effects() {
LL | | _ => println!("Side effects"),
LL | | }
| |_____^
|
help: consider using the scrutinee and body instead
|
LL | side_effects();
LL | println!("Side effects");
|

error: this match could be replaced by its scrutinee and body
--> $DIR/match_single_binding2.rs:49:5
|
LL | / match match x {
LL | | 0 => 1,
LL | | _ => 2,
LL | | } {
LL | | _ => println!("Single branch"),
LL | | }
| |_____^
|
help: consider using the scrutinee and body instead
|
LL | match x {
LL | 0 => 1,
LL | _ => 2,
LL | };
LL | println!("Single branch");
|

error: aborting due to 4 previous errors

0 comments on commit 8214bf0

Please sign in to comment.