Skip to content
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

match_single_binding: Fix invalid suggestion when match scrutinee has side effects #7095

Merged
merged 1 commit into from
May 13, 2021
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
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