Auto merge of rust-lang#11242 - samueltardieu:issue-11238, r=Centri3,giraffate

New lint `ignored_unit_patterns`

This idea comes from rust-lang#11238. I've put the lint in `pedantic` as it might trigger numerous positives (three in Clippy itself).

changelog: [`ignored_unit_patterns`]: new lint

Author: bors

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

clippy_dev/src/main.rs

@@ -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}"),
             }
         },

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) => {

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,

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,
+            );
+        }
+    }
+}

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`
 }
 

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,

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!());
+}

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!());
+}

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
+