Skip to content

Fix useless_format suggestions #3808

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from Feb 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/base-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ rustup override set nightly
# avoid loop spam and allow cmds with exit status != 0
set +ex

for file in `find tests -not -path "tests/ui/format.rs" -not -path "tests/ui/formatting.rs" -not -path "tests/ui/empty_line_after_outer_attribute.rs" -not -path "tests/ui/double_parens.rs" -not -path "tests/ui/doc.rs" -not -path "tests/ui/unused_unit.rs" | grep "\.rs$"` ; do
for file in `find tests -not -path "tests/ui/formatting.rs" -not -path "tests/ui/empty_line_after_outer_attribute.rs" -not -path "tests/ui/double_parens.rs" -not -path "tests/ui/doc.rs" -not -path "tests/ui/unused_unit.rs" | grep "\.rs$"` ; do
rustfmt ${file} --check
if [ $? -ne 0 ]; then
echo "${file} needs reformatting!"
Expand Down
40 changes: 23 additions & 17 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@ use crate::utils::{
};
use if_chain::if_chain;
use rustc::hir::*;
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass};
use rustc::ty;
use rustc::{declare_tool_lint, lint_array};
use rustc_errors::Applicability;
use syntax::ast::LitKind;
use syntax::source_map::Span;

/// **What it does:** Checks for the use of `format!("string literal with no
/// argument")` and `format!("{}", foo)` where `foo` is a string.
Expand Down Expand Up @@ -82,14 +83,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
}
};

span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |db| {
db.span_suggestion(
expr.span,
message,
sugg,
Applicability::MachineApplicable,
);
});
span_useless_format(cx, span, message, sugg);
}
}
},
Expand All @@ -98,14 +92,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
if let ExprKind::Tup(ref tup) = matchee.node {
if tup.is_empty() {
let sugg = format!("{}.to_string()", snippet(cx, expr.span, "<expr>").into_owned());
span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |db| {
db.span_suggestion(
span,
"consider using .to_string()",
sugg,
Applicability::MachineApplicable, // snippet
);
});
span_useless_format(cx, span, "consider using .to_string()", sugg);
}
}
},
Expand All @@ -115,6 +102,25 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
}
}

fn span_useless_format<'a, 'tcx: 'a, T: LintContext<'tcx>>(cx: &'a T, span: Span, help: &str, mut sugg: String) {
let to_replace = span.source_callsite();

// The callsite span contains the statement semicolon for some reason.
let snippet = snippet(cx, to_replace, "..");
if snippet.ends_with(';') {
sugg.push(';');
}

span_lint_and_then(cx, USELESS_FORMAT, span, "useless use of `format!`", |db| {
db.span_suggestion(
to_replace,
help,
sugg,
Applicability::MachineApplicable, // snippet
);
});
}

/// Checks if the expressions matches `&[""]`
fn check_single_piece(expr: &Expr) -> bool {
if_chain! {
Expand Down
60 changes: 60 additions & 0 deletions tests/ui/format.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// run-rustfix

#![allow(clippy::print_literal)]
#![warn(clippy::useless_format)]

struct Foo(pub String);

macro_rules! foo {
($($t:tt)*) => (Foo(format!($($t)*)))
}

fn main() {
"foo".to_string();

"foo".to_string();
format!("{:?}", "foo"); // don't warn about debug
format!("{:8}", "foo");
format!("{:width$}", "foo", width = 8);
"foo".to_string(); // warn when the format makes no difference
"foo".to_string(); // warn when the format makes no difference
format!("foo {}", "bar");
format!("{} bar", "foo");

let arg: String = "".to_owned();
arg.to_string();
format!("{:?}", arg); // don't warn about debug
format!("{:8}", arg);
format!("{:width$}", arg, width = 8);
arg.to_string(); // warn when the format makes no difference
arg.to_string(); // warn when the format makes no difference
format!("foo {}", arg);
format!("{} bar", arg);

// we don’t want to warn for non-string args, see #697
format!("{}", 42);
format!("{:?}", 42);
format!("{:+}", 42);
format!("foo {}", 42);
format!("{} bar", 42);

// we only want to warn about `format!` itself
println!("foo");
println!("{}", "foo");
println!("foo {}", "foo");
println!("{}", 42);
println!("foo {}", 42);

// A format! inside a macro should not trigger a warning
foo!("should not warn");

// precision on string means slicing without panicking on size:
format!("{:.1}", "foo"); // could be "foo"[..1]
format!("{:.10}", "foo"); // could not be "foo"[..10]
format!("{:.prec$}", "foo", prec = 1);
format!("{:.prec$}", "foo", prec = 10);

42.to_string();
let x = std::path::PathBuf::from("/bar/foo/qux");
x.display().to_string();
}
2 changes: 2 additions & 0 deletions tests/ui/format.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
// run-rustfix

#![allow(clippy::print_literal)]
#![warn(clippy::useless_format)]

Expand Down
52 changes: 18 additions & 34 deletions tests/ui/format.stderr
Original file line number Diff line number Diff line change
@@ -1,74 +1,58 @@
error: useless use of `format!`
--> $DIR/format.rs:11:5
--> $DIR/format.rs:13:5
|
LL | format!("foo");
| ^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
| ^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`
|
= note: `-D clippy::useless-format` implied by `-D warnings`

error: useless use of `format!`
--> $DIR/format.rs:13:5
--> $DIR/format.rs:15:5
|
LL | format!("{}", "foo");
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`

error: useless use of `format!`
--> $DIR/format.rs:17:5
--> $DIR/format.rs:19:5
|
LL | format!("{:+}", "foo"); // warn when the format makes no difference
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`

error: useless use of `format!`
--> $DIR/format.rs:18:5
--> $DIR/format.rs:20:5
|
LL | format!("{:<}", "foo"); // warn when the format makes no difference
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();`

error: useless use of `format!`
--> $DIR/format.rs:23:5
--> $DIR/format.rs:25:5
|
LL | format!("{}", arg);
| ^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();`

error: useless use of `format!`
--> $DIR/format.rs:27:5
--> $DIR/format.rs:29:5
|
LL | format!("{:+}", arg); // warn when the format makes no difference
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();`

error: useless use of `format!`
--> $DIR/format.rs:28:5
--> $DIR/format.rs:30:5
|
LL | format!("{:<}", arg); // warn when the format makes no difference
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();`

error: useless use of `format!`
--> $DIR/format.rs:55:5
--> $DIR/format.rs:57:5
|
LL | format!("{}", 42.to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `42.to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `42.to_string();`

error: useless use of `format!`
--> $DIR/format.rs:57:5
--> $DIR/format.rs:59:5
|
LL | format!("{}", x.display().to_string());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `x.display().to_string()`
|
= note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `x.display().to_string();`

error: aborting due to 9 previous errors