Skip to content

Commit 79f10cf

Browse files
borsbors
committed
Auto merge of rust-lang#12122 - andrewbanchich:tostring-impl, r=llogiq
add to_string_trait_impl lint closes rust-lang#12076 changelog: [`to_string_trait_impl`]: add lint for direct `ToString` implementations
2 parents 855aa08 + 6d76d14 commit 79f10cf

18 files changed

+265
-133
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5623,6 +5623,7 @@ Released 2018-09-13
56235623
[`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some
56245624
[`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display
56255625
[`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args
5626+
[`to_string_trait_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_trait_impl
56265627
[`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo
56275628
[`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
56285629
[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines

‎clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
657657
crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
658658
crate::thread_local_initializer_can_be_made_const::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST_INFO,
659659
crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
660+
crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO,
660661
crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO,
661662
crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO,
662663
crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO,

‎clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ mod temporary_assignment;
327327
mod tests_outside_test_module;
328328
mod thread_local_initializer_can_be_made_const;
329329
mod to_digit_is_some;
330+
mod to_string_trait_impl;
330331
mod trailing_empty_array;
331332
mod trait_bounds;
332333
mod transmute;
@@ -1103,6 +1104,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
11031104
Box::new(thread_local_initializer_can_be_made_const::ThreadLocalInitializerCanBeMadeConst::new(msrv()))
11041105
});
11051106
store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv())));
1107+
store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl));
11061108
// add lints here, do not remove this comment, it's used in `new_lint`
11071109
}
11081110

‎clippy_lints/src/returns.rs

+8-7
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ use rustc_session::declare_lint_pass;
1818
use rustc_span::def_id::LocalDefId;
1919
use rustc_span::{BytePos, Pos, Span};
2020
use std::borrow::Cow;
21+
use std::fmt::Display;
2122

2223
declare_clippy_lint! {
2324
/// ### What it does
@@ -146,14 +147,14 @@ impl<'tcx> RetReplacement<'tcx> {
146147
}
147148
}
148149

149-
impl<'tcx> ToString for RetReplacement<'tcx> {
150-
fn to_string(&self) -> String {
150+
impl<'tcx> Display for RetReplacement<'tcx> {
151+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
151152
match self {
152-
Self::Empty => String::new(),
153-
Self::Block => "{}".to_string(),
154-
Self::Unit => "()".to_string(),
155-
Self::IfSequence(inner, _) => format!("({inner})"),
156-
Self::Expr(inner, _) => inner.to_string(),
153+
Self::Empty => write!(f, ""),
154+
Self::Block => write!(f, "{{}}"),
155+
Self::Unit => write!(f, "()"),
156+
Self::IfSequence(inner, _) => write!(f, "({inner})"),
157+
Self::Expr(inner, _) => write!(f, "{inner}"),
157158
}
158159
}
159160
}

‎clippy_lints/src/to_string_trait_impl.rs

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
use clippy_utils::diagnostics::span_lint_and_help;
2+
use rustc_hir::{Impl, Item, ItemKind};
3+
use rustc_lint::{LateContext, LateLintPass};
4+
use rustc_session::declare_lint_pass;
5+
use rustc_span::sym;
6+
7+
declare_clippy_lint! {
8+
/// ### What it does
9+
/// Checks for direct implementations of `ToString`.
10+
/// ### Why is this bad?
11+
/// This trait is automatically implemented for any type which implements the `Display` trait.
12+
/// As such, `ToString` shouldn’t be implemented directly: `Display` should be implemented instead,
13+
/// and you get the `ToString` implementation for free.
14+
/// ### Example
15+
/// ```no_run
16+
/// struct Point {
17+
/// x: usize,
18+
/// y: usize,
19+
/// }
20+
///
21+
/// impl ToString for Point {
22+
/// fn to_string(&self) -> String {
23+
/// format!("({}, {})", self.x, self.y)
24+
/// }
25+
/// }
26+
/// ```
27+
/// Use instead:
28+
/// ```no_run
29+
/// struct Point {
30+
/// x: usize,
31+
/// y: usize,
32+
/// }
33+
///
34+
/// impl std::fmt::Display for Point {
35+
/// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
36+
/// write!(f, "({}, {})", self.x, self.y)
37+
/// }
38+
/// }
39+
/// ```
40+
#[clippy::version = "1.77.0"]
41+
pub TO_STRING_TRAIT_IMPL,
42+
style,
43+
"check for direct implementations of `ToString`"
44+
}
45+
46+
declare_lint_pass!(ToStringTraitImpl => [TO_STRING_TRAIT_IMPL]);
47+
48+
impl<'tcx> LateLintPass<'tcx> for ToStringTraitImpl {
49+
fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx Item<'tcx>) {
50+
if let ItemKind::Impl(Impl {
51+
of_trait: Some(trait_ref),
52+
..
53+
}) = it.kind
54+
&& let Some(trait_did) = trait_ref.trait_def_id()
55+
&& cx.tcx.is_diagnostic_item(sym::ToString, trait_did)
56+
{
57+
span_lint_and_help(
58+
cx,
59+
TO_STRING_TRAIT_IMPL,
60+
it.span,
61+
"direct implementation of `ToString`",
62+
None,
63+
"prefer implementing `Display` instead",
64+
);
65+
}
66+
}
67+
}

‎tests/ui/format_args.fixed

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::panic::Location;
1414

1515
struct Somewhere;
1616

17+
#[allow(clippy::to_string_trait_impl)]
1718
impl ToString for Somewhere {
1819
fn to_string(&self) -> String {
1920
String::from("somewhere")

‎tests/ui/format_args.rs

+1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ use std::panic::Location;
1414

1515
struct Somewhere;
1616

17+
#[allow(clippy::to_string_trait_impl)]
1718
impl ToString for Somewhere {
1819
fn to_string(&self) -> String {
1920
String::from("somewhere")

‎tests/ui/format_args.stderr

+25-25
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: `to_string` applied to a type that implements `Display` in `format!` args
2-
--> $DIR/format_args.rs:76:72
2+
--> $DIR/format_args.rs:77:72
33
|
44
LL | let _ = format!("error: something failed at {}", Location::caller().to_string());
55
| ^^^^^^^^^^^^ help: remove this
@@ -8,145 +8,145 @@ LL | let _ = format!("error: something failed at {}", Location::caller().to_
88
= help: to override `-D warnings` add `#[allow(clippy::to_string_in_format_args)]`
99

1010
error: `to_string` applied to a type that implements `Display` in `write!` args
11-
--> $DIR/format_args.rs:80:27
11+
--> $DIR/format_args.rs:81:27
1212
|
1313
LL | Location::caller().to_string()
1414
| ^^^^^^^^^^^^ help: remove this
1515

1616
error: `to_string` applied to a type that implements `Display` in `writeln!` args
17-
--> $DIR/format_args.rs:85:27
17+
--> $DIR/format_args.rs:86:27
1818
|
1919
LL | Location::caller().to_string()
2020
| ^^^^^^^^^^^^ help: remove this
2121

2222
error: `to_string` applied to a type that implements `Display` in `print!` args
23-
--> $DIR/format_args.rs:87:63
23+
--> $DIR/format_args.rs:88:63
2424
|
2525
LL | print!("error: something failed at {}", Location::caller().to_string());
2626
| ^^^^^^^^^^^^ help: remove this
2727

2828
error: `to_string` applied to a type that implements `Display` in `println!` args
29-
--> $DIR/format_args.rs:88:65
29+
--> $DIR/format_args.rs:89:65
3030
|
3131
LL | println!("error: something failed at {}", Location::caller().to_string());
3232
| ^^^^^^^^^^^^ help: remove this
3333

3434
error: `to_string` applied to a type that implements `Display` in `eprint!` args
35-
--> $DIR/format_args.rs:89:64
35+
--> $DIR/format_args.rs:90:64
3636
|
3737
LL | eprint!("error: something failed at {}", Location::caller().to_string());
3838
| ^^^^^^^^^^^^ help: remove this
3939

4040
error: `to_string` applied to a type that implements `Display` in `eprintln!` args
41-
--> $DIR/format_args.rs:90:66
41+
--> $DIR/format_args.rs:91:66
4242
|
4343
LL | eprintln!("error: something failed at {}", Location::caller().to_string());
4444
| ^^^^^^^^^^^^ help: remove this
4545

4646
error: `to_string` applied to a type that implements `Display` in `format_args!` args
47-
--> $DIR/format_args.rs:91:77
47+
--> $DIR/format_args.rs:92:77
4848
|
4949
LL | let _ = format_args!("error: something failed at {}", Location::caller().to_string());
5050
| ^^^^^^^^^^^^ help: remove this
5151

5252
error: `to_string` applied to a type that implements `Display` in `assert!` args
53-
--> $DIR/format_args.rs:92:70
53+
--> $DIR/format_args.rs:93:70
5454
|
5555
LL | assert!(true, "error: something failed at {}", Location::caller().to_string());
5656
| ^^^^^^^^^^^^ help: remove this
5757

5858
error: `to_string` applied to a type that implements `Display` in `assert_eq!` args
59-
--> $DIR/format_args.rs:93:73
59+
--> $DIR/format_args.rs:94:73
6060
|
6161
LL | assert_eq!(0, 0, "error: something failed at {}", Location::caller().to_string());
6262
| ^^^^^^^^^^^^ help: remove this
6363

6464
error: `to_string` applied to a type that implements `Display` in `assert_ne!` args
65-
--> $DIR/format_args.rs:94:73
65+
--> $DIR/format_args.rs:95:73
6666
|
6767
LL | assert_ne!(0, 0, "error: something failed at {}", Location::caller().to_string());
6868
| ^^^^^^^^^^^^ help: remove this
6969

7070
error: `to_string` applied to a type that implements `Display` in `panic!` args
71-
--> $DIR/format_args.rs:95:63
71+
--> $DIR/format_args.rs:96:63
7272
|
7373
LL | panic!("error: something failed at {}", Location::caller().to_string());
7474
| ^^^^^^^^^^^^ help: remove this
7575

7676
error: `to_string` applied to a type that implements `Display` in `println!` args
77-
--> $DIR/format_args.rs:96:20
77+
--> $DIR/format_args.rs:97:20
7878
|
7979
LL | println!("{}", X(1).to_string());
8080
| ^^^^^^^^^^^^^^^^ help: use this: `*X(1)`
8181

8282
error: `to_string` applied to a type that implements `Display` in `println!` args
83-
--> $DIR/format_args.rs:97:20
83+
--> $DIR/format_args.rs:98:20
8484
|
8585
LL | println!("{}", Y(&X(1)).to_string());
8686
| ^^^^^^^^^^^^^^^^^^^^ help: use this: `***Y(&X(1))`
8787

8888
error: `to_string` applied to a type that implements `Display` in `println!` args
89-
--> $DIR/format_args.rs:98:24
89+
--> $DIR/format_args.rs:99:24
9090
|
9191
LL | println!("{}", Z(1).to_string());
9292
| ^^^^^^^^^^^^ help: remove this
9393

9494
error: `to_string` applied to a type that implements `Display` in `println!` args
95-
--> $DIR/format_args.rs:99:20
95+
--> $DIR/format_args.rs:100:20
9696
|
9797
LL | println!("{}", x.to_string());
9898
| ^^^^^^^^^^^^^ help: use this: `**x`
9999

100100
error: `to_string` applied to a type that implements `Display` in `println!` args
101-
--> $DIR/format_args.rs:100:20
101+
--> $DIR/format_args.rs:101:20
102102
|
103103
LL | println!("{}", x_ref.to_string());
104104
| ^^^^^^^^^^^^^^^^^ help: use this: `***x_ref`
105105

106106
error: `to_string` applied to a type that implements `Display` in `println!` args
107-
--> $DIR/format_args.rs:102:39
107+
--> $DIR/format_args.rs:103:39
108108
|
109109
LL | println!("{foo}{bar}", foo = "foo".to_string(), bar = "bar");
110110
| ^^^^^^^^^^^^ help: remove this
111111

112112
error: `to_string` applied to a type that implements `Display` in `println!` args
113-
--> $DIR/format_args.rs:103:52
113+
--> $DIR/format_args.rs:104:52
114114
|
115115
LL | println!("{foo}{bar}", foo = "foo", bar = "bar".to_string());
116116
| ^^^^^^^^^^^^ help: remove this
117117

118118
error: `to_string` applied to a type that implements `Display` in `println!` args
119-
--> $DIR/format_args.rs:104:39
119+
--> $DIR/format_args.rs:105:39
120120
|
121121
LL | println!("{foo}{bar}", bar = "bar".to_string(), foo = "foo");
122122
| ^^^^^^^^^^^^ help: remove this
123123

124124
error: `to_string` applied to a type that implements `Display` in `println!` args
125-
--> $DIR/format_args.rs:105:52
125+
--> $DIR/format_args.rs:106:52
126126
|
127127
LL | println!("{foo}{bar}", bar = "bar", foo = "foo".to_string());
128128
| ^^^^^^^^^^^^ help: remove this
129129

130130
error: `to_string` applied to a type that implements `Display` in `print!` args
131-
--> $DIR/format_args.rs:117:37
131+
--> $DIR/format_args.rs:118:37
132132
|
133133
LL | print!("{}", (Location::caller().to_string()));
134134
| ^^^^^^^^^^^^ help: remove this
135135

136136
error: `to_string` applied to a type that implements `Display` in `print!` args
137-
--> $DIR/format_args.rs:118:39
137+
--> $DIR/format_args.rs:119:39
138138
|
139139
LL | print!("{}", ((Location::caller()).to_string()));
140140
| ^^^^^^^^^^^^ help: remove this
141141

142142
error: `to_string` applied to a type that implements `Display` in `format!` args
143-
--> $DIR/format_args.rs:146:38
143+
--> $DIR/format_args.rs:147:38
144144
|
145145
LL | let x = format!("{} {}", a, b.to_string());
146146
| ^^^^^^^^^^^^ help: remove this
147147

148148
error: `to_string` applied to a type that implements `Display` in `println!` args
149-
--> $DIR/format_args.rs:160:24
149+
--> $DIR/format_args.rs:161:24
150150
|
151151
LL | println!("{}", original[..10].to_string());
152152
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use this: `&original[..10]`

‎tests/ui/to_string_trait_impl.rs

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
#![warn(clippy::to_string_trait_impl)]
2+
3+
use std::fmt::{self, Display};
4+
5+
struct Point {
6+
x: usize,
7+
y: usize,
8+
}
9+
10+
impl ToString for Point {
11+
fn to_string(&self) -> String {
12+
format!("({}, {})", self.x, self.y)
13+
}
14+
}
15+
16+
struct Foo;
17+
18+
impl Display for Foo {
19+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
20+
write!(f, "Foo")
21+
}
22+
}
23+
24+
struct Bar;
25+
26+
impl Bar {
27+
#[allow(clippy::inherent_to_string)]
28+
fn to_string(&self) -> String {
29+
String::from("Bar")
30+
}
31+
}

‎tests/ui/to_string_trait_impl.stderr

+16
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
error: direct implementation of `ToString`
2+
--> $DIR/to_string_trait_impl.rs:10:1
3+
|
4+
LL | / impl ToString for Point {
5+
LL | | fn to_string(&self) -> String {
6+
LL | | format!("({}, {})", self.x, self.y)
7+
LL | | }
8+
LL | | }
9+
| |_^
10+
|
11+
= help: prefer implementing `Display` instead
12+
= note: `-D clippy::to-string-trait-impl` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::to_string_trait_impl)]`
14+
15+
error: aborting due to 1 previous error
16+

‎tests/ui/unconditional_recursion.rs

+3
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ impl PartialEq for S8 {
206206

207207
struct S9;
208208

209+
#[allow(clippy::to_string_trait_impl)]
209210
impl std::string::ToString for S9 {
210211
fn to_string(&self) -> String {
211212
//~^ ERROR: function cannot return without recursing
@@ -215,6 +216,7 @@ impl std::string::ToString for S9 {
215216

216217
struct S10;
217218

219+
#[allow(clippy::to_string_trait_impl)]
218220
impl std::string::ToString for S10 {
219221
fn to_string(&self) -> String {
220222
//~^ ERROR: function cannot return without recursing
@@ -225,6 +227,7 @@ impl std::string::ToString for S10 {
225227

226228
struct S11;
227229

230+
#[allow(clippy::to_string_trait_impl)]
228231
impl std::string::ToString for S11 {
229232
fn to_string(&self) -> String {
230233
//~^ ERROR: function cannot return without recursing

0 commit comments

Comments
 (0)