Skip to content

Commit 375c650

Browse files
fix: unnecessary_to_owned FP when map key is a reference (#14834)
Closes #14833 changelog: [`unnecessary_to_owned`] fix FP when map key is a reference
2 parents b03370b + 492c4e1 commit 375c650

File tree

3 files changed

+28
-6
lines changed

3 files changed

+28
-6
lines changed

clippy_lints/src/methods/unnecessary_to_owned.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -655,11 +655,18 @@ fn is_to_string_on_string_like<'a>(
655655
}
656656
}
657657

658-
fn is_a_std_map_type(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
659-
is_type_diagnostic_item(cx, ty, sym::HashSet)
660-
|| is_type_diagnostic_item(cx, ty, sym::HashMap)
661-
|| is_type_diagnostic_item(cx, ty, sym::BTreeMap)
662-
|| is_type_diagnostic_item(cx, ty, sym::BTreeSet)
658+
fn std_map_key<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'tcx>> {
659+
match ty.kind() {
660+
ty::Adt(adt, args)
661+
if matches!(
662+
cx.tcx.get_diagnostic_name(adt.did()),
663+
Some(sym::BTreeMap | sym::BTreeSet | sym::HashMap | sym::HashSet)
664+
) =>
665+
{
666+
Some(args.type_at(0))
667+
},
668+
_ => None,
669+
}
663670
}
664671

665672
fn is_str_and_string(cx: &LateContext<'_>, arg_ty: Ty<'_>, original_arg_ty: Ty<'_>) -> bool {
@@ -721,6 +728,7 @@ fn check_if_applicable_to_argument<'tcx>(cx: &LateContext<'tcx>, arg: &Expr<'tcx
721728
// 1. This is a method with only one argument that doesn't come from a trait.
722729
// 2. That it has `Borrow` in its generic predicates.
723730
// 3. `Self` is a std "map type" (ie `HashSet`, `HashMap`, `BTreeSet`, `BTreeMap`).
731+
// 4. The key to the "map type" is not a reference.
724732
fn check_borrow_predicate<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
725733
if let ExprKind::MethodCall(_, caller, &[arg], _) = expr.kind
726734
&& let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
@@ -738,7 +746,9 @@ fn check_borrow_predicate<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>) {
738746
})
739747
&& let caller_ty = cx.typeck_results().expr_ty(caller)
740748
// For now we limit it to "map types".
741-
&& is_a_std_map_type(cx, caller_ty)
749+
&& let Some(key_ty) = std_map_key(cx, caller_ty)
750+
// We need to check that the key type is not a reference.
751+
&& !key_ty.is_ref()
742752
{
743753
check_if_applicable_to_argument(cx, &arg);
744754
}

tests/ui/unnecessary_to_owned.fixed

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,3 +675,9 @@ mod issue_14242 {
675675
rc_slice_provider().to_vec().into_iter()
676676
}
677677
}
678+
679+
fn issue14833() {
680+
use std::collections::HashSet;
681+
let mut s = HashSet::<&String>::new();
682+
s.remove(&"hello".to_owned());
683+
}

tests/ui/unnecessary_to_owned.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,3 +675,9 @@ mod issue_14242 {
675675
rc_slice_provider().to_vec().into_iter()
676676
}
677677
}
678+
679+
fn issue14833() {
680+
use std::collections::HashSet;
681+
let mut s = HashSet::<&String>::new();
682+
s.remove(&"hello".to_owned());
683+
}

0 commit comments

Comments
 (0)