Skip to content

Commit 1207480

Browse files
committed
Auto merge of #9865 - nyurik:allow-mixed, r=xFrednet
Add allow-mixed-uninlined-format-args config Implement `allow-mixed-uninlined-format-args` config param to change the behavior of the `uninlined_format_args` lint. Now it is a part of `style` per [Zulip chat](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/.60uninlined_format_args.60.20category), and won't propose inlining in case of a mixed usage, e.g. `print!("{} {}", var, 1+2)`. If the user sets `allow-mixed-uninlined-format-args` config param to `false`, the lint would behave like it did before -- proposing to inline args even in the mixed case. --- changelog: [`uninlined_format_args`]: Added a new config `allow-mixed-uninlined-format-args` to allow the lint, if only some arguments can be inlined [#9865](#9865) changelog: Moved [`uninlined_format_args`] to `style` (Now warn-by-default) [#9865](#9865)
2 parents c8eba8e + ab576af commit 1207480

File tree

10 files changed

+156
-70
lines changed

10 files changed

+156
-70
lines changed

clippy_lints/src/format_args.rs

+39-16
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::is_diag_trait_item;
3-
use clippy_utils::macros::FormatParamKind::{Implicit, Named, Numbered, Starred};
3+
use clippy_utils::macros::FormatParamKind::{Implicit, Named, NamedInline, Numbered, Starred};
44
use clippy_utils::macros::{
55
is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
66
};
@@ -106,19 +106,25 @@ declare_clippy_lint! {
106106
/// format!("{var:.prec$}");
107107
/// ```
108108
///
109-
/// ### Known Problems
110-
///
111-
/// There may be a false positive if the format string is expanded from certain proc macros:
112-
///
113-
/// ```ignore
114-
/// println!(indoc!("{}"), var);
109+
/// If allow-mixed-uninlined-format-args is set to false in clippy.toml,
110+
/// the following code will also trigger the lint:
111+
/// ```rust
112+
/// # let var = 42;
113+
/// format!("{} {}", var, 1+2);
114+
/// ```
115+
/// Use instead:
116+
/// ```rust
117+
/// # let var = 42;
118+
/// format!("{var} {}", 1+2);
115119
/// ```
116120
///
121+
/// ### Known Problems
122+
///
117123
/// If a format string contains a numbered argument that cannot be inlined
118124
/// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`.
119125
#[clippy::version = "1.65.0"]
120126
pub UNINLINED_FORMAT_ARGS,
121-
pedantic,
127+
style,
122128
"using non-inlined variables in `format!` calls"
123129
}
124130

@@ -162,12 +168,16 @@ impl_lint_pass!(FormatArgs => [
162168

163169
pub struct FormatArgs {
164170
msrv: Msrv,
171+
ignore_mixed: bool,
165172
}
166173

167174
impl FormatArgs {
168175
#[must_use]
169-
pub fn new(msrv: Msrv) -> Self {
170-
Self { msrv }
176+
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
177+
Self {
178+
msrv,
179+
ignore_mixed: allow_mixed_uninlined_format_args,
180+
}
171181
}
172182
}
173183

@@ -192,7 +202,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
192202
check_to_string_in_format_args(cx, name, arg.param.value);
193203
}
194204
if self.msrv.meets(msrvs::FORMAT_ARGS_CAPTURE) {
195-
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id);
205+
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id, self.ignore_mixed);
196206
}
197207
}
198208
}
@@ -270,7 +280,13 @@ fn check_unused_format_specifier(cx: &LateContext<'_>, arg: &FormatArg<'_>) {
270280
}
271281
}
272282

273-
fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span, def_id: DefId) {
283+
fn check_uninlined_args(
284+
cx: &LateContext<'_>,
285+
args: &FormatArgsExpn<'_>,
286+
call_site: Span,
287+
def_id: DefId,
288+
ignore_mixed: bool,
289+
) {
274290
if args.format_string.span.from_expansion() {
275291
return;
276292
}
@@ -285,7 +301,7 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
285301
// we cannot remove any other arguments in the format string,
286302
// because the index numbers might be wrong after inlining.
287303
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
288-
if !args.params().all(|p| check_one_arg(args, &p, &mut fixes)) || fixes.is_empty() {
304+
if !args.params().all(|p| check_one_arg(args, &p, &mut fixes, ignore_mixed)) || fixes.is_empty() {
289305
return;
290306
}
291307

@@ -309,7 +325,12 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
309325
);
310326
}
311327

312-
fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut Vec<(Span, String)>) -> bool {
328+
fn check_one_arg(
329+
args: &FormatArgsExpn<'_>,
330+
param: &FormatParam<'_>,
331+
fixes: &mut Vec<(Span, String)>,
332+
ignore_mixed: bool,
333+
) -> bool {
313334
if matches!(param.kind, Implicit | Starred | Named(_) | Numbered)
314335
&& let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind
315336
&& let [segment] = path.segments
@@ -324,8 +345,10 @@ fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut
324345
fixes.push((arg_span, String::new()));
325346
true // successful inlining, continue checking
326347
} else {
327-
// if we can't inline a numbered argument, we can't continue
328-
param.kind != Numbered
348+
// Do not continue inlining (return false) in case
349+
// * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)`
350+
// * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already
351+
param.kind != Numbered && (!ignore_mixed || matches!(param.kind, NamedInline(_)))
329352
}
330353
}
331354

clippy_lints/src/lib.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -828,7 +828,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
828828
))
829829
});
830830
store.register_late_pass(move |_| Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks));
831-
store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv())));
831+
let allow_mixed_uninlined = conf.allow_mixed_uninlined_format_args;
832+
store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv(), allow_mixed_uninlined)));
832833
store.register_late_pass(|_| Box::new(trailing_empty_array::TrailingEmptyArray));
833834
store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
834835
store.register_late_pass(|_| Box::new(needless_late_init::NeedlessLateInit));

clippy_lints/src/utils/conf.rs

+4
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,10 @@ define_Conf! {
402402
/// A list of paths to types that should be treated like `Arc`, i.e. ignored but
403403
/// for the generic parameters for determining interior mutability
404404
(ignore_interior_mutability: Vec<String> = Vec::from(["bytes::Bytes".into()])),
405+
/// Lint: UNINLINED_FORMAT_ARGS.
406+
///
407+
/// Whether to allow mixed uninlined format args, e.g. `format!("{} {}", a, foo.bar)`
408+
(allow_mixed_uninlined_format_args: bool = true),
405409
}
406410

407411
/// Search for the configuration file.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
allow-mixed-uninlined-format-args = false
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// run-rustfix
2+
#![warn(clippy::uninlined_format_args)]
3+
4+
fn main() {
5+
let local_i32 = 1;
6+
let local_f64 = 2.0;
7+
let local_opt: Option<i32> = Some(3);
8+
9+
println!("val='{local_i32}'");
10+
println!("Hello x is {local_f64:.local_i32$}");
11+
println!("Hello {local_i32} is {local_f64:.*}", 5);
12+
println!("Hello {local_i32} is {local_f64:.*}", 5);
13+
println!("{local_i32}, {}", local_opt.unwrap());
14+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// run-rustfix
2+
#![warn(clippy::uninlined_format_args)]
3+
4+
fn main() {
5+
let local_i32 = 1;
6+
let local_f64 = 2.0;
7+
let local_opt: Option<i32> = Some(3);
8+
9+
println!("val='{}'", local_i32);
10+
println!("Hello {} is {:.*}", "x", local_i32, local_f64);
11+
println!("Hello {} is {:.*}", local_i32, 5, local_f64);
12+
println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
13+
println!("{}, {}", local_i32, local_opt.unwrap());
14+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
error: variables can be used directly in the `format!` string
2+
--> $DIR/uninlined_format_args.rs:9:5
3+
|
4+
LL | println!("val='{}'", local_i32);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= note: `-D clippy::uninlined-format-args` implied by `-D warnings`
8+
help: change this to
9+
|
10+
LL - println!("val='{}'", local_i32);
11+
LL + println!("val='{local_i32}'");
12+
|
13+
14+
error: literal with an empty format string
15+
--> $DIR/uninlined_format_args.rs:10:35
16+
|
17+
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
18+
| ^^^
19+
|
20+
= note: `-D clippy::print-literal` implied by `-D warnings`
21+
help: try this
22+
|
23+
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
24+
LL + println!("Hello x is {:.*}", local_i32, local_f64);
25+
|
26+
27+
error: variables can be used directly in the `format!` string
28+
--> $DIR/uninlined_format_args.rs:10:5
29+
|
30+
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
31+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
32+
|
33+
help: change this to
34+
|
35+
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
36+
LL + println!("Hello {} is {local_f64:.local_i32$}", "x");
37+
|
38+
39+
error: variables can be used directly in the `format!` string
40+
--> $DIR/uninlined_format_args.rs:11:5
41+
|
42+
LL | println!("Hello {} is {:.*}", local_i32, 5, local_f64);
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44+
|
45+
help: change this to
46+
|
47+
LL - println!("Hello {} is {:.*}", local_i32, 5, local_f64);
48+
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
49+
|
50+
51+
error: variables can be used directly in the `format!` string
52+
--> $DIR/uninlined_format_args.rs:12:5
53+
|
54+
LL | println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
56+
|
57+
help: change this to
58+
|
59+
LL - println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
60+
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
61+
|
62+
63+
error: variables can be used directly in the `format!` string
64+
--> $DIR/uninlined_format_args.rs:13:5
65+
|
66+
LL | println!("{}, {}", local_i32, local_opt.unwrap());
67+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
68+
|
69+
help: change this to
70+
|
71+
LL - println!("{}, {}", local_i32, local_opt.unwrap());
72+
LL + println!("{local_i32}, {}", local_opt.unwrap());
73+
|
74+
75+
error: aborting due to 6 previous errors
76+

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

+1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of
22
allow-dbg-in-tests
33
allow-expect-in-tests
4+
allow-mixed-uninlined-format-args
45
allow-print-in-tests
56
allow-unwrap-in-tests
67
allowed-scripts

tests/ui/uninlined_format_args.fixed

+4-4
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,11 @@ fn tester(fn_arg: i32) {
5454
println!("{local_i32:<3}");
5555
println!("{local_i32:#010x}");
5656
println!("{local_f64:.1}");
57-
println!("Hello {} is {local_f64:.local_i32$}", "x");
58-
println!("Hello {local_i32} is {local_f64:.*}", 5);
59-
println!("Hello {local_i32} is {local_f64:.*}", 5);
57+
println!("Hello {} is {:.*}", "x", local_i32, local_f64);
58+
println!("Hello {} is {:.*}", local_i32, 5, local_f64);
59+
println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
6060
println!("{local_i32} {local_f64}");
61-
println!("{local_i32}, {}", local_opt.unwrap());
61+
println!("{}, {}", local_i32, local_opt.unwrap());
6262
println!("{val}");
6363
println!("{val}");
6464
println!("{} {1}", local_i32, 42);

tests/ui/uninlined_format_args.stderr

+1-49
Original file line numberDiff line numberDiff line change
@@ -177,42 +177,6 @@ LL - println!("{:.1}", local_f64);
177177
LL + println!("{local_f64:.1}");
178178
|
179179

180-
error: variables can be used directly in the `format!` string
181-
--> $DIR/uninlined_format_args.rs:59:5
182-
|
183-
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
184-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
185-
|
186-
help: change this to
187-
|
188-
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
189-
LL + println!("Hello {} is {local_f64:.local_i32$}", "x");
190-
|
191-
192-
error: variables can be used directly in the `format!` string
193-
--> $DIR/uninlined_format_args.rs:60:5
194-
|
195-
LL | println!("Hello {} is {:.*}", local_i32, 5, local_f64);
196-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
197-
|
198-
help: change this to
199-
|
200-
LL - println!("Hello {} is {:.*}", local_i32, 5, local_f64);
201-
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
202-
|
203-
204-
error: variables can be used directly in the `format!` string
205-
--> $DIR/uninlined_format_args.rs:61:5
206-
|
207-
LL | println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
208-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
209-
|
210-
help: change this to
211-
|
212-
LL - println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
213-
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
214-
|
215-
216180
error: variables can be used directly in the `format!` string
217181
--> $DIR/uninlined_format_args.rs:62:5
218182
|
@@ -225,18 +189,6 @@ LL - println!("{} {}", local_i32, local_f64);
225189
LL + println!("{local_i32} {local_f64}");
226190
|
227191

228-
error: variables can be used directly in the `format!` string
229-
--> $DIR/uninlined_format_args.rs:63:5
230-
|
231-
LL | println!("{}, {}", local_i32, local_opt.unwrap());
232-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
233-
|
234-
help: change this to
235-
|
236-
LL - println!("{}, {}", local_i32, local_opt.unwrap());
237-
LL + println!("{local_i32}, {}", local_opt.unwrap());
238-
|
239-
240192
error: variables can be used directly in the `format!` string
241193
--> $DIR/uninlined_format_args.rs:64:5
242194
|
@@ -904,5 +856,5 @@ LL - println!("expand='{}'", local_i32);
904856
LL + println!("expand='{local_i32}'");
905857
|
906858

907-
error: aborting due to 76 previous errors
859+
error: aborting due to 72 previous errors
908860

0 commit comments

Comments
 (0)