From 954aab394d272ce8c0724a2999088a0f0e7939d8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 19 Feb 2024 16:42:10 +0100 Subject: [PATCH 1/2] Add new `useless_allocation` lint --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/useless_allocation.rs | 119 +++++++++++++++++++++++++ 4 files changed, 123 insertions(+) create mode 100644 clippy_lints/src/useless_allocation.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index b35475c7340e..927943ff8049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5777,6 +5777,7 @@ Released 2018-09-13 [`use_debug`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_debug [`use_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#use_self [`used_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#used_underscore_binding +[`useless_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_allocation [`useless_asref`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_asref [`useless_attribute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_attribute [`useless_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 60744fee34d2..87c122dfd168 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -726,6 +726,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::unwrap_in_result::UNWRAP_IN_RESULT_INFO, crate::upper_case_acronyms::UPPER_CASE_ACRONYMS_INFO, crate::use_self::USE_SELF_INFO, + crate::useless_allocation::USELESS_ALLOCATION_INFO, crate::useless_conversion::USELESS_CONVERSION_INFO, crate::vec::USELESS_VEC_INFO, crate::vec_init_then_push::VEC_INIT_THEN_PUSH_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5636f46b22fe..3eb27cbfb703 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -359,6 +359,7 @@ mod unwrap; mod unwrap_in_result; mod upper_case_acronyms; mod use_self; +mod useless_allocation; mod useless_conversion; mod vec; mod vec_init_then_push; @@ -1111,6 +1112,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { }); store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv()))); store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl)); + store.register_late_pass(|_| Box::new(useless_allocation::UselessAllocation)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/useless_allocation.rs b/clippy_lints/src/useless_allocation.rs new file mode 100644 index 000000000000..3a13d9d22786 --- /dev/null +++ b/clippy_lints/src/useless_allocation.rs @@ -0,0 +1,119 @@ +use rustc_errors::Applicability; +use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, ClauseKind, ImplPolarity, Mutability, Ty}; +use rustc_session::declare_lint_pass; +use rustc_span::sym; + +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_diag_trait_item; +use clippy_utils::source::snippet_opt; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; + +declare_clippy_lint! { + /// ### What it does + /// Checks if an unneeded allocation is performed when trying to get information + /// related to a given key is a `HashMap`-like type. + /// + /// ### Why is this bad? + /// Using less resources is generally a good idea. + /// + /// ### Example + /// ```no_run + /// let mut s = HashSet::from(["a".to_string()]); + /// s.remove(&"b".to_owned()); + /// ``` + /// Use instead: + /// ```no_run + /// let mut s = HashSet::from(["a".to_string()]); + /// s.remove("b"); + /// ``` + #[clippy::version = "1.78.0"] + pub USELESS_ALLOCATION, + suspicious, + "default lint description" +} + +declare_lint_pass!(UselessAllocation => [USELESS_ALLOCATION]); + +fn is_a_std_map_type(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { + is_type_diagnostic_item(cx, ty, sym::HashSet) + || is_type_diagnostic_item(cx, ty, sym::HashMap) + || is_type_diagnostic_item(cx, ty, sym::BTreeMap) + || is_type_diagnostic_item(cx, ty, sym::BTreeSet) +} + +fn is_str_and_string(cx: &LateContext<'_>, arg_ty: Ty<'_>, original_arg_ty: Ty<'_>) -> bool { + original_arg_ty.is_str() && is_type_lang_item(cx, arg_ty, LangItem::String) +} + +fn is_slice_and_vec(cx: &LateContext<'_>, arg_ty: Ty<'_>, original_arg_ty: Ty<'_>) -> bool { + (original_arg_ty.is_slice() || original_arg_ty.is_array() || original_arg_ty.is_array_slice()) + && is_type_diagnostic_item(cx, arg_ty, sym::Vec) +} + +fn check_if_applicable_to_argument(cx: &LateContext<'_>, arg: &Expr<'_>) { + if let ExprKind::AddrOf(BorrowKind::Ref, Mutability::Not, expr) = arg.kind + && let ExprKind::MethodCall(method_path, caller, &[], _) = expr.kind + && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + && let method_name = method_path.ident.name.as_str() + && match method_name { + "to_owned" => is_diag_trait_item(cx, method_def_id, sym::ToOwned), + "to_string" => is_diag_trait_item(cx, method_def_id, sym::ToString), + "to_vec" => cx + .tcx + .impl_of_method(method_def_id) + .filter(|&impl_did| { + cx.tcx.type_of(impl_did).instantiate_identity().is_slice() + && cx.tcx.impl_trait_ref(impl_did).is_none() + }) + .is_some(), + _ => false, + } + && let original_arg_ty = cx.typeck_results().node_type(caller.hir_id) + && let arg_ty = cx.typeck_results().expr_ty(arg) + && let ty::Ref(_, arg_ty, Mutability::Not) = arg_ty.kind() + && let arg_ty = arg_ty.peel_refs() + // For now we limit this lint to `String` and `Vec`. + && let is_str = is_str_and_string(cx, arg_ty, original_arg_ty.peel_refs()) + && (is_str || is_slice_and_vec(cx, arg_ty, original_arg_ty)) + && let Some(snippet) = snippet_opt(cx, caller.span) + { + span_lint_and_sugg( + cx, + USELESS_ALLOCATION, + arg.span, + "unneeded allocation", + "replace it with", + if is_str { + snippet + } else { + format!("{}.as_slice()", snippet) + }, + Applicability::MaybeIncorrect, + ); + } +} + +impl<'tcx> LateLintPass<'tcx> for UselessAllocation { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if let ExprKind::MethodCall(_, caller, &[arg], _) = expr.kind + && let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) + && let Some(borrow_id) = cx.tcx.get_diagnostic_item(sym::Borrow) + && cx.tcx.predicates_of(method_def_id).predicates.iter().any(|(pred, _)| { + if let ClauseKind::Trait(trait_pred) = pred.kind().skip_binder() + && trait_pred.polarity == ImplPolarity::Positive + && trait_pred.trait_ref.def_id == borrow_id + { + true + } else { + false + } + }) + && let caller_ty = cx.typeck_results().expr_ty(caller) + && is_a_std_map_type(cx, caller_ty) + { + check_if_applicable_to_argument(cx, &arg); + } + } +} From e3e541e6695b538fac0ddad82fe8475eef553500 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 19 Feb 2024 17:45:00 +0100 Subject: [PATCH 2/2] Add ui test for `useless_allocation` --- tests/ui/useless_allocation.fixed | 18 ++++++++++++++++++ tests/ui/useless_allocation.rs | 18 ++++++++++++++++++ tests/ui/useless_allocation.stderr | 23 +++++++++++++++++++++++ 3 files changed, 59 insertions(+) create mode 100644 tests/ui/useless_allocation.fixed create mode 100644 tests/ui/useless_allocation.rs create mode 100644 tests/ui/useless_allocation.stderr diff --git a/tests/ui/useless_allocation.fixed b/tests/ui/useless_allocation.fixed new file mode 100644 index 000000000000..bafecaad744b --- /dev/null +++ b/tests/ui/useless_allocation.fixed @@ -0,0 +1,18 @@ +#![warn(clippy::useless_allocation)] + +use std::collections::HashSet; + +fn main() { + let mut s = HashSet::from(["a".to_string()]); + s.remove("b"); //~ ERROR: unneeded allocation + s.remove("b"); //~ ERROR: unneeded allocation + // Should not warn. + s.remove("b"); + + let mut s = HashSet::from([vec!["a"]]); + s.remove(["b"].as_slice()); //~ ERROR: unneeded allocation + + // Should not warn. + s.remove(&["b"].to_vec().clone()); + s.remove(["a"].as_slice()); +} diff --git a/tests/ui/useless_allocation.rs b/tests/ui/useless_allocation.rs new file mode 100644 index 000000000000..ace3b2d2c9ce --- /dev/null +++ b/tests/ui/useless_allocation.rs @@ -0,0 +1,18 @@ +#![warn(clippy::useless_allocation)] + +use std::collections::HashSet; + +fn main() { + let mut s = HashSet::from(["a".to_string()]); + s.remove(&"b".to_owned()); //~ ERROR: unneeded allocation + s.remove(&"b".to_string()); //~ ERROR: unneeded allocation + // Should not warn. + s.remove("b"); + + let mut s = HashSet::from([vec!["a"]]); + s.remove(&["b"].to_vec()); //~ ERROR: unneeded allocation + + // Should not warn. + s.remove(&["b"].to_vec().clone()); + s.remove(["a"].as_slice()); +} diff --git a/tests/ui/useless_allocation.stderr b/tests/ui/useless_allocation.stderr new file mode 100644 index 000000000000..3114edc4da4c --- /dev/null +++ b/tests/ui/useless_allocation.stderr @@ -0,0 +1,23 @@ +error: unneeded allocation + --> tests/ui/useless_allocation.rs:7:14 + | +LL | s.remove(&"b".to_owned()); + | ^^^^^^^^^^^^^^^ help: replace it with: `"b"` + | + = note: `-D clippy::useless-allocation` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::useless_allocation)]` + +error: unneeded allocation + --> tests/ui/useless_allocation.rs:8:14 + | +LL | s.remove(&"b".to_string()); + | ^^^^^^^^^^^^^^^^ help: replace it with: `"b"` + +error: unneeded allocation + --> tests/ui/useless_allocation.rs:13:14 + | +LL | s.remove(&["b"].to_vec()); + | ^^^^^^^^^^^^^^^ help: replace it with: `["b"].as_slice()` + +error: aborting due to 3 previous errors +