Auto merge of #7875 - mbStavola:unit-hash, r=xFrednet

Add unit-hash lint

changelog: [`unit_hash`] Add lint for hashing unit values

This will lint for situations where the end user is attempting to hash a unit value (`()`), as the implementation in `std` simply [does nothing][impl]. Closes #7159 .

Example:

```rust
().hash(&mut state);

// Should (probably) be replaced with:
0_u8.hash(&mut state);
```

[impl]: https://github.com/rust-lang/rust/blob/a5f164faad4a2fed606b8160fd7ecd2d5cbba381/library/core/src/hash/mod.rs#L656

Author: bors

CHANGELOG.md

@@ -3194,6 +3194,7 @@ Released 2018-09-13
 [`uninit_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#uninit_vec
 [`unit_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_arg
 [`unit_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_cmp
+[`unit_hash`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_hash
 [`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord
 [`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
 [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map

clippy_lints/src/lib.register_all.rs

@@ -283,6 +283,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
     LintId::of(unicode::INVISIBLE_CHARACTERS),
     LintId::of(uninit_vec::UNINIT_VEC),
+    LintId::of(unit_hash::UNIT_HASH),
     LintId::of(unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
     LintId::of(unit_types::UNIT_ARG),
     LintId::of(unit_types::UNIT_CMP),

clippy_lints/src/lib.register_correctness.rs

@@ -64,6 +64,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
     LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
     LintId::of(unicode::INVISIBLE_CHARACTERS),
     LintId::of(uninit_vec::UNINIT_VEC),
+    LintId::of(unit_hash::UNIT_HASH),
     LintId::of(unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD),
     LintId::of(unit_types::UNIT_CMP),
     LintId::of(unnamed_address::FN_ADDRESS_COMPARISONS),

clippy_lints/src/lib.register_lints.rs

@@ -477,6 +477,7 @@ store.register_lints(&[
     unicode::NON_ASCII_LITERAL,
     unicode::UNICODE_NOT_NFC,
     uninit_vec::UNINIT_VEC,
+    unit_hash::UNIT_HASH,
     unit_return_expecting_ord::UNIT_RETURN_EXPECTING_ORD,
     unit_types::LET_UNIT_VALUE,
     unit_types::UNIT_ARG,

clippy_lints/src/lib.rs

@@ -364,6 +364,7 @@ mod undocumented_unsafe_blocks;
 mod undropped_manually_drops;
 mod unicode;
 mod uninit_vec;
+mod unit_hash;
 mod unit_return_expecting_ord;
 mod unit_types;
 mod unnamed_address;
@@ -522,6 +523,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(|| Box::new(collapsible_match::CollapsibleMatch));
     store.register_late_pass(|| Box::new(unicode::Unicode));
     store.register_late_pass(|| Box::new(uninit_vec::UninitVec));
+    store.register_late_pass(|| Box::new(unit_hash::UnitHash));
     store.register_late_pass(|| Box::new(unit_return_expecting_ord::UnitReturnExpectingOrd));
     store.register_late_pass(|| Box::new(strings::StringAdd));
     store.register_late_pass(|| Box::new(implicit_return::ImplicitReturn));

clippy_lints/src/unit_hash.rs

@@ -0,0 +1,77 @@
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Detects `().hash(_)`.
+    ///
+    /// ### Why is this bad?
+    /// Hashing a unit value doesn't do anything as the implementation of `Hash` for `()` is a no-op.
+    ///
+    /// ### Example
+    /// ```rust
+    /// # use std::hash::Hash;
+    /// # use std::collections::hash_map::DefaultHasher;
+    /// # enum Foo { Empty, WithValue(u8) }
+    /// # use Foo::*;
+    /// # let mut state = DefaultHasher::new();
+    /// # let my_enum = Foo::Empty;
+    /// match my_enum {
+    /// 	Empty => ().hash(&mut state),
+    /// 	WithValue(x) => x.hash(&mut state),
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// # use std::hash::Hash;
+    /// # use std::collections::hash_map::DefaultHasher;
+    /// # enum Foo { Empty, WithValue(u8) }
+    /// # use Foo::*;
+    /// # let mut state = DefaultHasher::new();
+    /// # let my_enum = Foo::Empty;
+    /// match my_enum {
+    /// 	Empty => 0_u8.hash(&mut state),
+    /// 	WithValue(x) => x.hash(&mut state),
+    /// }
+    /// ```
+    pub UNIT_HASH,
+    correctness,
+    "hashing a unit value, which does nothing"
+}
+declare_lint_pass!(UnitHash => [UNIT_HASH]);
+
+impl LateLintPass<'tcx> for UnitHash {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if_chain! {
+            if let ExprKind::MethodCall(name_ident, _, args, _) = &expr.kind;
+            if name_ident.ident.name == sym::hash;
+            if let [recv, state_param] = args;
+            if cx.typeck_results().expr_ty(recv).is_unit();
+            then {
+                span_lint_and_then(
+                    cx,
+                    UNIT_HASH,
+                    expr.span,
+                    "this call to `hash` on the unit type will do nothing",
+                    |diag| {
+                        diag.span_suggestion(
+                            expr.span,
+                            "remove the call to `hash` or consider using",
+                            format!(
+                                "0_u8.hash({})",
+                                snippet(cx, state_param.span, ".."),
+                            ),
+                            Applicability::MaybeIncorrect,
+                        );
+                        diag.note("the implementation of `Hash` for `()` is a no-op");
+                    }
+                );
+            }
+        }
+    }
+}

tests/ui/unit_hash.rs

@@ -0,0 +1,27 @@
+#![warn(clippy::unit_hash)]
+
+use std::collections::hash_map::DefaultHasher;
+use std::hash::Hash;
+
+enum Foo {
+    Empty,
+    WithValue(u8),
+}
+
+fn do_nothing() {}
+
+fn main() {
+    let mut state = DefaultHasher::new();
+    let my_enum = Foo::Empty;
+
+    match my_enum {
+        Foo::Empty => ().hash(&mut state),
+        Foo::WithValue(x) => x.hash(&mut state),
+    }
+
+    let res = ();
+    res.hash(&mut state);
+
+    #[allow(clippy::unit_arg)]
+    do_nothing().hash(&mut state);
+}

tests/ui/unit_hash.stderr

@@ -0,0 +1,27 @@
+error: this call to `hash` on the unit type will do nothing
+  --> $DIR/unit_hash.rs:18:23
+   |
+LL |         Foo::Empty => ().hash(&mut state),
+   |                       ^^^^^^^^^^^^^^^^^^^ help: remove the call to `hash` or consider using: `0_u8.hash(&mut state)`
+   |
+   = note: `-D clippy::unit-hash` implied by `-D warnings`
+   = note: the implementation of `Hash` for `()` is a no-op
+
+error: this call to `hash` on the unit type will do nothing
+  --> $DIR/unit_hash.rs:23:5
+   |
+LL |     res.hash(&mut state);
+   |     ^^^^^^^^^^^^^^^^^^^^ help: remove the call to `hash` or consider using: `0_u8.hash(&mut state)`
+   |
+   = note: the implementation of `Hash` for `()` is a no-op
+
+error: this call to `hash` on the unit type will do nothing
+  --> $DIR/unit_hash.rs:26:5
+   |
+LL |     do_nothing().hash(&mut state);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove the call to `hash` or consider using: `0_u8.hash(&mut state)`
+   |
+   = note: the implementation of `Hash` for `()` is a no-op
+
+error: aborting due to 3 previous errors
+