From 082cfa79902d1aac5e251a0c2018c0537bcb9be8 Mon Sep 17 00:00:00 2001 From: Philipp Hansch Date: Fri, 24 May 2019 08:19:51 +0200 Subject: [PATCH 1/8] [Backported] Rustup to https://github.com/rust-lang/rust/pull/59545 --- tests/ui/for_loop.stderr | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/ui/for_loop.stderr b/tests/ui/for_loop.stderr index a18eb20774d5..14082e8790b4 100644 --- a/tests/ui/for_loop.stderr +++ b/tests/ui/for_loop.stderr @@ -66,7 +66,7 @@ error: it is more concise to loop over references to containers instead of using --> $DIR/for_loop.rs:107:15 | LL | for _v in vec.iter() {} - | ^^^^^^^^^^ + | ^^^^^^^^^^ help: to write this more concisely, try: `&vec` | = note: `-D clippy::explicit-iter-loop` implied by `-D warnings` @@ -74,13 +74,13 @@ error: it is more concise to loop over references to containers instead of using --> $DIR/for_loop.rs:109:15 | LL | for _v in vec.iter_mut() {} - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&mut vec` error: it is more concise to loop over containers instead of using explicit iteration methods` --> $DIR/for_loop.rs:112:15 | LL | for _v in out_vec.into_iter() {} - | ^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `out_vec` | = note: `-D clippy::explicit-into-iter-loop` implied by `-D warnings` @@ -88,61 +88,61 @@ error: it is more concise to loop over references to containers instead of using --> $DIR/for_loop.rs:115:15 | LL | for _v in array.into_iter() {} - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&array` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:120:15 | LL | for _v in [1, 2, 3].iter() {} - | ^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[1, 2, 3]` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:124:15 | LL | for _v in [0; 32].iter() {} - | ^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^ help: to write this more concisely, try: `&[0; 32]` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:129:15 | LL | for _v in ll.iter() {} - | ^^^^^^^^^ + | ^^^^^^^^^ help: to write this more concisely, try: `&ll` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:132:15 | LL | for _v in vd.iter() {} - | ^^^^^^^^^ + | ^^^^^^^^^ help: to write this more concisely, try: `&vd` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:135:15 | LL | for _v in bh.iter() {} - | ^^^^^^^^^ + | ^^^^^^^^^ help: to write this more concisely, try: `&bh` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:138:15 | LL | for _v in hm.iter() {} - | ^^^^^^^^^ + | ^^^^^^^^^ help: to write this more concisely, try: `&hm` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:141:15 | LL | for _v in bt.iter() {} - | ^^^^^^^^^ + | ^^^^^^^^^ help: to write this more concisely, try: `&bt` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:144:15 | LL | for _v in hs.iter() {} - | ^^^^^^^^^ + | ^^^^^^^^^ help: to write this more concisely, try: `&hs` error: it is more concise to loop over references to containers instead of using explicit iteration methods --> $DIR/for_loop.rs:147:15 | LL | for _v in bs.iter() {} - | ^^^^^^^^^ + | ^^^^^^^^^ help: to write this more concisely, try: `&bs` error: you are iterating over `Iterator::next()` which is an Option; this will compile but is probably not what you want --> $DIR/for_loop.rs:149:15 From 92bd160aacadfc9af2f3c8e8edeba60aed1e1ddc Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 23 Aug 2019 05:42:45 +0000 Subject: [PATCH 2/8] Update Unicode lint tests --- clippy_lints/src/unicode.rs | 10 +++++----- tests/ui/unicode.stderr | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/clippy_lints/src/unicode.rs b/clippy_lints/src/unicode.rs index 15c74eff73b1..f7e1edae5162 100644 --- a/clippy_lints/src/unicode.rs +++ b/clippy_lints/src/unicode.rs @@ -56,17 +56,17 @@ declare_clippy_lint! { /// /// **Known problems** None. /// - /// **Example:** You may not see it, but “à” and “à” aren't the same string. The + /// **Example:** You may not see it, but "à"" and "à"" aren't the same string. The /// former when escaped is actually `"a\u{300}"` while the latter is `"\u{e0}"`. pub UNICODE_NOT_NFC, pedantic, - "using a unicode literal not in NFC normal form (see [unicode tr15](http://www.unicode.org/reports/tr15/) for further information)" + "using a Unicode literal not in NFC normal form (see [Unicode tr15](http://www.unicode.org/reports/tr15/) for further information)" } declare_lint_pass!(Unicode => [ZERO_WIDTH_SPACE, NON_ASCII_LITERAL, UNICODE_NOT_NFC]); -impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Unicode { - fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { +impl LateLintPass<'_, '_> for Unicode { + fn check_expr(&mut self, cx: &LateContext<'_, '_>, expr: &'_ Expr) { if let ExprKind::Lit(ref lit) = expr.node { if let LitKind::Str(_, _) = lit.node { check_str(cx, lit.span, expr.hir_id) @@ -122,7 +122,7 @@ fn check_str(cx: &LateContext<'_, '_>, span: Span, id: HirId) { cx, UNICODE_NOT_NFC, span, - "non-nfc unicode sequence detected", + "non-NFC Unicode sequence detected", "consider replacing the string with", string.nfc().collect::(), Applicability::MachineApplicable, diff --git a/tests/ui/unicode.stderr b/tests/ui/unicode.stderr index 641680431a2c..4575a132e5b2 100644 --- a/tests/ui/unicode.stderr +++ b/tests/ui/unicode.stderr @@ -2,15 +2,15 @@ error: zero-width space detected --> $DIR/unicode.rs:3:12 | LL | print!("Here >​< is a ZWS, and ​another"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider replacing the string with: `"Here >/u{200B}< is a ZWS, and /u{200B}another"` | = note: `-D clippy::zero-width-space` implied by `-D warnings` -error: non-nfc unicode sequence detected +error: non-NFC Unicode sequence detected --> $DIR/unicode.rs:9:12 | LL | print!("̀àh?"); - | ^^^^^ + | ^^^^^ help: consider replacing the string with: `"̀àh?"` | = note: `-D clippy::unicode-not-nfc` implied by `-D warnings` @@ -18,7 +18,7 @@ error: literal non-ASCII character detected --> $DIR/unicode.rs:15:12 | LL | print!("Üben!"); - | ^^^^^^^ + | ^^^^^^^ help: consider replacing the string with: `"/u{dc}ben!"` | = note: `-D clippy::non-ascii-literal` implied by `-D warnings` From 1564306943c670a6f9df6b58d1d6b56ac567eb01 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 23 Aug 2019 12:26:24 +0700 Subject: [PATCH 3/8] Re-factor useless_format lint --- clippy_lints/src/format.rs | 189 +++++++++++++++++--------------- clippy_lints/src/utils/paths.rs | 5 +- tests/ui/format.stderr | 4 +- 3 files changed, 105 insertions(+), 93 deletions(-) diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index 751d8fd00830..add1056b0f88 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -6,7 +6,6 @@ use crate::utils::{ use if_chain::if_chain; use rustc::hir::*; use rustc::lint::{LateContext, LateLintPass, LintArray, LintContext, LintPass}; -use rustc::ty; use rustc::{declare_lint_pass, declare_tool_lint}; use rustc_errors::Applicability; use syntax::ast::LitKind; @@ -39,56 +38,16 @@ declare_lint_pass!(UselessFormat => [USELESS_FORMAT]); impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UselessFormat { fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) { - if let Some(span) = is_expn_of(expr.span, "format") { - if in_macro_or_desugar(span) { - return; - } - match expr.node { - // `format!("{}", foo)` expansion - ExprKind::Call(ref fun, ref args) => { - if_chain! { - if let ExprKind::Path(ref qpath) = fun.node; - if let Some(fun_def_id) = resolve_node(cx, qpath, fun.hir_id).opt_def_id(); - let new_v1 = match_def_path(cx, fun_def_id, &paths::FMT_ARGUMENTS_NEWV1); - let new_v1_fmt = match_def_path(cx, - fun_def_id, - &paths::FMT_ARGUMENTS_NEWV1FORMATTED - ); - if new_v1 || new_v1_fmt; - if check_single_piece(&args[0]); - if let Some(format_arg) = get_single_string_arg(cx, &args[1]); - if new_v1 || check_unformatted(&args[2]); - if let ExprKind::AddrOf(_, ref format_arg) = format_arg.node; - then { - let (message, sugg) = if_chain! { - if let ExprKind::MethodCall(ref path, _, _) = format_arg.node; - if path.ident.as_interned_str().as_symbol() == sym!(to_string); - then { - ("`to_string()` is enough", - snippet(cx, format_arg.span, "").to_string()) - } else { - ("consider using .to_string()", - format!("{}.to_string()", snippet(cx, format_arg.span, ""))) - } - }; + let span = match is_expn_of(expr.span, "format") { + Some(s) if !in_macro_or_desugar(s) => s, + _ => return, + }; - span_useless_format(cx, span, message, sugg); - } - } - }, - // `format!("foo")` expansion contains `match () { () => [], }` - ExprKind::Match(ref matchee, _, _) => { - if let ExprKind::Tup(ref tup) = matchee.node { - if tup.is_empty() { - let actual_snippet = snippet(cx, expr.span, "").to_string(); - let actual_snippet = actual_snippet.replace("{{}}", "{}"); - let sugg = format!("{}.to_string()", actual_snippet); - span_useless_format(cx, span, "consider using .to_string()", sugg); - } - } - }, - _ => (), - } + // Operate on the only argument of `alloc::fmt::format`. + if let Some(sugg) = on_new_v1(cx, expr) { + span_useless_format(cx, span, "consider using .to_string()", sugg); + } else if let Some(sugg) = on_new_v1_fmt(cx, expr) { + span_useless_format(cx, span, "consider using .to_string()", sugg); } } } @@ -112,56 +71,105 @@ fn span_useless_format(cx: &T, span: Span, help: &str, mut sugg: }); } -/// Checks if the expressions matches `&[""]` -fn check_single_piece(expr: &Expr) -> bool { +fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arms: &'a [Arm]) -> Option { if_chain! { - if let ExprKind::AddrOf(_, ref expr) = expr.node; // &[""] - if let ExprKind::Array(ref exprs) = expr.node; // [""] - if exprs.len() == 1; - if let ExprKind::Lit(ref lit) = exprs[0].node; - if let LitKind::Str(ref lit, _) = lit.node; + if let ExprKind::AddrOf(_, ref format_args) = expr.node; + if let ExprKind::Array(ref elems) = arms[0].body.node; + if elems.len() == 1; + if let ExprKind::Call(ref fun, ref args) = elems[0].node; + if let ExprKind::Path(ref qpath) = fun.node; + if let Some(did) = resolve_node(cx, qpath, fun.hir_id).opt_def_id(); + if match_def_path(cx, did, &paths::FMT_ARGUMENTV1_NEW); + // matches `core::fmt::Display::fmt` + if args.len() == 2; + if let ExprKind::Path(ref qpath) = args[1].node; + if let Some(did) = resolve_node(cx, qpath, args[1].hir_id).opt_def_id(); + if match_def_path(cx, did, &paths::DISPLAY_FMT_METHOD); + if arms[0].pats.len() == 1; + // check `(arg0,)` in match block + if let PatKind::Tuple(ref pats, None) = arms[0].pats[0].node; + if pats.len() == 1; then { - return lit.as_str().is_empty(); + if let ExprKind::Lit(ref lit) = format_args.node { + if let LitKind::Str(ref s, _) = lit.node { + let snip = s.as_str().replace("{{}}", "{}"); + let sugg = format!("\"{}\".to_string()", snip); + return Some(sugg); + } + return None; + } else { + let snip = snippet(cx, format_args.span, ""); + if let ExprKind::MethodCall(ref path, _, _) = format_args.node { + if path.ident.name == sym!(to_string) { + return Some(format!("{}", snip)); + } + } + return Some(format!("{}.to_string()", snip)); + } } } - - false + None } -/// Checks if the expressions matches -/// ```rust,ignore -/// &match (&"arg",) { -/// (__arg0,) => [::std::fmt::ArgumentV1::new(__arg0, -/// ::std::fmt::Display::fmt)], -/// } -/// ``` -/// and that the type of `__arg0` is `&str` or `String`, -/// then returns the span of first element of the matched tuple. -fn get_single_string_arg<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option<&'a Expr> { +fn on_new_v1<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option { if_chain! { - if let ExprKind::AddrOf(_, ref expr) = expr.node; - if let ExprKind::Match(ref match_expr, ref arms, _) = expr.node; - if arms.len() == 1; - if arms[0].pats.len() == 1; - if let PatKind::Tuple(ref pat, None) = arms[0].pats[0].node; - if pat.len() == 1; - if let ExprKind::Array(ref exprs) = arms[0].body.node; - if exprs.len() == 1; - if let ExprKind::Call(_, ref args) = exprs[0].node; + if let ExprKind::Call(ref fun, ref args) = expr.node; if args.len() == 2; - if let ExprKind::Path(ref qpath) = args[1].node; - if let Some(fun_def_id) = resolve_node(cx, qpath, args[1].hir_id).opt_def_id(); - if match_def_path(cx, fun_def_id, &paths::DISPLAY_FMT_METHOD); + if let ExprKind::Path(ref qpath) = fun.node; + if let Some(did) = resolve_node(cx, qpath, fun.hir_id).opt_def_id(); + if match_def_path(cx, did, &paths::FMT_ARGUMENTS_NEW_V1); + // Argument 1 in `new_v1()` + if let ExprKind::AddrOf(_, ref arr) = args[0].node; + if let ExprKind::Array(ref pieces) = arr.node; + if pieces.len() == 1; + if let ExprKind::Lit(ref lit) = pieces[0].node; + if let LitKind::Str(ref s, _) = lit.node; + // Argument 2 in `new_v1()` + if let ExprKind::AddrOf(_, ref arg1) = args[1].node; + if let ExprKind::Match(ref matchee, ref arms, MatchSource::Normal) = arg1.node; + if arms.len() == 1; + if let ExprKind::Tup(ref tup) = matchee.node; then { - let ty = walk_ptrs_ty(cx.tables.pat_ty(&pat[0])); - if ty.sty == ty::Str || match_type(cx, ty, &paths::STRING) { - if let ExprKind::Tup(ref values) = match_expr.node { - return Some(&values[0]); + // `format!("foo")` expansion contains `match () { () => [], }` + if tup.is_empty() { + let snip = s.as_str().replace("{{}}", "{}"); + let sugg = format!("\"{}\".to_string()", snip); + return Some(sugg); + } else { + if s.as_str().is_empty() { + return on_argumentv1_new(cx, &tup[0], arms); + } else { + return None; } } } } + None +} +fn on_new_v1_fmt<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option { + if_chain! { + if let ExprKind::Call(ref fun, ref args) = expr.node; + if args.len() == 3; + if let ExprKind::Path(ref qpath) = fun.node; + if let Some(did) = resolve_node(cx, qpath, fun.hir_id).opt_def_id(); + if match_def_path(cx, did, &paths::FMT_ARGUMENTS_NEW_V1_FORMATTED); + if check_unformatted(&args[2]); + // Argument 1 in `new_v1_formatted()` + if let ExprKind::AddrOf(_, ref arr) = args[0].node; + if let ExprKind::Array(ref pieces) = arr.node; + if pieces.len() == 1; + if let ExprKind::Lit(ref lit) = pieces[0].node; + if let LitKind::Str(..) = lit.node; + // Argument 2 in `new_v1_formatted()` + if let ExprKind::AddrOf(_, ref arg1) = args[1].node; + if let ExprKind::Match(ref matchee, ref arms, MatchSource::Normal) = arg1.node; + if arms.len() == 1; + if let ExprKind::Tup(ref tup) = matchee.node; + then { + return on_argumentv1_new(cx, &tup[0], arms); + } + } None } @@ -170,6 +178,7 @@ fn get_single_string_arg<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr) -> Option /// &[_ { /// format: _ { /// width: _::Implied, +/// precision: _::Implied, /// ... /// }, /// ..., @@ -180,15 +189,17 @@ fn check_unformatted(expr: &Expr) -> bool { if let ExprKind::AddrOf(_, ref expr) = expr.node; if let ExprKind::Array(ref exprs) = expr.node; if exprs.len() == 1; + // struct `core::fmt::rt::v1::Argument` if let ExprKind::Struct(_, ref fields, _) = exprs[0].node; if let Some(format_field) = fields.iter().find(|f| f.ident.name == sym!(format)); + // struct `core::fmt::rt::v1::FormatSpec` if let ExprKind::Struct(_, ref fields, _) = format_field.expr.node; - if let Some(width_field) = fields.iter().find(|f| f.ident.name == sym!(width)); - if let ExprKind::Path(ref width_qpath) = width_field.expr.node; - if last_path_segment(width_qpath).ident.name == sym!(Implied); if let Some(precision_field) = fields.iter().find(|f| f.ident.name == sym!(precision)); if let ExprKind::Path(ref precision_path) = precision_field.expr.node; if last_path_segment(precision_path).ident.name == sym!(Implied); + if let Some(width_field) = fields.iter().find(|f| f.ident.name == sym!(width)); + if let ExprKind::Path(ref width_qpath) = width_field.expr.node; + if last_path_segment(width_qpath).ident.name == sym!(Implied); then { return true; } diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 62b22afff95b..14f570abcdbc 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -27,8 +27,9 @@ pub const DROP: [&str; 3] = ["core", "mem", "drop"]; pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"]; pub const DURATION: [&str; 3] = ["core", "time", "Duration"]; pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"]; -pub const FMT_ARGUMENTS_NEWV1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"]; -pub const FMT_ARGUMENTS_NEWV1FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"]; +pub const FMT_ARGUMENTS_NEW_V1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"]; +pub const FMT_ARGUMENTS_NEW_V1_FORMATTED: [&str; 4] = ["core", "fmt", "Arguments", "new_v1_formatted"]; +pub const FMT_ARGUMENTV1_NEW: [&str; 4] = ["core", "fmt", "ArgumentV1", "new"]; pub const FROM_FROM: [&str; 4] = ["core", "convert", "From", "from"]; pub const FROM_TRAIT: [&str; 3] = ["core", "convert", "From"]; pub const HASH: [&str; 2] = ["hash", "Hash"]; diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index ec1253589f62..d81f48b0864c 100644 --- a/tests/ui/format.stderr +++ b/tests/ui/format.stderr @@ -58,13 +58,13 @@ error: useless use of `format!` --> $DIR/format.rs:59:5 | LL | format!("{}", 42.to_string()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `42.to_string();` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `42.to_string();` error: useless use of `format!` --> $DIR/format.rs:61:5 | LL | format!("{}", x.display().to_string()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: `to_string()` is enough: `x.display().to_string();` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `x.display().to_string();` error: aborting due to 11 previous errors From 413eb5b946955cd33ffd6fb58d7c59de978f92d6 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 23 Aug 2019 08:01:41 +0000 Subject: [PATCH 4/8] Add raw string regression test for useless_format lint --- clippy_lints/src/format.rs | 17 ++++------------- tests/ui/format.fixed | 1 + tests/ui/format.rs | 4 ++++ tests/ui/format.stderr | 27 ++++++++++++++++++--------- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index add1056b0f88..f28f98b1caba 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -92,11 +92,8 @@ fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arm then { if let ExprKind::Lit(ref lit) = format_args.node { if let LitKind::Str(ref s, _) = lit.node { - let snip = s.as_str().replace("{{}}", "{}"); - let sugg = format!("\"{}\".to_string()", snip); - return Some(sugg); + return Some(format!("{:?}.to_string()", s.as_str())); } - return None; } else { let snip = snippet(cx, format_args.span, ""); if let ExprKind::MethodCall(ref path, _, _) = format_args.node { @@ -132,15 +129,9 @@ fn on_new_v1<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) -> Option [], }` if tup.is_empty() { - let snip = s.as_str().replace("{{}}", "{}"); - let sugg = format!("\"{}\".to_string()", snip); - return Some(sugg); - } else { - if s.as_str().is_empty() { - return on_argumentv1_new(cx, &tup[0], arms); - } else { - return None; - } + return Some(format!("{:?}.to_string()", s.as_str())); + } else if s.as_str().is_empty() { + return on_argumentv1_new(cx, &tup[0], arms); } } } diff --git a/tests/ui/format.fixed b/tests/ui/format.fixed index e4217411f053..401a1c533fef 100644 --- a/tests/ui/format.fixed +++ b/tests/ui/format.fixed @@ -13,6 +13,7 @@ fn main() { "foo".to_string(); "{}".to_string(); "{} abc {}".to_string(); + "foo {}\n\" bar".to_string(); "foo".to_string(); format!("{:?}", "foo"); // Don't warn about `Debug`. diff --git a/tests/ui/format.rs b/tests/ui/format.rs index 61ef3e7243d7..1cbc15cfcecb 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -13,6 +13,10 @@ fn main() { format!("foo"); format!("{{}}"); format!("{{}} abc {{}}"); + format!( + r##"foo {{}} +" bar"## + ); format!("{}", "foo"); format!("{:?}", "foo"); // Don't warn about `Debug`. diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index d81f48b0864c..433a0e705aca 100644 --- a/tests/ui/format.stderr +++ b/tests/ui/format.stderr @@ -19,52 +19,61 @@ LL | format!("{{}} abc {{}}"); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"{} abc {}".to_string();` error: useless use of `format!` - --> $DIR/format.rs:17:5 + --> $DIR/format.rs:16:5 + | +LL | / format!( +LL | | r##"foo {{}} +LL | | " bar"## +LL | | ); + | |______^ help: consider using .to_string(): `"foo {}/n/" bar".to_string();` + +error: useless use of `format!` + --> $DIR/format.rs:21:5 | LL | format!("{}", "foo"); | ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();` error: useless use of `format!` - --> $DIR/format.rs:21:5 + --> $DIR/format.rs:25:5 | LL | format!("{:+}", "foo"); // Warn when the format makes no difference. | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();` error: useless use of `format!` - --> $DIR/format.rs:22:5 + --> $DIR/format.rs:26:5 | LL | format!("{:<}", "foo"); // Warn when the format makes no difference. | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `"foo".to_string();` error: useless use of `format!` - --> $DIR/format.rs:27:5 + --> $DIR/format.rs:31:5 | LL | format!("{}", arg); | ^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();` error: useless use of `format!` - --> $DIR/format.rs:31:5 + --> $DIR/format.rs:35:5 | LL | format!("{:+}", arg); // Warn when the format makes no difference. | ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();` error: useless use of `format!` - --> $DIR/format.rs:32:5 + --> $DIR/format.rs:36:5 | LL | format!("{:<}", arg); // Warn when the format makes no difference. | ^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `arg.to_string();` error: useless use of `format!` - --> $DIR/format.rs:59:5 + --> $DIR/format.rs:63:5 | LL | format!("{}", 42.to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `42.to_string();` error: useless use of `format!` - --> $DIR/format.rs:61:5 + --> $DIR/format.rs:65:5 | LL | format!("{}", x.display().to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `x.display().to_string();` -error: aborting due to 11 previous errors +error: aborting due to 12 previous errors From 652840b4f16ee8f5b99349bcadb0931ed5b064b2 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 23 Aug 2019 15:46:23 +0700 Subject: [PATCH 5/8] Re-add false positive check --- clippy_lints/src/format.rs | 6 ++++++ tests/ui/format.fixed | 4 ++++ tests/ui/format.rs | 4 ++++ tests/ui/format.stderr | 8 +++++++- 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/format.rs b/clippy_lints/src/format.rs index f28f98b1caba..a20217991ab9 100644 --- a/clippy_lints/src/format.rs +++ b/clippy_lints/src/format.rs @@ -90,6 +90,10 @@ fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arm if let PatKind::Tuple(ref pats, None) = arms[0].pats[0].node; if pats.len() == 1; then { + let ty = walk_ptrs_ty(cx.tables.pat_ty(&pats[0])); + if ty.sty != rustc::ty::Str && !match_type(cx, ty, &paths::STRING) { + return None; + } if let ExprKind::Lit(ref lit) = format_args.node { if let LitKind::Str(ref s, _) = lit.node { return Some(format!("{:?}.to_string()", s.as_str())); @@ -100,6 +104,8 @@ fn on_argumentv1_new<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr, arm if path.ident.name == sym!(to_string) { return Some(format!("{}", snip)); } + } else if let ExprKind::Binary(..) = format_args.node { + return Some(format!("{}", snip)); } return Some(format!("{}.to_string()", snip)); } diff --git a/tests/ui/format.fixed b/tests/ui/format.fixed index 401a1c533fef..6e100230a3ad 100644 --- a/tests/ui/format.fixed +++ b/tests/ui/format.fixed @@ -60,4 +60,8 @@ fn main() { 42.to_string(); let x = std::path::PathBuf::from("/bar/foo/qux"); x.display().to_string(); + + // False positive + let a = "foo".to_string(); + let _ = Some(a + "bar"); } diff --git a/tests/ui/format.rs b/tests/ui/format.rs index 1cbc15cfcecb..1fae6603ac09 100644 --- a/tests/ui/format.rs +++ b/tests/ui/format.rs @@ -63,4 +63,8 @@ fn main() { format!("{}", 42.to_string()); let x = std::path::PathBuf::from("/bar/foo/qux"); format!("{}", x.display().to_string()); + + // False positive + let a = "foo".to_string(); + let _ = Some(format!("{}", a + "bar")); } diff --git a/tests/ui/format.stderr b/tests/ui/format.stderr index 433a0e705aca..9736f34b03b4 100644 --- a/tests/ui/format.stderr +++ b/tests/ui/format.stderr @@ -75,5 +75,11 @@ error: useless use of `format!` LL | format!("{}", x.display().to_string()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `x.display().to_string();` -error: aborting due to 12 previous errors +error: useless use of `format!` + --> $DIR/format.rs:69:18 + | +LL | let _ = Some(format!("{}", a + "bar")); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using .to_string(): `a + "bar"` + +error: aborting due to 13 previous errors From 3aea86030eeca7dff94139b24d6b76294609dbce Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Fri, 23 Aug 2019 09:49:49 +0000 Subject: [PATCH 6/8] Run update_lints for Unicode lint --- src/lintlist/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index b41ed4a5910e..a10c1d3901f7 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -1858,7 +1858,7 @@ pub const ALL_LINTS: [Lint; 309] = [ Lint { name: "unicode_not_nfc", group: "pedantic", - desc: "using a unicode literal not in NFC normal form (see [unicode tr15](http://www.unicode.org/reports/tr15/) for further information)", + desc: "using a Unicode literal not in NFC normal form (see [Unicode tr15](http://www.unicode.org/reports/tr15/) for further information)", deprecation: None, module: "unicode", }, From 75d951e1adf515b6ef549c4cf12432dfbe1b8ac5 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 27 Sep 2019 15:36:20 +0200 Subject: [PATCH 7/8] Add regression test for ICE #4579 --- tests/ui/ice-4579.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 tests/ui/ice-4579.rs diff --git a/tests/ui/ice-4579.rs b/tests/ui/ice-4579.rs new file mode 100644 index 000000000000..2e7e279f847d --- /dev/null +++ b/tests/ui/ice-4579.rs @@ -0,0 +1,13 @@ +#![allow(clippy::single_match)] + +use std::ptr; + +fn main() { + match Some(0_usize) { + Some(_) => { + let s = "012345"; + unsafe { ptr::read(s.as_ptr().offset(1) as *const [u8; 5]) }; + }, + _ => (), + }; +} From 4e7e71b46c88f9ba8b0a424053fe0f28c5e61ab3 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 27 Sep 2019 15:36:56 +0200 Subject: [PATCH 8/8] Fix ICE #4579 --- clippy_lints/src/consts.rs | 8 +++++--- clippy_lints/src/neg_multiply.rs | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/consts.rs b/clippy_lints/src/consts.rs index 0e1c1e60178f..c8ee818d885f 100644 --- a/clippy_lints/src/consts.rs +++ b/clippy_lints/src/consts.rs @@ -152,7 +152,7 @@ impl Constant { } /// Parses a `LitKind` to a `Constant`. -pub fn lit_to_constant(lit: &LitKind, ty: Ty<'_>) -> Constant { +pub fn lit_to_constant(lit: &LitKind, ty: Option>) -> Constant { use syntax::ast::*; match *lit { @@ -161,7 +161,9 @@ pub fn lit_to_constant(lit: &LitKind, ty: Ty<'_>) -> Constant { LitKind::ByteStr(ref s) => Constant::Binary(Lrc::clone(s)), LitKind::Char(c) => Constant::Char(c), LitKind::Int(n, _) => Constant::Int(n), - LitKind::Float(ref is, _) | LitKind::FloatUnsuffixed(ref is) => match ty.sty { + LitKind::Float(ref is, FloatTy::F32) => Constant::F32(is.as_str().parse().unwrap()), + LitKind::Float(ref is, FloatTy::F64) => Constant::F64(is.as_str().parse().unwrap()), + LitKind::FloatUnsuffixed(ref is) => match ty.expect("type of float is known").sty { ty::Float(FloatTy::F32) => Constant::F32(is.as_str().parse().unwrap()), ty::Float(FloatTy::F64) => Constant::F64(is.as_str().parse().unwrap()), _ => bug!(), @@ -225,7 +227,7 @@ impl<'c, 'cc> ConstEvalLateContext<'c, 'cc> { match e.node { ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id), ExprKind::Block(ref block, _) => self.block(block), - ExprKind::Lit(ref lit) => Some(lit_to_constant(&lit.node, self.tables.expr_ty(e))), + ExprKind::Lit(ref lit) => Some(lit_to_constant(&lit.node, self.tables.expr_ty_opt(e))), ExprKind::Array(ref vec) => self.multi(vec).map(Constant::Vec), ExprKind::Tup(ref tup) => self.multi(tup).map(Constant::Tuple), ExprKind::Repeat(ref value, _) => { diff --git a/clippy_lints/src/neg_multiply.rs b/clippy_lints/src/neg_multiply.rs index c235661a432b..8cbfee4f1fea 100644 --- a/clippy_lints/src/neg_multiply.rs +++ b/clippy_lints/src/neg_multiply.rs @@ -49,7 +49,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NegMultiply { fn check_mul(cx: &LateContext<'_, '_>, span: Span, lit: &Expr, exp: &Expr) { if_chain! { if let ExprKind::Lit(ref l) = lit.node; - if let Constant::Int(val) = consts::lit_to_constant(&l.node, cx.tables.expr_ty(lit)); + if let Constant::Int(val) = consts::lit_to_constant(&l.node, cx.tables.expr_ty_opt(lit)); if val == 1; if cx.tables.expr_ty(exp).is_integral(); then {