From 36ec166d9a394f0f2fd8a8cfc4c0a076828f34eb Mon Sep 17 00:00:00 2001 From: merelymyself Date: Fri, 3 Jun 2022 14:46:24 +0800 Subject: [PATCH] second suggestion --- clippy_lints/src/path_from_format.rs | 77 +++++++++++++--------------- tests/ui/path_from_format.rs | 3 ++ tests/ui/path_from_format.stderr | 22 +++++++- 3 files changed, 60 insertions(+), 42 deletions(-) diff --git a/clippy_lints/src/path_from_format.rs b/clippy_lints/src/path_from_format.rs index 10a104a90b5e..9370c73f8345 100644 --- a/clippy_lints/src/path_from_format.rs +++ b/clippy_lints/src/path_from_format.rs @@ -1,5 +1,5 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet; +use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_help}; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::macros::{root_macro_call, FormatArgsExpn}; use rustc_errors::Applicability; @@ -7,7 +7,6 @@ use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; -use std::fmt::Write as _; declare_clippy_lint! { /// ### What it does @@ -46,55 +45,53 @@ impl<'tcx> LateLintPass<'tcx> for PathFromFormat { if let Some(macro_call) = root_macro_call(args[0].span); if cx.tcx.item_name(macro_call.def_id) == sym::format; if let Some(format_args) = FormatArgsExpn::find_nested(cx, &args[0], macro_call.expn); + let mut applicability = Applicability::MachineApplicable; + let format_args_snip = snippet_with_applicability(cx, format_args.inputs_span(), "..", &mut applicability); + if let Some(end_of_literal) = format_args_snip.find("\","); then { - let full_expr = snippet(cx, expr.span, "error").to_string(); - let split_expr: Vec<&str> = full_expr.split('!').collect(); - let args_to_macro = split_expr[1]; - let replaced = args_to_macro.replace('(', "").replace(')', ""); - let unformatted: Vec<&str> = replaced.split(',').collect(); - let mut push_targets: Vec = Vec::new(); - let mut temp_string = String::new(); - for c in unformatted[0].chars() { - if c == '/' || c == '\\' { - push_targets.push(temp_string.clone()); - temp_string = String::new(); - } - else if c == '}' { - temp_string.push_str(&unformatted[1].replace(' ', "")); - } - else if c != '{' && c != '"' { - temp_string.push(c); - } - } - if !temp_string.is_empty() { - push_targets.push(temp_string.clone()); - temp_string = String::new(); + let (literal, vars) = format_args_snip.split_at(end_of_literal); + let mut literal = literal.to_string(); + literal.remove(0); + let v: Vec<&str> = literal.split("{}").collect(); + let real_vars = vars.strip_prefix("\", ").unwrap_or(vars); + if v.len() != 2 || real_vars.contains(',') { + span_lint_and_help( + cx, + PATH_FROM_FORMAT, + expr.span, + "`format!(..)` used to form `PathBuf`", + None, + "consider using `.join()` to avoid the extra allocation", + ); + return; } - for target in push_targets { - let target_processed = - if target == unformatted[1].replace(' ', "") { - target + let sugg = { + if v[0].is_empty() { + let mut str1 = v[1].to_string(); + if str1.starts_with('\\') || str1.starts_with('/') { + str1.remove(0); } - else { - let mut s = String::from("\""); - s.push_str(&target); - s.push('"'); - s - }; - if temp_string.is_empty() { - let _ = write!(temp_string, "Path::new({})", target_processed); + format!("Path::new({real_vars}).join(\"{str1}\")") + } + else if v[1].is_empty() { + let str1 = v[0].to_string(); + format!("Path::new(\"{str1}\").join({real_vars})") } else { - let _ = write!(temp_string, ".join({})", target_processed); + let (str1, mut str2) = (v[0].to_string(), v[1].to_string()); + if str2.starts_with('\\') || str2.starts_with('/') { + str2.remove(0); + } + format!("Path::new(\"{str1}\").join({real_vars}).join(\"{str2}\")") } - } + }; span_lint_and_sugg( cx, PATH_FROM_FORMAT, expr.span, "`format!(..)` used to form `PathBuf`", "consider using `.join()` to avoid the extra allocation", - temp_string, + sugg, Applicability::MaybeIncorrect, ); } diff --git a/tests/ui/path_from_format.rs b/tests/ui/path_from_format.rs index 846493953420..7eebc906b2eb 100644 --- a/tests/ui/path_from_format.rs +++ b/tests/ui/path_from_format.rs @@ -5,4 +5,7 @@ use std::path::PathBuf; fn main() { let mut base_path1 = ""; PathBuf::from(format!("{}/foo/bar", base_path1)); + PathBuf::from(format!("/foo/bar/{}", base_path1)); + PathBuf::from(format!("/foo/{}/bar", base_path1)); + PathBuf::from(format!("foo/{}/bar", base_path1)); } diff --git a/tests/ui/path_from_format.stderr b/tests/ui/path_from_format.stderr index 3de5ce51015e..986743da0800 100644 --- a/tests/ui/path_from_format.stderr +++ b/tests/ui/path_from_format.stderr @@ -2,9 +2,27 @@ error: `format!(..)` used to form `PathBuf` --> $DIR/path_from_format.rs:7:5 | LL | PathBuf::from(format!("{}/foo/bar", base_path1)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new(base_path1).join("foo").join("bar")` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new(base_path1).join("foo/bar")` | = note: `-D clippy::path-from-format` implied by `-D warnings` -error: aborting due to previous error +error: `format!(..)` used to form `PathBuf` + --> $DIR/path_from_format.rs:8:5 + | +LL | PathBuf::from(format!("/foo/bar/{}", base_path1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new("/foo/bar/").join(base_path1)` + +error: `format!(..)` used to form `PathBuf` + --> $DIR/path_from_format.rs:9:5 + | +LL | PathBuf::from(format!("/foo/{}/bar", base_path1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new("/foo/").join(base_path1).join("bar")` + +error: `format!(..)` used to form `PathBuf` + --> $DIR/path_from_format.rs:10:5 + | +LL | PathBuf::from(format!("foo/{}/bar", base_path1)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new("foo/").join(base_path1).join("bar")` + +error: aborting due to 4 previous errors