Skip to content

Commit 21f2b6e

Browse files
merelymyselfmerelymyself
committed
suggestion 2
1 parent 63cf5b7 commit 21f2b6e

File tree

3 files changed

+124
-18
lines changed

3 files changed

+124
-18
lines changed

‎clippy_lints/src/path_from_format.rs

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use clippy_utils::diagnostics::span_lint_and_sugg;
1+
use clippy_utils::diagnostics::{span_lint_and_note, 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;
@@ -7,6 +7,7 @@ use rustc_hir::{Expr, ExprKind};
77
use rustc_lint::{LateContext, LateLintPass};
88
use rustc_session::{declare_lint_pass, declare_tool_lint};
99
use rustc_span::sym;
10+
use std::path::Path;
1011

1112
declare_clippy_lint! {
1213
/// ### What it does
@@ -55,12 +56,24 @@ impl<'tcx> LateLintPass<'tcx> for PathFromFormat {
5556
let order_of_real_vars: Vec<usize> = format_args.formatters.iter().map(|(x, _)| *x).collect();
5657
let mut sugg = String::new();
5758
for n in 0..real_vars.len() {
59+
if (!string_parts[n].is_empty() && !(string_parts[n].ends_with('/') || string_parts[n].ends_with('\\'))) || (!string_parts[n+1].is_empty() && (!(string_parts[n+1].starts_with('/') || string_parts[n+1].starts_with('\\')))) {
60+
span_lint_and_note(
61+
cx,
62+
PATH_FROM_FORMAT,
63+
expr.span,
64+
"`format!(..)` used to form `PathBuf`",
65+
None,
66+
"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.",
67+
);
68+
return;
69+
}
5870
if n == 0 {
5971
if string_parts[0].is_empty() {
6072
sugg = format!("Path::new({})", real_vars[order_of_real_vars[0]]);
6173
}
6274
else {
63-
sugg = format!("Path::new(\"{}\").join({y})", string_parts[0], y = real_vars[order_of_real_vars[0]]);
75+
push_comps(&mut sugg, Path::new(string_parts[0]));
76+
sugg.push_str(&format!(".join({})", real_vars[order_of_real_vars[0]]));
6477
}
6578
}
6679
else if string_parts[n].is_empty() {
@@ -71,26 +84,40 @@ impl<'tcx> LateLintPass<'tcx> for PathFromFormat {
7184
if string.starts_with('/') || string.starts_with('\\') {
7285
string.remove(0);
7386
}
74-
sugg = format!("{sugg}.join(\"{}\").join({y})", string, y = real_vars[order_of_real_vars[n]]);
87+
push_comps(&mut sugg, Path::new(&string));
88+
sugg.push_str(&format!(".join({})", real_vars[order_of_real_vars[n]]));
7589
}
7690
}
7791
if !string_parts[real_vars.len()].is_empty() {
7892
let mut string = String::from(string_parts[real_vars.len()]);
7993
if string.starts_with('/') || string.starts_with('\\') {
8094
string.remove(0);
8195
}
82-
sugg = format!("{sugg}.join(\"{}\")", string);
96+
push_comps(&mut sugg, Path::new(&string));
8397
}
8498
span_lint_and_sugg(
8599
cx,
86100
PATH_FROM_FORMAT,
87101
expr.span,
88102
"`format!(..)` used to form `PathBuf`",
89-
"consider using `.join()` to avoid the extra allocation",
103+
"consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.",
90104
sugg,
91105
Applicability::MaybeIncorrect,
92106
);
93107
}
94108
}
95109
}
96110
}
111+
112+
fn push_comps(string: &mut String, path: &Path) {
113+
let comps = path.components();
114+
for n in comps {
115+
let x = n.as_os_str().to_string_lossy().to_string();
116+
if string.is_empty() {
117+
string.push_str(&format!("Path::new(\"{x}\")"));
118+
}
119+
else {
120+
string.push_str(&format!(".join(\"{x}\")"));
121+
}
122+
}
123+
}

‎tests/ui/path_from_format.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,15 @@ use std::path::PathBuf;
44

55
fn main() {
66
let mut base_path1 = "";
7+
let mut base_path2 = "";
78
PathBuf::from(format!("{}/foo/bar", base_path1));
89
PathBuf::from(format!("/foo/bar/{}", base_path1));
910
PathBuf::from(format!("/foo/{}/bar", base_path1));
1011
PathBuf::from(format!("foo/{}/bar", base_path1));
11-
PathBuf::from(format!("foo/{base_path1}/bar"));
12+
PathBuf::from(format!("foo/foooo/{base_path1}/bar/barrr"));
13+
PathBuf::from(format!("foo/foooo/{base_path1}/bar/barrr"));
14+
PathBuf::from(format!("foo/foooo/{base_path1}/bar/barrr/{base_path2}"));
15+
PathBuf::from(format!("{base_path2}/foo/{base_path1}/bar"));
16+
PathBuf::from(format!("foo/{base_path1}a/bar"));
17+
PathBuf::from(format!("foo/a{base_path1}/bar"));
1218
}

‎tests/ui/path_from_format.stderr

Lines changed: 85 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,107 @@
11
error: `format!(..)` used to form `PathBuf`
2-
--> $DIR/path_from_format.rs:7:5
2+
--> $DIR/path_from_format.rs:8:5
33
|
44
LL | PathBuf::from(format!("{}/foo/bar", base_path1));
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new(base_path1).join("foo/bar")`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::path-from-format` implied by `-D warnings`
8+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
9+
|
10+
LL | Path::new(base_path1).join("foo").join("bar");
11+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
812

913
error: `format!(..)` used to form `PathBuf`
10-
--> $DIR/path_from_format.rs:8:5
14+
--> $DIR/path_from_format.rs:9:5
1115
|
1216
LL | PathBuf::from(format!("/foo/bar/{}", base_path1));
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new("/foo/bar/").join(base_path1)`
17+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
18+
|
19+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
20+
|
21+
LL | Path::new("/").join("foo").join("bar").join(base_path1);
22+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1423

1524
error: `format!(..)` used to form `PathBuf`
16-
--> $DIR/path_from_format.rs:9:5
25+
--> $DIR/path_from_format.rs:10:5
1726
|
1827
LL | PathBuf::from(format!("/foo/{}/bar", base_path1));
19-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new("/foo/").join(base_path1).join("bar")`
28+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
|
30+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
31+
|
32+
LL | Path::new("/").join("foo").join(base_path1).join("bar");
33+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2034

2135
error: `format!(..)` used to form `PathBuf`
22-
--> $DIR/path_from_format.rs:10:5
36+
--> $DIR/path_from_format.rs:11:5
2337
|
2438
LL | PathBuf::from(format!("foo/{}/bar", base_path1));
25-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new("foo/").join(base_path1).join("bar")`
39+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
40+
|
41+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
42+
|
43+
LL | Path::new("foo").join(base_path1).join("bar");
44+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2645

2746
error: `format!(..)` used to form `PathBuf`
28-
--> $DIR/path_from_format.rs:11:5
47+
--> $DIR/path_from_format.rs:12:5
48+
|
49+
LL | PathBuf::from(format!("foo/foooo/{base_path1}/bar/barrr"));
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
51+
|
52+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
53+
|
54+
LL | Path::new("foo").join("foooo").join(base_path1).join("bar").join("barrr");
55+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
56+
57+
error: `format!(..)` used to form `PathBuf`
58+
--> $DIR/path_from_format.rs:13:5
59+
|
60+
LL | PathBuf::from(format!("foo/foooo/{base_path1}/bar/barrr"));
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
62+
|
63+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
64+
|
65+
LL | Path::new("foo").join("foooo").join(base_path1).join("bar").join("barrr");
66+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
67+
68+
error: `format!(..)` used to form `PathBuf`
69+
--> $DIR/path_from_format.rs:14:5
70+
|
71+
LL | PathBuf::from(format!("foo/foooo/{base_path1}/bar/barrr/{base_path2}"));
72+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
73+
|
74+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
75+
|
76+
LL | Path::new("foo").join("foooo").join(base_path1).join("bar").join("barrr").join(base_path2);
77+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
78+
79+
error: `format!(..)` used to form `PathBuf`
80+
--> $DIR/path_from_format.rs:15:5
81+
|
82+
LL | PathBuf::from(format!("{base_path2}/foo/{base_path1}/bar"));
83+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
84+
|
85+
help: consider using `Path::new()` and `.join()` to make it OS-agnostic and improve code readability.
86+
|
87+
LL | Path::new(base_path2).join("foo").join(base_path1).join("bar");
88+
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
89+
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+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29103
|
30-
LL | PathBuf::from(format!("foo/{base_path1}/bar"));
31-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.join()` to avoid the extra allocation: `Path::new("foo/").join(base_path1).join("bar")`
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.
32105

33-
error: aborting due to 5 previous errors
106+
error: aborting due to 10 previous errors
34107

0 commit comments

Comments
 (0)