From fab10c07e8012b1190550615173ba6e5bca45c5e Mon Sep 17 00:00:00 2001 From: mcarton Date: Wed, 3 Feb 2016 13:42:46 +0100 Subject: [PATCH] Fix confusing message for STRING_TO_STRING --- README.md | 2 +- src/methods.rs | 7 ++++--- tests/compile-fail/methods.rs | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 006ddcd7a7f..3f99b73c4cb 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ name [string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add) | allow | using `x + ..` where x is a `String`; suggests using `push_str()` instead [string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign) | allow | using `x = x + ..` where x is a `String`; suggests using `push_str()` instead [string_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes) | warn | calling `as_bytes` on a string literal; suggests using a byte string literal instead -[string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String.to_string()` which is a no-op +[string_to_string](https://github.com/Manishearth/rust-clippy/wiki#string_to_string) | warn | calling `String::to_string` which is inefficient [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment) | warn | assignments to temporaries [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg) | warn | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`. [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity) | warn | usage of very complex types; recommends factoring out parts into `type` definitions diff --git a/src/methods.rs b/src/methods.rs index 0584d39330d..0a06351afac 100644 --- a/src/methods.rs +++ b/src/methods.rs @@ -56,13 +56,14 @@ declare_lint!(pub STR_TO_STRING, Warn, /// **What it does:** This lint checks for `.to_string()` method calls on values of type `String`. It is `Warn` by default. /// -/// **Why is this bad?** As our string is already owned, this whole operation is basically a no-op, but still creates a clone of the string (which, if really wanted, should be done with `.clone()`). +/// **Why is this bad?** This is an non-efficient way to clone a `String`, `.clone()` should be used +/// instead. `String` implements `ToString` mostly for generics. /// /// **Known problems:** None /// /// **Example:** `s.to_string()` where `s: String` declare_lint!(pub STRING_TO_STRING, Warn, - "calling `String.to_string()` which is a no-op"); + "calling `String::to_string` which is inefficient"); /// **What it does:** This lint checks for methods that should live in a trait implementation of a `std` trait (see [llogiq's blog post](http://llogiq.github.io/2015/07/30/traits.html) for further information) instead of an inherent implementation. It is `Warn` by default. /// @@ -560,7 +561,7 @@ fn lint_to_string(cx: &LateContext, expr: &Expr, to_string_args: &MethodArgs) { span_lint(cx, STRING_TO_STRING, expr.span, - "`String.to_string()` is a no-op; use `clone()` to make a copy"); + "`String::to_string` is an inefficient way to clone a `String`; use `clone()` instead"); } } diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs index f998a83e831..464a7c26e44 100644 --- a/tests/compile-fail/methods.rs +++ b/tests/compile-fail/methods.rs @@ -267,7 +267,7 @@ fn main() { let v = &"str"; let string = v.to_string(); //~ERROR `(*v).to_owned()` is faster - let _again = string.to_string(); //~ERROR `String.to_string()` is a no-op + let _again = string.to_string(); //~ERROR `String::to_string` is an inefficient way to clone a `String`; use `clone()` instead res.ok().expect("disaster!"); //~ERROR called `ok().expect()` // the following should not warn, since `expect` isn't implemented unless