Skip to content

Commit c7a0759

Browse files
nyuriknyurik
committed
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`, 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`, then it would behave like before, proposing to inline args even in the mixed case.
1 parent 08a80d3 commit c7a0759

13 files changed

+1337
-70
lines changed

‎clippy_lints/src/format_args.rs

+46-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
};
@@ -103,19 +103,25 @@ declare_clippy_lint! {
103103
/// format!("{var:.prec$}");
104104
/// ```
105105
///
106-
/// ### Known Problems
107-
///
108-
/// There may be a false positive if the format string is expanded from certain proc macros:
109-
///
110-
/// ```ignore
111-
/// println!(indoc!("{}"), var);
106+
/// If allow-mixed-uninlined-format-args is set to false in clippy.toml,
107+
/// the following code will also trigger the lint:
108+
/// ```rust
109+
/// # let var = 42;
110+
/// format!("{} {}", var, 1+2);
111+
/// ```
112+
/// Use instead:
113+
/// ```rust
114+
/// # let var = 42;
115+
/// format!("{var} {}", 1+2);
112116
/// ```
113117
///
118+
/// ### Known Problems
119+
///
114120
/// If a format string contains a numbered argument that cannot be inlined
115121
/// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`.
116122
#[clippy::version = "1.65.0"]
117123
pub UNINLINED_FORMAT_ARGS,
118-
pedantic,
124+
style,
119125
"using non-inlined variables in `format!` calls"
120126
}
121127

@@ -159,12 +165,16 @@ impl_lint_pass!(FormatArgs => [
159165

160166
pub struct FormatArgs {
161167
msrv: Msrv,
168+
ignore_mixed: bool,
162169
}
163170

164171
impl FormatArgs {
165172
#[must_use]
166-
pub fn new(msrv: Msrv) -> Self {
167-
Self { msrv }
173+
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
174+
Self {
175+
msrv,
176+
ignore_mixed: allow_mixed_uninlined_format_args,
177+
}
168178
}
169179
}
170180

@@ -189,7 +199,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
189199
check_to_string_in_format_args(cx, name, arg.param.value);
190200
}
191201
if self.msrv.meets(msrvs::FORMAT_ARGS_CAPTURE) {
192-
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id);
202+
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id, self.ignore_mixed);
193203
}
194204
}
195205
}
@@ -267,7 +277,13 @@ fn check_unused_format_specifier(cx: &LateContext<'_>, arg: &FormatArg<'_>) {
267277
}
268278
}
269279

270-
fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span, def_id: DefId) {
280+
fn check_uninlined_args(
281+
cx: &LateContext<'_>,
282+
args: &FormatArgsExpn<'_>,
283+
call_site: Span,
284+
def_id: DefId,
285+
ignore_mixed: bool,
286+
) {
271287
if args.format_string.span.from_expansion() {
272288
return;
273289
}
@@ -282,7 +298,7 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
282298
// we cannot remove any other arguments in the format string,
283299
// because the index numbers might be wrong after inlining.
284300
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
285-
if !args.params().all(|p| check_one_arg(args, &p, &mut fixes)) || fixes.is_empty() {
301+
if !args.params().all(|p| check_one_arg(args, &p, &mut fixes, ignore_mixed)) || fixes.is_empty() {
286302
return;
287303
}
288304

@@ -298,11 +314,23 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
298314
"variables can be used directly in the `format!` string",
299315
|diag| {
300316
diag.multipart_suggestion("change this to", fixes, Applicability::MachineApplicable);
317+
if ignore_mixed {
318+
// Improve lint config discoverability
319+
diag.note_once(
320+
"this lint can also fix mixed format arg inlining if \
321+
`allow-mixed-uninlined-format-args = false` is set in the `clippy.toml` file",
322+
);
323+
}
301324
},
302325
);
303326
}
304327

305-
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 {
306334
if matches!(param.kind, Implicit | Starred | Named(_) | Numbered)
307335
&& let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind
308336
&& let [segment] = path.segments
@@ -317,8 +345,10 @@ fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut
317345
fixes.push((arg_span, String::new()));
318346
true // successful inlining, continue checking
319347
} else {
320-
// if we can't inline a numbered argument, we can't continue
321-
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(_)))
322352
}
323353
}
324354

‎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.

‎tests/ui-toml/allow_mixed_uninlined_format_args/auxiliary/proc_macro_with_span.rs

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// compile-flags: --emit=link
2+
// no-prefer-dynamic
3+
4+
#![crate_type = "proc-macro"]
5+
6+
extern crate proc_macro;
7+
8+
use proc_macro::{token_stream::IntoIter, Group, Span, TokenStream, TokenTree};
9+
10+
#[proc_macro]
11+
pub fn with_span(input: TokenStream) -> TokenStream {
12+
let mut iter = input.into_iter();
13+
let span = iter.next().unwrap().span();
14+
let mut res = TokenStream::new();
15+
write_with_span(span, iter, &mut res);
16+
res
17+
}
18+
19+
fn write_with_span(s: Span, input: IntoIter, out: &mut TokenStream) {
20+
for mut tt in input {
21+
if let TokenTree::Group(g) = tt {
22+
let mut stream = TokenStream::new();
23+
write_with_span(s, g.stream().into_iter(), &mut stream);
24+
let mut group = Group::new(g.delimiter(), stream);
25+
group.set_span(s);
26+
out.extend([TokenTree::Group(group)]);
27+
} else {
28+
tt.set_span(s);
29+
out.extend([tt]);
30+
}
31+
}
32+
}

‎tests/ui-toml/allow_mixed_uninlined_format_args/clippy.toml

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
allow-mixed-uninlined-format-args = false

‎tests/ui-toml/allow_mixed_uninlined_format_args/uninlined_format_args.fixed

+182
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
// aux-build:proc_macro_with_span.rs
2+
// run-rustfix
3+
#![feature(custom_inner_attributes)]
4+
#![warn(clippy::uninlined_format_args)]
5+
#![allow(named_arguments_used_positionally, unused_imports, unused_macros, unused_variables)]
6+
#![allow(clippy::eq_op, clippy::format_in_format_args, clippy::print_literal)]
7+
8+
extern crate proc_macro_with_span;
9+
use proc_macro_with_span::with_span;
10+
11+
macro_rules! no_param_str {
12+
() => {
13+
"{}"
14+
};
15+
}
16+
17+
macro_rules! my_println {
18+
($($args:tt),*) => {{
19+
println!($($args),*)
20+
}};
21+
}
22+
23+
macro_rules! my_println_args {
24+
($($args:tt),*) => {{
25+
println!("foo: {}", format_args!($($args),*))
26+
}};
27+
}
28+
29+
fn tester(fn_arg: i32) {
30+
let local_i32 = 1;
31+
let local_f64 = 2.0;
32+
let local_opt: Option<i32> = Some(3);
33+
let width = 4;
34+
let prec = 5;
35+
let val = 6;
36+
37+
// make sure this file hasn't been corrupted with tabs converted to spaces
38+
// let _ = ' '; // <- this is a single tab character
39+
let _: &[u8; 3] = b" "; // <- <tab><space><tab>
40+
41+
println!("val='{local_i32}'");
42+
println!("val='{local_i32}'"); // 3 spaces
43+
println!("val='{local_i32}'"); // tab
44+
println!("val='{local_i32}'"); // space+tab
45+
println!("val='{local_i32}'"); // tab+space
46+
println!(
47+
"val='{
48+
}'",
49+
local_i32
50+
);
51+
println!("{local_i32}");
52+
println!("{fn_arg}");
53+
println!("{local_i32:?}");
54+
println!("{local_i32:#?}");
55+
println!("{local_i32:4}");
56+
println!("{local_i32:04}");
57+
println!("{local_i32:<3}");
58+
println!("{local_i32:#010x}");
59+
println!("{local_f64:.1}");
60+
println!("Hello {} is {local_f64:.local_i32$}", "x");
61+
println!("Hello {local_i32} is {local_f64:.*}", 5);
62+
println!("Hello {local_i32} is {local_f64:.*}", 5);
63+
println!("{local_i32} {local_f64}");
64+
println!("{local_i32}, {}", local_opt.unwrap());
65+
println!("{val}");
66+
println!("{val}");
67+
println!("{} {1}", local_i32, 42);
68+
println!("val='{local_i32}'");
69+
println!("val='{local_i32}'");
70+
println!("val='{local_i32}'");
71+
println!("val='{fn_arg}'");
72+
println!("{local_i32}");
73+
println!("{local_i32:?}");
74+
println!("{local_i32:#?}");
75+
println!("{local_i32:04}");
76+
println!("{local_i32:<3}");
77+
println!("{local_i32:#010x}");
78+
println!("{local_f64:.1}");
79+
println!("{local_i32} {local_i32}");
80+
println!("{local_f64} {local_i32} {local_i32} {local_f64}");
81+
println!("{local_i32} {local_f64}");
82+
println!("{local_f64} {local_i32}");
83+
println!("{local_f64} {local_i32} {local_f64} {local_i32}");
84+
println!("{1} {0}", "str", local_i32);
85+
println!("{local_i32}");
86+
println!("{local_i32:width$}");
87+
println!("{local_i32:width$}");
88+
println!("{local_i32:.prec$}");
89+
println!("{local_i32:.prec$}");
90+
println!("{val:val$}");
91+
println!("{val:val$}");
92+
println!("{val:val$.val$}");
93+
println!("{val:val$.val$}");
94+
println!("{val:val$.val$}");
95+
println!("{val:val$.val$}");
96+
println!("{val:val$.val$}");
97+
println!("{val:val$.val$}");
98+
println!("{val:val$.val$}");
99+
println!("{val:val$.val$}");
100+
println!("{width:width$}");
101+
println!("{local_i32:width$}");
102+
println!("{width:width$}");
103+
println!("{local_i32:width$}");
104+
println!("{prec:.prec$}");
105+
println!("{local_i32:.prec$}");
106+
println!("{prec:.prec$}");
107+
println!("{local_i32:.prec$}");
108+
println!("{width:width$.prec$}");
109+
println!("{width:width$.prec$}");
110+
println!("{local_f64:width$.prec$}");
111+
println!("{local_f64:width$.prec$} {local_f64} {width} {prec}");
112+
println!(
113+
"{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$}",
114+
local_i32, width, prec,
115+
);
116+
println!(
117+
"{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$} {3}",
118+
local_i32,
119+
width,
120+
prec,
121+
1 + 2
122+
);
123+
println!("Width = {local_i32}, value with width = {local_f64:local_i32$}");
124+
println!("{local_i32:width$.prec$}");
125+
println!("{width:width$.prec$}");
126+
println!("{}", format!("{local_i32}"));
127+
my_println!("{}", local_i32);
128+
my_println_args!("{}", local_i32);
129+
130+
// these should NOT be modified by the lint
131+
println!(concat!("nope ", "{}"), local_i32);
132+
println!("val='{local_i32}'");
133+
println!("val='{local_i32 }'");
134+
println!("val='{local_i32 }'"); // with tab
135+
println!("val='{local_i32\n}'");
136+
println!("{}", usize::MAX);
137+
println!("{}", local_opt.unwrap());
138+
println!(
139+
"val='{local_i32
140+
}'"
141+
);
142+
println!(no_param_str!(), local_i32);
143+
144+
println!(
145+
"{}",
146+
// comment with a comma , in it
147+
val,
148+
);
149+
println!("{val}");
150+
151+
println!(with_span!("{0} {1}" "{1} {0}"), local_i32, local_f64);
152+
println!("{}", with_span!(span val));
153+
154+
if local_i32 > 0 {
155+
panic!("p1 {local_i32}");
156+
}
157+
if local_i32 > 0 {
158+
panic!("p2 {local_i32}");
159+
}
160+
if local_i32 > 0 {
161+
panic!("p3 {local_i32}");
162+
}
163+
if local_i32 > 0 {
164+
panic!("p4 {local_i32}");
165+
}
166+
}
167+
168+
fn main() {
169+
tester(42);
170+
}
171+
172+
fn _under_msrv() {
173+
#![clippy::msrv = "1.57"]
174+
let local_i32 = 1;
175+
println!("don't expand='{}'", local_i32);
176+
}
177+
178+
fn _meets_msrv() {
179+
#![clippy::msrv = "1.58"]
180+
let local_i32 = 1;
181+
println!("expand='{local_i32}'");
182+
}

0 commit comments

Comments
 (0)