bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rename lint, generalize function, add known issues, use multispan

Browse source
This commit is contained in:
Renato Lochetti 2024-06-03 19:04:07 +01:00
parent 0f915f6f30
commit 6661e83e7b
No known key found for this signature in database
GPG key ID: 4B78B34B3DE7EBCC
7 changed files with 105 additions and 162 deletions
Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -5401,7 +5401,6 @@ Released 2018-09-13
[`get_first`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_first
[`get_last_with_len`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_last_with_len
[`get_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#get_unwrap
[`hashset_insert_after_contains`]: https://rust-lang.github.io/rust-clippy/master/index.html#hashset_insert_after_contains
[`host_endian_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#host_endian_bytes
[`identity_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_conversion
[`identity_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#identity_op
@ -5799,6 +5798,7 @@ Released 2018-09-13
[`semicolon_outside_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#semicolon_outside_block
[`separated_literal_suffix`]: https://rust-lang.github.io/rust-clippy/master/index.html#separated_literal_suffix
[`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
[`set_contains_or_insert`]: https://rust-lang.github.io/rust-clippy/master/index.html#set_contains_or_insert
[`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse
[`shadow_same`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_same
[`shadow_unrelated`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_unrelated

2
clippy_lints/src/declared_lints.rs
View file

@ -211,7 +211,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::functions::TOO_MANY_ARGUMENTS_INFO,
crate::functions::TOO_MANY_LINES_INFO,
crate::future_not_send::FUTURE_NOT_SEND_INFO,
crate::hashset_insert_after_contains::HASHSET_INSERT_AFTER_CONTAINS_INFO,
crate::if_let_mutex::IF_LET_MUTEX_INFO,
crate::if_not_else::IF_NOT_ELSE_INFO,
crate::if_then_some_else_none::IF_THEN_SOME_ELSE_NONE_INFO,
@ -646,6 +645,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::semicolon_block::SEMICOLON_OUTSIDE_BLOCK_INFO,
crate::semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED_INFO,
crate::serde_api::SERDE_API_MISUSE_INFO,
crate::set_contains_or_insert::SET_CONTAINS_OR_INSERT_INFO,
crate::shadow::SHADOW_REUSE_INFO,
crate::shadow::SHADOW_SAME_INFO,
crate::shadow::SHADOW_UNRELATED_INFO,

4
clippy_lints/src/lib.rs
View file

@ -150,7 +150,6 @@ mod from_raw_with_void_ptr;
mod from_str_radix_10;
mod functions;
mod future_not_send;
mod hashset_insert_after_contains;
mod if_let_mutex;
mod if_not_else;
mod if_then_some_else_none;
@ -319,6 +318,7 @@ mod self_named_constructors;
mod semicolon_block;
mod semicolon_if_nothing_returned;
mod serde_api;
mod set_contains_or_insert;
mod shadow;
mod significant_drop_tightening;
mod single_call_fn;
@ -1173,7 +1173,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
});
store.register_late_pass(move |_| Box::new(string_patterns::StringPatterns::new(msrv())));
store.register_early_pass(|| Box::new(field_scoped_visibility_modifiers::FieldScopedVisibilityModifiers));
store.register_late_pass(|_| Box::new(hashset_insert_after_contains::HashsetInsertAfterContains));
store.register_late_pass(|_| Box::new(set_contains_or_insert::HashsetInsertAfterContains));
// add lints here, do not remove this comment, it's used in `new_lint`
}

78
clippy_lints/src/hashset_insert_after_contains.rs → clippy_lints/src/set_contains_or_insert.rs
View file

@ -1,12 +1,13 @@
use std::ops::ControlFlow;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::span_lint;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{higher, peel_hir_expr_while, SpanlessEq};
use rustc_hir::{Expr, ExprKind, UnOp};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::symbol::Symbol;
use rustc_span::{sym, Span};
declare_clippy_lint! {
@ -17,6 +18,11 @@ declare_clippy_lint! {
/// ### Why is this bad?
/// Using just `insert` and checking the returned `bool` is more efficient.
///
/// ### Known problems
/// In case the value that wants to be inserted is borrowed and also expensive or impossible
/// to clone. In such scenario, the developer might want to check with `contain` before inserting,
/// to avoid the clone. In this case, it will report a false positive.
///
/// ### Example
/// ```rust
/// use std::collections::HashSet;
@ -37,12 +43,12 @@ declare_clippy_lint! {
/// }
/// ```
#[clippy::version = "1.80.0"]
pub HASHSET_INSERT_AFTER_CONTAINS,
pub SET_CONTAINS_OR_INSERT,
nursery,
"unnecessary call to `HashSet::contains` followed by `HashSet::insert`"
"call to `HashSet::contains` followed by `HashSet::insert`"
}
declare_lint_pass!(HashsetInsertAfterContains => [HASHSET_INSERT_AFTER_CONTAINS]);
declare_lint_pass!(HashsetInsertAfterContains => [SET_CONTAINS_OR_INSERT]);
impl<'tcx> LateLintPass<'tcx> for HashsetInsertAfterContains {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
@ -52,29 +58,26 @@ impl<'tcx> LateLintPass<'tcx> for HashsetInsertAfterContains {
then: then_expr,
..
}) = higher::If::hir(expr)
&& let Some(contains_expr) = try_parse_contains(cx, cond_expr)
&& find_insert_calls(cx, &contains_expr, then_expr)
&& let Some(contains_expr) = try_parse_op_call(cx, cond_expr, sym!(contains))//try_parse_contains(cx, cond_expr)
&& let Some(insert_expr) = find_insert_calls(cx, &contains_expr, then_expr)
{
span_lint_and_then(
span_lint(
cx,
HASHSET_INSERT_AFTER_CONTAINS,
expr.span,
SET_CONTAINS_OR_INSERT,
vec![contains_expr.span, insert_expr.span],
"usage of `HashSet::insert` after `HashSet::contains`",
|diag| {
diag.note("`HashSet::insert` returns whether it was inserted")
.span_help(contains_expr.span, "remove the `HashSet::contains` call");
},
);
}
}
}
struct ContainsExpr<'tcx> {
struct OpExpr<'tcx> {
receiver: &'tcx Expr<'tcx>,
value: &'tcx Expr<'tcx>,
span: Span,
}
fn try_parse_contains<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<ContainsExpr<'tcx>> {
fn try_parse_op_call<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, symbol: Symbol) -> Option<OpExpr<'tcx>> {
let expr = peel_hir_expr_while(expr, |e| {
if let ExprKind::Unary(UnOp::Not, e) = e.kind {
Some(e)
@ -82,26 +85,8 @@ fn try_parse_contains<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Optio
None
}
});
if let ExprKind::MethodCall(path, receiver, [value], span) = expr.kind {
let value = value.peel_borrows();
let receiver = receiver.peel_borrows();
let receiver_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
if value.span.eq_ctxt(expr.span)
&& is_type_diagnostic_item(cx, receiver_ty, sym::HashSet)
&& path.ident.name == sym!(contains)
{
return Some(ContainsExpr { receiver, value, span });
}
}
None
}
struct InsertExpr<'tcx> {
receiver: &'tcx Expr<'tcx>,
value: &'tcx Expr<'tcx>,
}
fn try_parse_insert<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<InsertExpr<'tcx>> {
if let ExprKind::MethodCall(path, receiver, [value], _) = expr.kind {
if let ExprKind::MethodCall(path, receiver, [value], span) = expr.kind {
let value = value.peel_borrows();
let value = peel_hir_expr_while(value, |e| {
if let ExprKind::Unary(UnOp::Deref, e) = e.kind {
@ -110,28 +95,31 @@ fn try_parse_insert<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Optio
None
}
});
let receiver = receiver.peel_borrows();
let receiver_ty = cx.typeck_results().expr_ty(receiver).peel_refs();
if is_type_diagnostic_item(cx, receiver_ty, sym::HashSet) && path.ident.name == sym!(insert) {
Some(InsertExpr { receiver, value })
} else {
None
if value.span.eq_ctxt(expr.span)
&& is_type_diagnostic_item(cx, receiver_ty, sym::HashSet)
&& path.ident.name == symbol
{
return Some(OpExpr { receiver, value, span });
}
} else {
None
}
None
}
fn find_insert_calls<'tcx>(cx: &LateContext<'tcx>, contains_expr: &ContainsExpr<'tcx>, expr: &'tcx Expr<'_>) -> bool {
fn find_insert_calls<'tcx>(
cx: &LateContext<'tcx>,
contains_expr: &OpExpr<'tcx>,
expr: &'tcx Expr<'_>,
) -> Option<OpExpr<'tcx>> {
for_each_expr(expr, |e| {
if let Some(insert_expr) = try_parse_insert(cx, e)
if let Some(insert_expr) = try_parse_op_call(cx, e, sym!(insert))
&& SpanlessEq::new(cx).eq_expr(contains_expr.receiver, insert_expr.receiver)
&& SpanlessEq::new(cx).eq_expr(contains_expr.value, insert_expr.value)
{
ControlFlow::Break(())
ControlFlow::Break(insert_expr)
} else {
ControlFlow::Continue(())
}
})
.is_some()
}

112
tests/ui/hashset_insert_after_contains.stderr
View file

@ -1,112 +0,0 @@
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/hashset_insert_after_contains.rs:18:5
|
LL | / if !set.contains(&value) {
LL | | set.insert(value);
LL | | println!("Just a comment");
LL | | }
| |_____^
|
= note: `HashSet::insert` returns whether it was inserted
help: remove the `HashSet::contains` call
--> tests/ui/hashset_insert_after_contains.rs:18:13
|
LL | if !set.contains(&value) {
| ^^^^^^^^^^^^^^^^
= note: `-D clippy::hashset-insert-after-contains` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::hashset_insert_after_contains)]`
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/hashset_insert_after_contains.rs:23:5
|
LL | / if set.contains(&value) {
LL | | set.insert(value);
LL | | println!("Just a comment");
LL | | }
| |_____^
|
= note: `HashSet::insert` returns whether it was inserted
help: remove the `HashSet::contains` call
--> tests/ui/hashset_insert_after_contains.rs:23:12
|
LL | if set.contains(&value) {
| ^^^^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/hashset_insert_after_contains.rs:28:5
|
LL | / if !set.contains(&value) {
LL | | set.insert(value);
LL | | }
| |_____^
|
= note: `HashSet::insert` returns whether it was inserted
help: remove the `HashSet::contains` call
--> tests/ui/hashset_insert_after_contains.rs:28:13
|
LL | if !set.contains(&value) {
| ^^^^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/hashset_insert_after_contains.rs:32:5
|
LL | / if !!set.contains(&value) {
LL | | set.insert(value);
LL | | println!("Just a comment");
LL | | }
| |_____^
|
= note: `HashSet::insert` returns whether it was inserted
help: remove the `HashSet::contains` call
--> tests/ui/hashset_insert_after_contains.rs:32:14
|
LL | if !!set.contains(&value) {
| ^^^^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/hashset_insert_after_contains.rs:37:5
|
LL | / if (&set).contains(&value) {
LL | | set.insert(value);
LL | | }
| |_____^
|
= note: `HashSet::insert` returns whether it was inserted
help: remove the `HashSet::contains` call
--> tests/ui/hashset_insert_after_contains.rs:37:15
|
LL | if (&set).contains(&value) {
| ^^^^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/hashset_insert_after_contains.rs:42:5
|
LL | / if !set.contains(borrow_value) {
LL | | set.insert(*borrow_value);
LL | | }
| |_____^
|
= note: `HashSet::insert` returns whether it was inserted
help: remove the `HashSet::contains` call
--> tests/ui/hashset_insert_after_contains.rs:42:13
|
LL | if !set.contains(borrow_value) {
| ^^^^^^^^^^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/hashset_insert_after_contains.rs:47:5
|
LL | / if !borrow_set.contains(&value) {
LL | | borrow_set.insert(value);
LL | | }
| |_____^
|
= note: `HashSet::insert` returns whether it was inserted
help: remove the `HashSet::contains` call
--> tests/ui/hashset_insert_after_contains.rs:47:20
|
LL | if !borrow_set.contains(&value) {
| ^^^^^^^^^^^^^^^^
error: aborting due to 7 previous errors

8
tests/ui/hashset_insert_after_contains.rs → tests/ui/set_contains_or_insert.rs
View file

@ -1,7 +1,7 @@
#![allow(unused)]
#![allow(clippy::nonminimal_bool)]
#![allow(clippy::needless_borrow)]
#![warn(clippy::hashset_insert_after_contains)]
#![warn(clippy::set_contains_or_insert)]
use std::collections::HashSet;
@ -70,6 +70,12 @@ fn should_not_warn_cases() {
set.replace(value); //it is not insert
println!("Just a comment");
}
if set.contains(&value) {
println!("value is already in set");
} else {
set.insert(value);
}
}
fn simply_true() -> bool {

61
tests/ui/set_contains_or_insert.stderr Normal file
View file

@ -0,0 +1,61 @@
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/set_contains_or_insert.rs:18:13
|
LL | if !set.contains(&value) {
| ^^^^^^^^^^^^^^^^
LL | set.insert(value);
| ^^^^^^^^^^^^^
|
= note: `-D clippy::set-contains-or-insert` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::set_contains_or_insert)]`
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/set_contains_or_insert.rs:23:12
|
LL | if set.contains(&value) {
| ^^^^^^^^^^^^^^^^
LL | set.insert(value);
| ^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/set_contains_or_insert.rs:28:13
|
LL | if !set.contains(&value) {
| ^^^^^^^^^^^^^^^^
LL | set.insert(value);
| ^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/set_contains_or_insert.rs:32:14
|
LL | if !!set.contains(&value) {
| ^^^^^^^^^^^^^^^^
LL | set.insert(value);
| ^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/set_contains_or_insert.rs:37:15
|
LL | if (&set).contains(&value) {
| ^^^^^^^^^^^^^^^^
LL | set.insert(value);
| ^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/set_contains_or_insert.rs:42:13
|
LL | if !set.contains(borrow_value) {
| ^^^^^^^^^^^^^^^^^^^^^^
LL | set.insert(*borrow_value);
| ^^^^^^^^^^^^^^^^^^^^^
error: usage of `HashSet::insert` after `HashSet::contains`
--> tests/ui/set_contains_or_insert.rs:47:20
|
LL | if !borrow_set.contains(&value) {
| ^^^^^^^^^^^^^^^^
LL | borrow_set.insert(value);
| ^^^^^^^^^^^^^
error: aborting due to 7 previous errors