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

New lint ignored_unit_patterns #11242

Merged
merged 1 commit into from
Aug 3, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clippy_dev/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ fn main() {
matches.get_one::<String>("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}"),
}
},
Expand Down
2 changes: 1 addition & 1 deletion clippy_dev/src/setup/vscode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
52 changes: 52 additions & 0 deletions clippy_lints/src/ignored_unit_patterns.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
}
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/option_if_let_else.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/ignored_unit_patterns.fixed
Original file line number Diff line number Diff line change
@@ -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!());
}
17 changes: 17 additions & 0 deletions tests/ui/ignored_unit_patterns.rs
Original file line number Diff line number Diff line change
@@ -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!());
}
28 changes: 28 additions & 0 deletions tests/ui/ignored_unit_patterns.stderr
Original file line number Diff line number Diff line change
@@ -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