Skip to content

Commit

Permalink
Auto merge of #12122 - andrewbanchich:tostring-impl, r=llogiq
Browse files Browse the repository at this point in the history
add to_string_trait_impl lint

closes #12076

changelog: [`to_string_trait_impl`]: add lint for direct `ToString` implementations
  • Loading branch information
bors committed Jan 27, 2024
2 parents 855aa08 + 6d76d14 commit 79f10cf
Show file tree
Hide file tree
Showing 18 changed files with 265 additions and 133 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5623,6 +5623,7 @@ Released 2018-09-13
[`to_digit_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_digit_is_some
[`to_string_in_display`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_display
[`to_string_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_in_format_args
[`to_string_trait_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#to_string_trait_impl
[`todo`]: https://rust-lang.github.io/rust-clippy/master/index.html#todo
[`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::tests_outside_test_module::TESTS_OUTSIDE_TEST_MODULE_INFO,
crate::thread_local_initializer_can_be_made_const::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST_INFO,
crate::to_digit_is_some::TO_DIGIT_IS_SOME_INFO,
crate::to_string_trait_impl::TO_STRING_TRAIT_IMPL_INFO,
crate::trailing_empty_array::TRAILING_EMPTY_ARRAY_INFO,
crate::trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS_INFO,
crate::trait_bounds::TYPE_REPETITION_IN_BOUNDS_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,7 @@ mod temporary_assignment;
mod tests_outside_test_module;
mod thread_local_initializer_can_be_made_const;
mod to_digit_is_some;
mod to_string_trait_impl;
mod trailing_empty_array;
mod trait_bounds;
mod transmute;
Expand Down Expand Up @@ -1103,6 +1104,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
Box::new(thread_local_initializer_can_be_made_const::ThreadLocalInitializerCanBeMadeConst::new(msrv()))
});
store.register_late_pass(move |_| Box::new(incompatible_msrv::IncompatibleMsrv::new(msrv())));
store.register_late_pass(|_| Box::new(to_string_trait_impl::ToStringTraitImpl));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
15 changes: 8 additions & 7 deletions clippy_lints/src/returns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use rustc_session::declare_lint_pass;
use rustc_span::def_id::LocalDefId;
use rustc_span::{BytePos, Pos, Span};
use std::borrow::Cow;
use std::fmt::Display;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -146,14 +147,14 @@ impl<'tcx> RetReplacement<'tcx> {
}
}

impl<'tcx> ToString for RetReplacement<'tcx> {
fn to_string(&self) -> String {
impl<'tcx> Display for RetReplacement<'tcx> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Empty => String::new(),
Self::Block => "{}".to_string(),
Self::Unit => "()".to_string(),
Self::IfSequence(inner, _) => format!("({inner})"),
Self::Expr(inner, _) => inner.to_string(),
Self::Empty => write!(f, ""),
Self::Block => write!(f, "{{}}"),
Self::Unit => write!(f, "()"),
Self::IfSequence(inner, _) => write!(f, "({inner})"),
Self::Expr(inner, _) => write!(f, "{inner}"),
}
}
}
Expand Down
67 changes: 67 additions & 0 deletions clippy_lints/src/to_string_trait_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
use clippy_utils::diagnostics::span_lint_and_help;
use rustc_hir::{Impl, Item, ItemKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
/// Checks for direct implementations of `ToString`.
/// ### Why is this bad?
/// This trait is automatically implemented for any type which implements the `Display` trait.
/// As such, `ToString` shouldn’t be implemented directly: `Display` should be implemented instead,
/// and you get the `ToString` implementation for free.
/// ### Example
/// ```no_run
/// struct Point {
/// x: usize,
/// y: usize,
/// }
///
/// impl ToString for Point {
/// fn to_string(&self) -> String {
/// format!("({}, {})", self.x, self.y)
/// }
/// }
/// ```
/// Use instead:
/// ```no_run
/// struct Point {
/// x: usize,
/// y: usize,
/// }
///
/// impl std::fmt::Display for Point {
/// fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
/// write!(f, "({}, {})", self.x, self.y)
/// }
/// }
/// ```
#[clippy::version = "1.77.0"]
pub TO_STRING_TRAIT_IMPL,
style,
"check for direct implementations of `ToString`"
}

declare_lint_pass!(ToStringTraitImpl => [TO_STRING_TRAIT_IMPL]);

impl<'tcx> LateLintPass<'tcx> for ToStringTraitImpl {
fn check_item(&mut self, cx: &LateContext<'tcx>, it: &'tcx Item<'tcx>) {
if let ItemKind::Impl(Impl {
of_trait: Some(trait_ref),
..
}) = it.kind
&& let Some(trait_did) = trait_ref.trait_def_id()
&& cx.tcx.is_diagnostic_item(sym::ToString, trait_did)
{
span_lint_and_help(
cx,
TO_STRING_TRAIT_IMPL,
it.span,
"direct implementation of `ToString`",
None,
"prefer implementing `Display` instead",
);
}
}
}
1 change: 1 addition & 0 deletions tests/ui/format_args.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::panic::Location;

struct Somewhere;

#[allow(clippy::to_string_trait_impl)]
impl ToString for Somewhere {
fn to_string(&self) -> String {
String::from("somewhere")
Expand Down
1 change: 1 addition & 0 deletions tests/ui/format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use std::panic::Location;

struct Somewhere;

#[allow(clippy::to_string_trait_impl)]
impl ToString for Somewhere {
fn to_string(&self) -> String {
String::from("somewhere")
Expand Down
50 changes: 25 additions & 25 deletions tests/ui/format_args.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: `to_string` applied to a type that implements `Display` in `format!` args
--> $DIR/format_args.rs:76:72
--> $DIR/format_args.rs:77:72
|
LL | let _ = format!("error: something failed at {}", Location::caller().to_string());
| ^^^^^^^^^^^^ help: remove this
Expand All @@ -8,145 +8,145 @@ LL | let _ = format!("error: something failed at {}", Location::caller().to_
= help: to override `-D warnings` add `#[allow(clippy::to_string_in_format_args)]`

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

error: `to_string` applied to a type that implements `Display` in `println!` args
--> $DIR/format_args.rs:160:24
--> $DIR/format_args.rs:161:24
|
LL | println!("{}", original[..10].to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use this: `&original[..10]`
Expand Down
31 changes: 31 additions & 0 deletions tests/ui/to_string_trait_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#![warn(clippy::to_string_trait_impl)]

use std::fmt::{self, Display};

struct Point {
x: usize,
y: usize,
}

impl ToString for Point {
fn to_string(&self) -> String {
format!("({}, {})", self.x, self.y)
}
}

struct Foo;

impl Display for Foo {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "Foo")
}
}

struct Bar;

impl Bar {
#[allow(clippy::inherent_to_string)]
fn to_string(&self) -> String {
String::from("Bar")
}
}
16 changes: 16 additions & 0 deletions tests/ui/to_string_trait_impl.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: direct implementation of `ToString`
--> $DIR/to_string_trait_impl.rs:10:1
|
LL | / impl ToString for Point {
LL | | fn to_string(&self) -> String {
LL | | format!("({}, {})", self.x, self.y)
LL | | }
LL | | }
| |_^
|
= help: prefer implementing `Display` instead
= note: `-D clippy::to-string-trait-impl` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::to_string_trait_impl)]`

error: aborting due to 1 previous error

3 changes: 3 additions & 0 deletions tests/ui/unconditional_recursion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ impl PartialEq for S8 {

struct S9;

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

struct S10;

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

struct S11;

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

0 comments on commit 79f10cf

Please sign in to comment.