Auto merge of #11556 - Alexendoo:manual-hash-one, r=Manishearth

bors · bors · commit 3422282d35df · 2023-09-25T20:34:16.000Z
Add `manual_hash_one` lint Adds a lint to suggest using [`BuildHasher::hash_one`](https://doc.rust-lang.org/std/hash/trait.BuildHasher.html#method.hash_one) changelog: [`manual_hash_one`]: new lint
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5072,6 +5072,7 @@ Released 2018-09-13
 [`manual_find`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find
 [`manual_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_find_map
 [`manual_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_flatten
+[`manual_hash_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one
 [`manual_instant_elapsed`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_instant_elapsed
 [`manual_is_ascii_check`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_ascii_check
 [`manual_is_finite`]: https://rust-lang.github.io/rust-clippy/master/index.html#manual_is_finite
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
@@ -280,6 +280,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::manual_clamp::MANUAL_CLAMP_INFO,
     crate::manual_float_methods::MANUAL_IS_FINITE_INFO,
     crate::manual_float_methods::MANUAL_IS_INFINITE_INFO,
+    crate::manual_hash_one::MANUAL_HASH_ONE_INFO,
     crate::manual_is_ascii_check::MANUAL_IS_ASCII_CHECK_INFO,
     crate::manual_let_else::MANUAL_LET_ELSE_INFO,
     crate::manual_main_separator_str::MANUAL_MAIN_SEPARATOR_STR_INFO,
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -190,6 +190,7 @@ mod manual_async_fn;
 mod manual_bits;
 mod manual_clamp;
 mod manual_float_methods;
+mod manual_hash_one;
 mod manual_is_ascii_check;
 mod manual_let_else;
 mod manual_main_separator_str;
@@ -1119,6 +1120,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
             msrv(),
         ))
     });
+    store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 
diff --git a/clippy_lints/src/manual_hash_one.rs b/clippy_lints/src/manual_hash_one.rs
@@ -0,0 +1,133 @@
+use clippy_utils::diagnostics::span_lint_hir_and_then;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::snippet_opt;
+use clippy_utils::visitors::{is_local_used, local_used_once};
+use clippy_utils::{is_trait_method, path_to_local_id};
+use rustc_errors::Applicability;
+use rustc_hir::{BindingAnnotation, ExprKind, Local, Node, PatKind, StmtKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+use rustc_span::sym;
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for cases where [`BuildHasher::hash_one`] can be used.
+    ///
+    /// [`BuildHasher::hash_one`]: https://doc.rust-lang.org/std/hash/trait.BuildHasher.html#method.hash_one
+    ///
+    /// ### Why is this bad?
+    /// It is more concise to use the `hash_one` method.
+    ///
+    /// ### Example
+    /// ```rust
+    /// use std::hash::{BuildHasher, Hash, Hasher};
+    /// use std::collections::hash_map::RandomState;
+    ///
+    /// let s = RandomState::new();
+    /// let value = vec![1, 2, 3];
+    ///
+    /// let mut hasher = s.build_hasher();
+    /// value.hash(&mut hasher);
+    /// let hash = hasher.finish();
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::hash::BuildHasher;
+    /// use std::collections::hash_map::RandomState;
+    ///
+    /// let s = RandomState::new();
+    /// let value = vec![1, 2, 3];
+    ///
+    /// let hash = s.hash_one(&value);
+    /// ```
+    #[clippy::version = "1.74.0"]
+    pub MANUAL_HASH_ONE,
+    complexity,
+    "manual implementations of `BuildHasher::hash_one`"
+}
+
+pub struct ManualHashOne {
+    msrv: Msrv,
+}
+
+impl ManualHashOne {
+    #[must_use]
+    pub fn new(msrv: Msrv) -> Self {
+        Self { msrv }
+    }
+}
+
+impl_lint_pass!(ManualHashOne => [MANUAL_HASH_ONE]);
+
+impl LateLintPass<'_> for ManualHashOne {
+    fn check_local(&mut self, cx: &LateContext<'_>, local: &Local<'_>) {
+        // `let mut hasher = seg.build_hasher();`
+        if let PatKind::Binding(BindingAnnotation::MUT, hasher, _, None) = local.pat.kind
+            && let Some(init) = local.init
+            && !init.span.from_expansion()
+            && let ExprKind::MethodCall(seg, build_hasher, [], _) = init.kind
+            && seg.ident.name == sym!(build_hasher)
+
+            && let Node::Stmt(local_stmt) = cx.tcx.hir().get_parent(local.hir_id)
+            && let Node::Block(block) = cx.tcx.hir().get_parent(local_stmt.hir_id)
+
+            && let mut stmts = block.stmts.iter()
+                .skip_while(|stmt| stmt.hir_id != local_stmt.hir_id)
+                .skip(1)
+                .filter(|&stmt| is_local_used(cx, stmt, hasher))
+
+            // `hashed_value.hash(&mut hasher);`
+            && let Some(hash_stmt) = stmts.next()
+            && let StmtKind::Semi(hash_expr) = hash_stmt.kind
+            && !hash_expr.span.from_expansion()
+            && let ExprKind::MethodCall(seg, hashed_value, [ref_to_hasher], _) = hash_expr.kind
+            && seg.ident.name == sym::hash
+            && is_trait_method(cx, hash_expr, sym::Hash)
+            && path_to_local_id(ref_to_hasher.peel_borrows(), hasher)
+
+            && let maybe_finish_stmt = stmts.next()
+            // There should be no more statements referencing `hasher`
+            && stmts.next().is_none()
+
+            // `hasher.finish()`, may be anywhere in a statement or the trailing expr of the block
+            && let Some(path_expr) = local_used_once(cx, (maybe_finish_stmt, block.expr), hasher)
+            && let Node::Expr(finish_expr) = cx.tcx.hir().get_parent(path_expr.hir_id)
+            && !finish_expr.span.from_expansion()
+            && let ExprKind::MethodCall(seg, _, [], _) = finish_expr.kind
+            && seg.ident.name == sym!(finish)
+
+            && self.msrv.meets(msrvs::BUILD_HASHER_HASH_ONE)
+        {
+            span_lint_hir_and_then(
+                cx,
+                MANUAL_HASH_ONE,
+                finish_expr.hir_id,
+                finish_expr.span,
+                "manual implementation of `BuildHasher::hash_one`",
+                |diag| {
+                    if let Some(build_hasher) = snippet_opt(cx, build_hasher.span)
+                        && let Some(hashed_value) = snippet_opt(cx, hashed_value.span)
+                    {
+                        diag.multipart_suggestion(
+                            "try",
+                            vec![
+                                (local_stmt.span, String::new()),
+                                (hash_stmt.span, String::new()),
+                                (
+                                    finish_expr.span,
+                                    // `needless_borrows_for_generic_args` will take care of
+                                    // removing the `&` when it isn't needed
+                                    format!("{build_hasher}.hash_one(&{hashed_value})")
+                                )
+                            ],
+                            Applicability::MachineApplicable,
+                        );
+
+                    }
+                },
+            );
+        }
+    }
+
+    extract_msrv_attr!(LateContext);
+}
diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
@@ -294,7 +294,7 @@ define_Conf! {
     ///
     /// Suppress lints whenever the suggested change would cause breakage for other crates.
     (avoid_breaking_exported_api: bool = true),
-    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD.
+    /// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS, MANUAL_TRY_FOLD, MANUAL_HASH_ONE.
     ///
     /// The minimum rust version that the project supports
     (msrv: Option<String> = None),
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
@@ -19,7 +19,7 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
-    1,71,0 { TUPLE_ARRAY_CONVERSIONS }
+    1,71,0 { TUPLE_ARRAY_CONVERSIONS, BUILD_HASHER_HASH_ONE }
     1,70,0 { OPTION_IS_SOME_AND, BINARY_HEAP_RETAIN }
     1,68,0 { PATH_MAIN_SEPARATOR_STR }
     1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs
@@ -62,6 +62,27 @@ where
         }
     }
 }
+impl<'tcx, A, B> Visitable<'tcx> for (A, B)
+where
+    A: Visitable<'tcx>,
+    B: Visitable<'tcx>,
+{
+    fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) {
+        let (a, b) = self;
+        a.visit(visitor);
+        b.visit(visitor);
+    }
+}
+impl<'tcx, T> Visitable<'tcx> for Option<T>
+where
+    T: Visitable<'tcx>,
+{
+    fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) {
+        if let Some(x) = self {
+            x.visit(visitor);
+        }
+    }
+}
 macro_rules! visitable_ref {
     ($t:ident, $f:ident) => {
         impl<'tcx> Visitable<'tcx> for &'tcx $t<'tcx> {
@@ -748,3 +769,26 @@ pub fn contains_break_or_continue(expr: &Expr<'_>) -> bool {
     })
     .is_some()
 }
+
+/// If the local is only used once in `visitable` returns the path expression referencing the given
+/// local
+pub fn local_used_once<'tcx>(
+    cx: &LateContext<'tcx>,
+    visitable: impl Visitable<'tcx>,
+    id: HirId,
+) -> Option<&'tcx Expr<'tcx>> {
+    let mut expr = None;
+
+    let cf = for_each_expr_with_closures(cx, visitable, |e| {
+        if path_to_local_id(e, id) && expr.replace(e).is_some() {
+            ControlFlow::Break(())
+        } else {
+            ControlFlow::Continue(())
+        }
+    });
+    if cf.is_some() {
+        return None;
+    }
+
+    expr
+}
diff --git a/tests/ui/manual_hash_one.fixed b/tests/ui/manual_hash_one.fixed
@@ -0,0 +1,89 @@
+#![warn(clippy::manual_hash_one)]
+#![allow(clippy::needless_borrows_for_generic_args)]
+
+use std::hash::{BuildHasher, Hash, Hasher};
+
+fn returned(b: impl BuildHasher) -> u64 {
+    
+    
+    b.hash_one(&true)
+}
+
+fn unsized_receiver(b: impl BuildHasher, s: &str) {
+    
+    
+    let _ = b.hash_one(&s[4..10]);
+}
+
+fn owned_value(b: impl BuildHasher, v: Vec<u32>) -> Vec<u32> {
+    
+    
+    let _ = b.hash_one(&v);
+    v
+}
+
+fn reused_hasher(b: impl BuildHasher) {
+    let mut hasher = b.build_hasher();
+    true.hash(&mut hasher);
+    let _ = hasher.finish();
+    let _ = hasher.finish();
+}
+
+fn reused_hasher_in_return(b: impl BuildHasher) -> u64 {
+    let mut hasher = b.build_hasher();
+    true.hash(&mut hasher);
+    let _ = hasher.finish();
+    hasher.finish()
+}
+
+fn no_hash(b: impl BuildHasher) {
+    let mut hasher = b.build_hasher();
+    let _ = hasher.finish();
+}
+
+fn hash_twice(b: impl BuildHasher) {
+    let mut hasher = b.build_hasher();
+    true.hash(&mut hasher);
+    true.hash(&mut hasher);
+    let _ = hasher.finish();
+}
+
+fn other_hasher(b: impl BuildHasher) {
+    let mut other_hasher = b.build_hasher();
+
+    let mut hasher = b.build_hasher();
+    true.hash(&mut other_hasher);
+    let _ = hasher.finish();
+}
+
+fn finish_then_hash(b: impl BuildHasher) {
+    let mut hasher = b.build_hasher();
+    let _ = hasher.finish();
+    true.hash(&mut hasher);
+}
+
+fn in_macro(b: impl BuildHasher) {
+    macro_rules! m {
+        ($b:expr) => {{
+            let mut hasher = $b.build_hasher();
+            true.hash(&mut hasher);
+            let _ = hasher.finish();
+        }};
+    }
+
+    m!(b);
+}
+
+#[clippy::msrv = "1.70"]
+fn msrv_1_70(b: impl BuildHasher, v: impl Hash) {
+    let mut hasher = b.build_hasher();
+    v.hash(&mut hasher);
+    let _ = hasher.finish();
+}
+
+#[clippy::msrv = "1.71"]
+fn msrv_1_71(b: impl BuildHasher, v: impl Hash) {
+    
+    
+    let _ = b.hash_one(&v);
+}
diff --git a/tests/ui/manual_hash_one.rs b/tests/ui/manual_hash_one.rs
diff --git a/tests/ui/manual_hash_one.stderr b/tests/ui/manual_hash_one.stderr
