From f9a6dfa60d4f55ec9a98bc517dea4e4fb9c0f61a Mon Sep 17 00:00:00 2001 From: "Samuel \"Sam\" Tardieu" Date: Thu, 27 Jul 2023 19:04:01 +0200 Subject: [PATCH] New lint `ignored_unit_patterns` --- CHANGELOG.md | 1 + clippy_dev/src/main.rs | 2 +- clippy_dev/src/setup/vscode.rs | 2 +- clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/ignored_unit_patterns.rs | 52 +++++++++++++++++++++++ clippy_lints/src/lib.rs | 2 + clippy_lints/src/option_if_let_else.rs | 2 +- tests/ui/ignored_unit_patterns.fixed | 17 ++++++++ tests/ui/ignored_unit_patterns.rs | 17 ++++++++ tests/ui/ignored_unit_patterns.stderr | 28 ++++++++++++ 10 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 clippy_lints/src/ignored_unit_patterns.rs create mode 100644 tests/ui/ignored_unit_patterns.fixed create mode 100644 tests/ui/ignored_unit_patterns.rs create mode 100644 tests/ui/ignored_unit_patterns.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 251f32d2e..370772498 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4890,6 +4890,7 @@ Released 2018-09-13 [`if_same_then_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_same_then_else [`if_then_some_else_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#if_then_some_else_none [`ifs_same_cond`]: https://rust-lang.github.io/rust-clippy/master/index.html#ifs_same_cond +[`ignored_unit_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#ignored_unit_patterns [`impl_trait_in_params`]: https://rust-lang.github.io/rust-clippy/master/index.html#impl_trait_in_params [`implicit_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_clone [`implicit_hasher`]: https://rust-lang.github.io/rust-clippy/master/index.html#implicit_hasher diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs index 43eaccdf5..fca750faf 100644 --- a/clippy_dev/src/main.rs +++ b/clippy_dev/src/main.rs @@ -41,7 +41,7 @@ fn main() { matches.get_one::("type").map(String::as_str), matches.get_flag("msrv"), ) { - Ok(_) => update_lints::update(update_lints::UpdateMode::Change), + Ok(()) => update_lints::update(update_lints::UpdateMode::Change), Err(e) => eprintln!("Unable to create lint: {e}"), } }, diff --git a/clippy_dev/src/setup/vscode.rs b/clippy_dev/src/setup/vscode.rs index dbcdc9b59..204f4af2c 100644 --- a/clippy_dev/src/setup/vscode.rs +++ b/clippy_dev/src/setup/vscode.rs @@ -47,7 +47,7 @@ fn check_install_precondition(force_override: bool) -> bool { } } else { match fs::create_dir(vs_dir_path) { - Ok(_) => { + Ok(()) => { println!("info: created `{VSCODE_DIR}` directory for clippy"); }, Err(err) => { diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index e08447143..413025daf 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -203,6 +203,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::if_let_mutex::IF_LET_MUTEX_INFO, crate::if_not_else::IF_NOT_ELSE_INFO, crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO, + crate::ignored_unit_patterns::IGNORED_UNIT_PATTERNS_INFO, crate::implicit_hasher::IMPLICIT_HASHER_INFO, crate::implicit_return::IMPLICIT_RETURN_INFO, crate::implicit_saturating_add::IMPLICIT_SATURATING_ADD_INFO, diff --git a/clippy_lints/src/ignored_unit_patterns.rs b/clippy_lints/src/ignored_unit_patterns.rs new file mode 100644 index 000000000..c635120b8 --- /dev/null +++ b/clippy_lints/src/ignored_unit_patterns.rs @@ -0,0 +1,52 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use hir::PatKind; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `_` in patterns of type `()`. + /// + /// ### Why is this bad? + /// Matching with `()` explicitly instead of `_` outlines + /// the fact that the pattern contains no data. Also it + /// would detect a type change that `_` would ignore. + /// + /// ### Example + /// ```rust + /// match std::fs::create_dir("tmp-work-dir") { + /// Ok(_) => println!("Working directory created"), + /// Err(s) => eprintln!("Could not create directory: {s}"), + /// } + /// ``` + /// Use instead: + /// ```rust + /// match std::fs::create_dir("tmp-work-dir") { + /// Ok(()) => println!("Working directory created"), + /// Err(s) => eprintln!("Could not create directory: {s}"), + /// } + /// ``` + #[clippy::version = "1.73.0"] + pub IGNORED_UNIT_PATTERNS, + pedantic, + "suggest replacing `_` by `()` in patterns where appropriate" +} +declare_lint_pass!(IgnoredUnitPatterns => [IGNORED_UNIT_PATTERNS]); + +impl<'tcx> LateLintPass<'tcx> for IgnoredUnitPatterns { + fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx hir::Pat<'tcx>) { + if matches!(pat.kind, PatKind::Wild) && cx.typeck_results().pat_ty(pat).is_unit() { + span_lint_and_sugg( + cx, + IGNORED_UNIT_PATTERNS, + pat.span, + "matching over `()` is more explicit", + "use `()` instead of `_`", + String::from("()"), + Applicability::MachineApplicable, + ); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9d6096ccb..358004cf4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -147,6 +147,7 @@ mod future_not_send; mod if_let_mutex; mod if_not_else; mod if_then_some_else_none; +mod ignored_unit_patterns; mod implicit_hasher; mod implicit_return; mod implicit_saturating_add; @@ -1093,6 +1094,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: }) }); store.register_late_pass(|_| Box::new(redundant_locals::RedundantLocals)); + store.register_late_pass(|_| Box::new(ignored_unit_patterns::IgnoredUnitPatterns)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index 40da002f4..a7a7f4fd8 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -155,7 +155,7 @@ fn try_get_option_occurrence<'tcx>( }); if let ExprKind::Path(QPath::Resolved(None, Path { res: Res::Local(local_id), .. })) = e.kind { match some_captures.get(local_id) - .or_else(|| (method_sugg == "map_or_else").then_some(()).and_then(|_| none_captures.get(local_id))) + .or_else(|| (method_sugg == "map_or_else").then_some(()).and_then(|()| none_captures.get(local_id))) { Some(CaptureKind::Value | CaptureKind::Ref(Mutability::Mut)) => return None, Some(CaptureKind::Ref(Mutability::Not)) if as_mut => return None, diff --git a/tests/ui/ignored_unit_patterns.fixed b/tests/ui/ignored_unit_patterns.fixed new file mode 100644 index 000000000..492219fe4 --- /dev/null +++ b/tests/ui/ignored_unit_patterns.fixed @@ -0,0 +1,17 @@ +//@run-rustfix + +#![warn(clippy::ignored_unit_patterns)] +#![allow(clippy::redundant_pattern_matching, clippy::single_match)] + +fn foo() -> Result<(), ()> { + unimplemented!() +} + +fn main() { + match foo() { + Ok(()) => {}, + Err(()) => {}, + } + if let Ok(()) = foo() {} + let _ = foo().map_err(|()| todo!()); +} diff --git a/tests/ui/ignored_unit_patterns.rs b/tests/ui/ignored_unit_patterns.rs new file mode 100644 index 000000000..90af36f8e --- /dev/null +++ b/tests/ui/ignored_unit_patterns.rs @@ -0,0 +1,17 @@ +//@run-rustfix + +#![warn(clippy::ignored_unit_patterns)] +#![allow(clippy::redundant_pattern_matching, clippy::single_match)] + +fn foo() -> Result<(), ()> { + unimplemented!() +} + +fn main() { + match foo() { + Ok(_) => {}, + Err(_) => {}, + } + if let Ok(_) = foo() {} + let _ = foo().map_err(|_| todo!()); +} diff --git a/tests/ui/ignored_unit_patterns.stderr b/tests/ui/ignored_unit_patterns.stderr new file mode 100644 index 000000000..8feea3cc2 --- /dev/null +++ b/tests/ui/ignored_unit_patterns.stderr @@ -0,0 +1,28 @@ +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:12:12 + | +LL | Ok(_) => {}, + | ^ help: use `()` instead of `_`: `()` + | + = note: `-D clippy::ignored-unit-patterns` implied by `-D warnings` + +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:13:13 + | +LL | Err(_) => {}, + | ^ help: use `()` instead of `_`: `()` + +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:15:15 + | +LL | if let Ok(_) = foo() {} + | ^ help: use `()` instead of `_`: `()` + +error: matching over `()` is more explicit + --> $DIR/ignored_unit_patterns.rs:16:28 + | +LL | let _ = foo().map_err(|_| todo!()); + | ^ help: use `()` instead of `_`: `()` + +error: aborting due to 4 previous errors +