Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-10 23:24:24 +00:00
Code Issues Projects Releases Packages Wiki Activity

single_match: Don't lint non-exhaustive matches; support tuples

Browse source 
This commit changes the behavior of `single_match` lint.

After that, we won't lint non-exhaustive matches like this:

```rust
match Some(v) {
    Some(a) => println!("${:?}", a),
    None => {},
}
```

The rationale is that, because the type of `a` could be changed, so the
user can get non-exhaustive match after applying the suggested lint (see
https://github.com/rust-lang/rust-clippy/issues/8282#issuecomment-1013566068
for context).

We also will lint `match` constructions with tuples. When we see the
tuples on the both arms, we will check them both at the same time, and
if they form exhaustive match, we could display the warning.

Closes #8282
This commit is contained in:
Georgy Komarov 2022-01-20 14:50:14 +03:00
parent 496f26c229
commit a5a07e503f
4 changed files with 209 additions and 72 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

151
clippy_lints/src/matches.rs
View file

@ -5,7 +5,7 @@ use clippy_utils::diagnostics::{
use clippy_utils::macros::{is_panic, root_macro_call};
use clippy_utils::source::{expr_block, indent_of, snippet, snippet_block, snippet_opt, snippet_with_applicability};
use clippy_utils::sugg::Sugg;
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, match_type, peel_mid_ty_refs};
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item, peel_mid_ty_refs};
use clippy_utils::visitors::is_local_used;
use clippy_utils::{
get_parent_expr, is_lang_ctor, is_lint_allowed, is_refutable, is_unit_expr, is_wild, meets_msrv, msrvs,
@ -31,7 +31,7 @@ use rustc_semver::RustcVersion;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::source_map::{Span, Spanned};
use rustc_span::sym;
use std::cmp::Ordering;
use std::cmp::{max, Ordering};
use std::collections::hash_map::Entry;
declare_clippy_lint! {
@ -741,7 +741,7 @@ fn check_single_match(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], exp
let ty = cx.typeck_results().expr_ty(ex);
if *ty.kind() != ty::Bool || is_lint_allowed(cx, MATCH_BOOL, ex.hir_id) {
check_single_match_single_pattern(cx, ex, arms, expr, els);
check_single_match_opt_like(cx, ex, arms, expr, ty, els);
check_single_match_opt_like(cx, ex, arms, expr, els);
}
}
}
@ -835,7 +835,6 @@ fn check_single_match_opt_like(
ex: &Expr<'_>,
arms: &[Arm<'_>],
expr: &Expr<'_>,
ty: Ty<'_>,
els: Option<&Expr<'_>>,
) {
// list of candidate `Enum`s we know will never get any more members
@ -849,25 +848,135 @@ fn check_single_match_opt_like(
(&paths::RESULT, "Ok"),
];
let path = match arms[1].pat.kind {
PatKind::TupleStruct(ref path, inner, _) => {
// Contains any non wildcard patterns (e.g., `Err(err)`)?
if !inner.iter().all(is_wild) {
return;
}
rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false))
},
PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => ident.to_string(),
PatKind::Path(ref path) => {
rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| s.print_qpath(path, false))
},
_ => return,
};
// We want to suggest to exclude an arm that contains only wildcards or forms the ehaustive
// match with the second branch.
if !contains_only_wilds(arms[1].pat) && !form_exhaustive_tuples(arms[0].pat, arms[1].pat) {
return;
}
for &(ty_path, pat_path) in candidates {
if path == *pat_path && match_type(cx, ty, ty_path) {
report_single_match_single_pattern(cx, ex, arms, expr, els);
let mut paths = Vec::new();
if !collect_pat_paths(&mut paths, arms[1].pat) {
return;
}
let in_candidate_enum = |path: &String| -> bool {
for &(_, pat_path) in candidates {
if path == pat_path {
return true;
}
}
false
};
if paths.iter().all(in_candidate_enum) {
report_single_match_single_pattern(cx, ex, arms, expr, els);
}
}
/// Collects paths from the given paths. Returns true if the given pattern could be simplified,
/// false otherwise.
fn collect_pat_paths(acc: &mut Vec<String>, pat: &Pat<'_>) -> bool {
match pat.kind {
PatKind::Wild => true,
PatKind::Tuple(inner, _) => {
for p in inner {
if !collect_pat_paths(acc, p) {
return false;
}
}
true
},
PatKind::TupleStruct(ref path, ..) => {
acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| {
s.print_qpath(path, false);
}));
true
},
PatKind::Binding(BindingAnnotation::Unannotated, .., ident, None) => {
acc.push(ident.to_string());
true
},
PatKind::Path(ref path) => {
acc.push(rustc_hir_pretty::to_string(rustc_hir_pretty::NO_ANN, |s| {
s.print_qpath(path, false);
}));
true
},
_ => false,
}
}
/// Returns true if the given arm of pattern matching contains wildcard patterns.
fn contains_only_wilds(pat: &Pat<'_>) -> bool {
match pat.kind {
PatKind::Wild => true,
PatKind::Tuple(inner, _) | PatKind::TupleStruct(_, inner, ..) => inner.iter().all(contains_only_wilds),
_ => false,
}
}
/// Returns true if the given patterns form the tuples that exhaustively matches all possible
/// parameters.
///
/// Here are some examples:
///
/// ```
/// // Doesn't form exhaustive match, because the first arm may contain not only E::V.
/// match x {
/// (Some(E::V), _) => todo!(),
/// (None, _) => {}
/// }
///
/// // Forms exhaustive match, because the patterns cover all possible cases at the same positions.
/// match x {
/// (Some(_), _) => todo!(),
/// (None, _) => {}
/// }
/// ```
fn form_exhaustive_tuples(left: &Pat<'_>, right: &Pat<'_>) -> bool {
match (&left.kind, &right.kind) {
(PatKind::Wild, _) | (_, PatKind::Wild) => true,
(PatKind::Tuple(left_in, left_pos), PatKind::Tuple(right_in, right_pos)) => {
// We don't actually know the position and presence of the `..` (dotdot) operator in
// the arms, so we need to evaluate the correct offsets here in order to iterate in
// both arms at the same time.
let len = max(
left_in.len() + {
if left_pos.is_some() { 1 } else { 0 }
},
right_in.len() + {
if right_pos.is_some() { 1 } else { 0 }
},
);
let mut left_pos = left_pos.unwrap_or(usize::MAX);
let mut right_pos = right_pos.unwrap_or(usize::MAX);
let mut left_span = 0;
let mut right_span = 0;
for i in 0..len {
let mut found_dotdot = false;
if i == left_pos {
left_span += 1;
if left_span < len - left_in.len() {
left_pos += 1;
}
found_dotdot = true;
}
if i == right_pos {
right_span += 1;
if right_span < len - right_in.len() {
right_pos += 1;
}
found_dotdot = true;
}
if found_dotdot {
continue;
}
if !contains_only_wilds(&left_in[i - left_span]) && !contains_only_wilds(&right_in[i - right_span]) {
return false;
}
}
true
},
_ => false,
}
}

50
tests/ui/single_match.rs
View file

@ -145,6 +145,56 @@ fn if_suggestion() {
};
}
// See: issue #8282
fn ranges() {
enum E {
V,
}
let x = (Some(E::V), Some(42));
// don't lint
match x {
(Some(E::V), _) => {},
(None, _) => {},
}
// lint
match x {
(Some(_), _) => {},
(None, _) => {},
}
// lint
match x {
(Some(E::V), _) => todo!(),
(_, _) => {},
}
// lint
match (Some(42), Some(E::V), Some(42)) {
(.., Some(E::V), _) => {},
(..) => {},
}
// don't lint
match (Some(E::V), Some(E::V), Some(E::V)) {
(.., Some(E::V), _) => {},
(.., None, _) => {},
}
// don't lint
match (Some(E::V), Some(E::V), Some(E::V)) {
(Some(E::V), ..) => {},
(None, ..) => {},
}
// don't lint
match (Some(E::V), Some(E::V), Some(E::V)) {
(_, Some(E::V), ..) => {},
(_, None, ..) => {},
}
}
macro_rules! single_match {
($num:literal) => {
match $num {

38
tests/ui/single_match.stderr
View file

@ -38,15 +38,6 @@ LL | | _ => {},
LL | | };
| |_____^ help: try this: `if let (2..=3, 7..=9) = z { dummy() }`
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:54:5
|
LL | / match x {
LL | | Some(y) => dummy(),
LL | | None => (),
LL | | };
| |_____^ help: try this: `if let Some(y) = x { dummy() }`
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:59:5
|
@ -128,5 +119,32 @@ LL | | _ => (),
LL | | };
| |_____^ help: try this: `if let None = x { println!() }`
error: aborting due to 13 previous errors
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:162:5
|
LL | / match x {
LL | | (Some(_), _) => {},
LL | | (None, _) => {},
LL | | }
| |_____^ help: try this: `if let (Some(_), _) = x {}`
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:168:5
|
LL | / match x {
LL | | (Some(E::V), _) => todo!(),
LL | | (_, _) => {},
LL | | }
| |_____^ help: try this: `if let (Some(E::V), _) = x { todo!() }`
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match.rs:174:5
|
LL | / match (Some(42), Some(E::V), Some(42)) {
LL | | (.., Some(E::V), _) => {},
LL | | (..) => {},
LL | | }
| |_____^ help: try this: `if let (.., Some(E::V), _) = (Some(42), Some(E::V), Some(42)) {}`
error: aborting due to 15 previous errors

42
tests/ui/single_match_else.stderr
View file

@ -19,45 +19,5 @@ LL + None
LL + }
|
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match_else.rs:70:5
|
LL | / match Some(1) {
LL | | Some(a) => println!("${:?}", a),
LL | | None => {
LL | | println!("else block");
LL | | return
LL | | },
LL | | }
| |_____^
|
help: try this
|
LL ~ if let Some(a) = Some(1) { println!("${:?}", a) } else {
LL + println!("else block");
LL + return
LL + }
|
error: you seem to be trying to use `match` for destructuring a single pattern. Consider using `if let`
--> $DIR/single_match_else.rs:79:5
|
LL | / match Some(1) {
LL | | Some(a) => println!("${:?}", a),
LL | | None => {
LL | | println!("else block");
LL | | return;
LL | | },
LL | | }
| |_____^
|
help: try this
|
LL ~ if let Some(a) = Some(1) { println!("${:?}", a) } else {
LL + println!("else block");
LL + return;
LL + }
|
error: aborting due to 3 previous errors
error: aborting due to previous error