Skip to content

Commit 5b08c84

Browse files
merelymyselfmerelymyself
committed
fix suggestions
1 parent ca59dec commit 5b08c84

File tree

2 files changed

+46
-63
lines changed

2 files changed

+46
-63
lines changed

‎clippy_lints/src/path_from_format.rs

Lines changed: 45 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg};
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::macros::{root_macro_call, FormatArgsExpn};
33
use clippy_utils::sugg::Sugg;
44
use clippy_utils::ty::is_type_diagnostic_item;
@@ -54,60 +54,41 @@ impl<'tcx> LateLintPass<'tcx> for PathFromFormat {
5454
then {
5555
let format_string_parts = format_args.format_string_parts;
5656
let format_value_args = format_args.value_args;
57-
let mut string_parts: Vec<&str> = format_string_parts.iter().map(rustc_span::Symbol::as_str).collect();
58-
string_parts.push("");
57+
let string_parts: Vec<&str> = format_string_parts.iter().map(rustc_span::Symbol::as_str).collect();
5958
let mut applicability = Applicability::MachineApplicable;
6059
let real_vars: Vec<Sugg<'_>> = format_value_args.iter().map(|x| Sugg::hir_with_applicability(cx, x, "..", &mut applicability)).collect();
6160
let order_of_real_vars: Vec<usize> = format_args.formatters.iter().map(|(x, _)| *x).collect();
61+
let mut arguments_in_order = Vec::new();
62+
for n in 0..real_vars.len() {
63+
arguments_in_order.push(real_vars[order_of_real_vars[n]].clone());
64+
}
65+
let mut paths_zip = string_parts.iter().take(arguments_in_order.len()).zip(arguments_in_order);
6266
let mut sugg = String::new();
6367
for n in 0..real_vars.len() {
64-
if !(
65-
string_parts[n].is_empty()
66-
|| string_parts[n].ends_with('/')
67-
|| string_parts[n].ends_with('\\')
68-
)
69-
|| !(
70-
string_parts[n+1].is_empty()
71-
|| string_parts[n+1].starts_with('/')
72-
|| string_parts[n+1].starts_with('\\')
73-
){
74-
span_lint_and_note(
75-
cx,
76-
PATH_FROM_FORMAT,
77-
expr.span,
78-
"`format!(..)` used to form `PathBuf`",
79-
None,
80-
"if it fits your use case, you may want to consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability",
81-
);
82-
return;
83-
}
84-
if n == 0 {
85-
if string_parts[0].is_empty() {
86-
sugg = format!("Path::new({})", real_vars[order_of_real_vars[0]]);
68+
if let Some((part, arg)) = paths_zip.next() {
69+
if is_valid_use_case(string_parts[n], string_parts.get(n+1).unwrap_or(&"")) {
70+
return;
8771
}
88-
else {
89-
push_comps(&mut sugg, Path::new(string_parts[0]));
90-
let _ = write!(sugg, ".join({})", real_vars[order_of_real_vars[0]]);
72+
if n == 0 {
73+
if part.is_empty() {
74+
sugg = format!("Path::new({})", arg);
75+
}
76+
else {
77+
push_comps(&mut sugg, part, false);
78+
let _ = write!(sugg, ".join({})", arg);
79+
}
9180
}
92-
}
93-
else if string_parts[n].is_empty() {
94-
sugg = format!("{sugg}.join({})", real_vars[order_of_real_vars[n]]);
95-
}
96-
else {
97-
let mut string = String::from(string_parts[n]);
98-
if string.starts_with('/') || string.starts_with('\\') {
99-
string.remove(0);
81+
else if n < real_vars.len() {
82+
push_comps(&mut sugg, &part.to_string(), true);
83+
let _ = write!(sugg, ".join({})", arg);
84+
}
85+
else {
86+
sugg = format!("{sugg}.join({})", arg);
10087
}
101-
push_comps(&mut sugg, Path::new(&string));
102-
let _ = write!(sugg, ".join({})", real_vars[order_of_real_vars[n]]);
10388
}
10489
}
105-
if !string_parts[real_vars.len()].is_empty() {
106-
let mut string = String::from(string_parts[real_vars.len()]);
107-
if string.starts_with('/') || string.starts_with('\\') {
108-
string.remove(0);
109-
}
110-
push_comps(&mut sugg, Path::new(&string));
90+
if real_vars.len() < string_parts.len() {
91+
push_comps(&mut sugg, &string_parts[real_vars.len()], true);
11192
}
11293
span_lint_and_sugg(
11394
cx,
@@ -123,7 +104,12 @@ impl<'tcx> LateLintPass<'tcx> for PathFromFormat {
123104
}
124105
}
125106

126-
fn push_comps(string: &mut String, path: &Path) {
107+
fn push_comps(string: &mut String, path: &str, trim_first_slash: bool) {
108+
let mut path = path.to_string();
109+
if (path.starts_with('/') || path.starts_with('\\')) && trim_first_slash {
110+
path.remove(0);
111+
}
112+
let path = Path::new(&path);
127113
let comps = path.components();
128114
for n in comps {
129115
let x = n.as_os_str().to_string_lossy().to_string();
@@ -134,3 +120,16 @@ fn push_comps(string: &mut String, path: &Path) {
134120
}
135121
}
136122
}
123+
124+
fn is_valid_use_case(string: &str, string2: &str) -> bool {
125+
!(
126+
string.is_empty()
127+
|| string.ends_with('/')
128+
|| string.ends_with('\\')
129+
)
130+
|| !(
131+
string2.is_empty()
132+
|| string2.starts_with('/')
133+
|| string2.starts_with('\\')
134+
)
135+
}

‎tests/ui/path_from_format.stderr

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -87,21 +87,5 @@ help: consider using `Path::new()` and `.join()` to make it OS-agnostic and impr
8787
LL | Path::new(base_path2).join("foo").join(base_path1).join("bar");
8888
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8989

90-
error: `format!(..)` used to form `PathBuf`
91-
--> $DIR/path_from_format.rs:16:5
92-
|
93-
LL | PathBuf::from(format!("foo/{base_path1}a/bar"));
94-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
95-
|
96-
= note: if it fits your use case, you may want to consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability
97-
98-
error: `format!(..)` used to form `PathBuf`
99-
--> $DIR/path_from_format.rs:17:5
100-
|
101-
LL | PathBuf::from(format!("foo/a{base_path1}/bar"));
102-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
103-
|
104-
= note: if it fits your use case, you may want to consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability
105-
106-
error: aborting due to 10 previous errors
90+
error: aborting due to 8 previous errors
10791

0 commit comments

Comments
 (0)