From 36d4a4ea82b235e65d5ec32bf8f7a81438cf0e10 Mon Sep 17 00:00:00 2001 From: Matt Stavola Date: Sun, 24 Oct 2021 21:14:20 -0700 Subject: [PATCH] Add unit-hash lint --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_correctness.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/unit_hash.rs | 77 ++++++++++++++++++++ tests/ui/unit_hash.rs | 27 +++++++ tests/ui/unit_hash.stderr | 27 +++++++ 8 files changed, 137 insertions(+) create mode 100644 clippy_lints/src/unit_hash.rs create mode 100644 tests/ui/unit_hash.rs create mode 100644 tests/ui/unit_hash.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b4c687209e..c1dba75a4c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3046,6 +3046,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 diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index c949ee23ecc..1dd70ce7c15 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -282,6 +282,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), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index ff56a6081fb..4217fd3a3ea 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/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), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index e8dd3708c8e..d293bb6dfc2 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -476,6 +476,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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ed7e8277023..a167c65263b 100644 --- a/clippy_lints/src/lib.rs +++ b/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)); diff --git a/clippy_lints/src/unit_hash.rs b/clippy_lints/src/unit_hash.rs new file mode 100644 index 00000000000..a3a3f2d41c7 --- /dev/null +++ b/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"); + } + ); + } + } + } +} diff --git a/tests/ui/unit_hash.rs b/tests/ui/unit_hash.rs new file mode 100644 index 00000000000..989916c239b --- /dev/null +++ b/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); +} diff --git a/tests/ui/unit_hash.stderr b/tests/ui/unit_hash.stderr new file mode 100644 index 00000000000..da276296e02 --- /dev/null +++ b/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 +