Skip to content

Commit b303ab4

Browse files
fix: handle false negative for str_to_string (rust-lang#16512)
Replace `ToString::to_string` with `ToOwned::to_owned` when it is passed as a function parameter. ```rust fn issue16511(x: Option<&str>) -> Option<String> { // Replace with ToOwned::to_owned x.map(ToString::to_string) } ``` fixes rust-lang#16511 --- changelog: [`str_to_string`]: handle a case when `ToString::to_string` is passed as function parameter
2 parents 1826ec0 + 691e226 commit b303ab4

File tree

4 files changed

+107
-1
lines changed

4 files changed

+107
-1
lines changed

clippy_lints/src/strings.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use clippy_utils::{
55
SpanlessEq, get_expr_use_or_unification_node, get_parent_expr, is_lint_allowed, method_calls, peel_blocks, sym,
66
};
77
use rustc_errors::Applicability;
8+
use rustc_hir::def::{DefKind, Res};
89
use rustc_hir::def_id::DefId;
910
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, LangItem, Node};
1011
use rustc_lint::{LateContext, LateLintPass, LintContext};
@@ -410,6 +411,23 @@ impl<'tcx> LateLintPass<'tcx> for StrToString {
410411
diag.span_suggestion(expr.span, "try", format!("{snippet}.to_owned()"), applicability);
411412
},
412413
);
414+
} else if let ExprKind::Path(_) = expr.kind
415+
&& let Some(parent) = get_parent_expr(cx, expr)
416+
&& let ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args, _) = &parent.kind
417+
&& args.iter().any(|a| a.hir_id == expr.hir_id)
418+
&& let Res::Def(DefKind::AssocFn, def_id) = expr.res(cx)
419+
&& cx.tcx.is_diagnostic_item(sym::to_string_method, def_id)
420+
{
421+
// Detected `ToString::to_string` passed as an argument (generic: any call or method call)
422+
span_lint_and_sugg(
423+
cx,
424+
STR_TO_STRING,
425+
expr.span,
426+
"`ToString::to_string` used as `&str` to `String` converter",
427+
"try",
428+
"ToOwned::to_owned".to_string(),
429+
Applicability::MachineApplicable,
430+
);
413431
}
414432
}
415433
}

tests/ui/str_to_string.fixed

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,32 @@ fn issue16271(key: &[u8]) {
2222
let _value = t!(str::from_utf8(key)).to_owned();
2323
//~^ str_to_string
2424
}
25+
26+
struct GenericWrapper<T>(T);
27+
28+
impl<T> GenericWrapper<T> {
29+
fn mapper<U, F: FnOnce(T) -> U>(self, f: F) -> U {
30+
f(self.0)
31+
}
32+
}
33+
34+
fn issue16511(x: Option<&str>) {
35+
let _ = x.map(ToOwned::to_owned);
36+
//~^ str_to_string
37+
38+
let _ = x.map(ToOwned::to_owned);
39+
//~^ str_to_string
40+
41+
let _ = ["a", "b"].iter().map(ToOwned::to_owned);
42+
//~^ str_to_string
43+
44+
fn mapper<F: Fn(&str) -> String>(f: F) -> String {
45+
f("hello")
46+
}
47+
let _ = mapper(ToOwned::to_owned);
48+
//~^ str_to_string
49+
50+
let w = GenericWrapper("hello");
51+
let _ = w.mapper(ToOwned::to_owned);
52+
//~^ str_to_string
53+
}

tests/ui/str_to_string.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,3 +22,32 @@ fn issue16271(key: &[u8]) {
2222
let _value = t!(str::from_utf8(key)).to_string();
2323
//~^ str_to_string
2424
}
25+
26+
struct GenericWrapper<T>(T);
27+
28+
impl<T> GenericWrapper<T> {
29+
fn mapper<U, F: FnOnce(T) -> U>(self, f: F) -> U {
30+
f(self.0)
31+
}
32+
}
33+
34+
fn issue16511(x: Option<&str>) {
35+
let _ = x.map(ToString::to_string);
36+
//~^ str_to_string
37+
38+
let _ = x.map(str::to_string);
39+
//~^ str_to_string
40+
41+
let _ = ["a", "b"].iter().map(ToString::to_string);
42+
//~^ str_to_string
43+
44+
fn mapper<F: Fn(&str) -> String>(f: F) -> String {
45+
f("hello")
46+
}
47+
let _ = mapper(ToString::to_string);
48+
//~^ str_to_string
49+
50+
let w = GenericWrapper("hello");
51+
let _ = w.mapper(ToString::to_string);
52+
//~^ str_to_string
53+
}

tests/ui/str_to_string.stderr

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,35 @@ error: `to_string()` called on a `&str`
1919
LL | let _value = t!(str::from_utf8(key)).to_string();
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `t!(str::from_utf8(key)).to_owned()`
2121

22-
error: aborting due to 3 previous errors
22+
error: `ToString::to_string` used as `&str` to `String` converter
23+
--> tests/ui/str_to_string.rs:35:19
24+
|
25+
LL | let _ = x.map(ToString::to_string);
26+
| ^^^^^^^^^^^^^^^^^^^ help: try: `ToOwned::to_owned`
27+
28+
error: `ToString::to_string` used as `&str` to `String` converter
29+
--> tests/ui/str_to_string.rs:38:19
30+
|
31+
LL | let _ = x.map(str::to_string);
32+
| ^^^^^^^^^^^^^^ help: try: `ToOwned::to_owned`
33+
34+
error: `ToString::to_string` used as `&str` to `String` converter
35+
--> tests/ui/str_to_string.rs:41:35
36+
|
37+
LL | let _ = ["a", "b"].iter().map(ToString::to_string);
38+
| ^^^^^^^^^^^^^^^^^^^ help: try: `ToOwned::to_owned`
39+
40+
error: `ToString::to_string` used as `&str` to `String` converter
41+
--> tests/ui/str_to_string.rs:47:20
42+
|
43+
LL | let _ = mapper(ToString::to_string);
44+
| ^^^^^^^^^^^^^^^^^^^ help: try: `ToOwned::to_owned`
45+
46+
error: `ToString::to_string` used as `&str` to `String` converter
47+
--> tests/ui/str_to_string.rs:51:22
48+
|
49+
LL | let _ = w.mapper(ToString::to_string);
50+
| ^^^^^^^^^^^^^^^^^^^ help: try: `ToOwned::to_owned`
51+
52+
error: aborting due to 8 previous errors
2353

0 commit comments

Comments
 (0)