Skip to content

Commit

Permalink
Add manual_hash_one lint
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexendoo committed Sep 26, 2023
1 parent 585b56f commit ec2f626
Show file tree
Hide file tree
Showing 11 changed files with 418 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ The minimum rust version that the project supports
* [`type_repetition_in_bounds`](https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds)
* [`tuple_array_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions)
* [`manual_try_fold`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_try_fold)
* [`manual_hash_one`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_hash_one)


## `cognitive-complexity-threshold`
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 @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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`
}

Expand Down
133 changes: 133 additions & 0 deletions clippy_lints/src/manual_hash_one.rs
Original file line number Diff line number Diff line change
@@ -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);
}
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
44 changes: 44 additions & 0 deletions clippy_utils/src/visitors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down Expand Up @@ -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
}
89 changes: 89 additions & 0 deletions tests/ui/manual_hash_one.fixed
Original file line number Diff line number Diff line change
@@ -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);
}
Loading

0 comments on commit ec2f626

Please sign in to comment.