mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add manual_hash_one lint

Browse Source
This commit is contained in:
Alex Macleod 2023-09-23 17:34:05 +00:00
parent 585b56f35f
commit ec2f62677f
11 changed files with 418 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -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

1
book/src/lint_configuration.md
View File

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

1
clippy_lints/src/declared_lints.rs
View File

@ -280,6 +280,7 @@
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,

2
clippy_lints/src/lib.rs
View File

@ -190,6 +190,7 @@
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`
}

133
clippy_lints/src/manual_hash_one.rs Normal file
View File

@ -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
clippy_lints/src/utils/conf.rs
View File

@ -294,7 +294,7 @@ pub fn get_configuration_metadata() -> Vec<ClippyConfiguration> {
///
/// 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),

2
clippy_utils/src/msrvs.rs
View File

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

44
clippy_utils/src/visitors.rs
View File

@ -62,6 +62,27 @@ fn visit<V: Visitor<'tcx>>(self, visitor: &mut V) {
}
}
}
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
}

89
tests/ui/manual_hash_one.fixed Normal file
View File

@ -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);
}

89
tests/ui/manual_hash_one.rs Normal file
View File

@ -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 {
let mut hasher = b.build_hasher();
true.hash(&mut hasher);
hasher.finish()
}
fn unsized_receiver(b: impl BuildHasher, s: &str) {
let mut hasher = b.build_hasher();
s[4..10].hash(&mut hasher);
let _ = hasher.finish();
}
fn owned_value(b: impl BuildHasher, v: Vec<u32>) -> Vec<u32> {
let mut hasher = b.build_hasher();
v.hash(&mut hasher);
let _ = hasher.finish();
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 mut hasher = b.build_hasher();
v.hash(&mut hasher);
let _ = hasher.finish();
}

56
tests/ui/manual_hash_one.stderr Normal file
View File

@ -0,0 +1,56 @@
error: manual implementation of `BuildHasher::hash_one`
--> $DIR/manual_hash_one.rs:9:5
|
LL | hasher.finish()
| ^^^^^^^^^^^^^^^
|
= note: `-D clippy::manual-hash-one` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::manual_hash_one)]`
help: try
|
LL ~
LL ~
LL ~ b.hash_one(&true)
|
error: manual implementation of `BuildHasher::hash_one`
--> $DIR/manual_hash_one.rs:15:13
|
LL | let _ = hasher.finish();
| ^^^^^^^^^^^^^^^
|
help: try
|
LL ~
LL ~
LL ~ let _ = b.hash_one(&s[4..10]);
|
error: manual implementation of `BuildHasher::hash_one`
--> $DIR/manual_hash_one.rs:21:13
|
LL | let _ = hasher.finish();
| ^^^^^^^^^^^^^^^
|
help: try
|
LL ~
LL ~
LL ~ let _ = b.hash_one(&v);
|
error: manual implementation of `BuildHasher::hash_one`
--> $DIR/manual_hash_one.rs:88:13
|
LL | let _ = hasher.finish();
| ^^^^^^^^^^^^^^^
|
help: try
|
LL ~
LL ~
LL ~ let _ = b.hash_one(&v);
|
error: aborting due to 4 previous errors