Skip to content

Commit c4b4b27

Browse files
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 dac600e commit c4b4b27

File tree

11 files changed

+1315
-64
lines changed

11 files changed

+1315
-64
lines changed

clippy_lints/src/format_args.rs

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
2-
use clippy_utils::macros::FormatParamKind::{Implicit, Named, Numbered, Starred};
2+
use clippy_utils::macros::FormatParamKind::{Implicit, Named, NamedInline, Numbered, Starred};
33
use clippy_utils::macros::{
44
is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
55
};
@@ -115,7 +115,7 @@ declare_clippy_lint! {
115115
/// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`.
116116
#[clippy::version = "1.65.0"]
117117
pub UNINLINED_FORMAT_ARGS,
118-
pedantic,
118+
style,
119119
"using non-inlined variables in `format!` calls"
120120
}
121121

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

160160
pub struct FormatArgs {
161161
msrv: Option<RustcVersion>,
162+
ignore_mixed: bool,
162163
}
163164

164165
impl FormatArgs {
165166
#[must_use]
166-
pub fn new(msrv: Option<RustcVersion>) -> Self {
167-
Self { msrv }
167+
pub fn new(msrv: Option<RustcVersion>, allow_mixed_uninlined_format_args: bool) -> Self {
168+
Self {
169+
msrv,
170+
ignore_mixed: allow_mixed_uninlined_format_args,
171+
}
168172
}
169173
}
170174

@@ -189,7 +193,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
189193
check_to_string_in_format_args(cx, name, arg.param.value);
190194
}
191195
if meets_msrv(self.msrv, msrvs::FORMAT_ARGS_CAPTURE) {
192-
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id);
196+
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id, self.ignore_mixed);
193197
}
194198
}
195199
}
@@ -267,7 +271,13 @@ fn check_unused_format_specifier(cx: &LateContext<'_>, arg: &FormatArg<'_>) {
267271
}
268272
}
269273

270-
fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span, def_id: DefId) {
274+
fn check_uninlined_args(
275+
cx: &LateContext<'_>,
276+
args: &FormatArgsExpn<'_>,
277+
call_site: Span,
278+
def_id: DefId,
279+
ignore_mixed: bool,
280+
) {
271281
if args.format_string.span.from_expansion() {
272282
return;
273283
}
@@ -282,7 +292,7 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
282292
// we cannot remove any other arguments in the format string,
283293
// because the index numbers might be wrong after inlining.
284294
// 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() {
295+
if !args.params().all(|p| check_one_arg(args, &p, &mut fixes, ignore_mixed)) || fixes.is_empty() {
286296
return;
287297
}
288298

@@ -302,7 +312,12 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
302312
);
303313
}
304314

305-
fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut Vec<(Span, String)>) -> bool {
315+
fn check_one_arg(
316+
args: &FormatArgsExpn<'_>,
317+
param: &FormatParam<'_>,
318+
fixes: &mut Vec<(Span, String)>,
319+
ignore_mixed: bool,
320+
) -> bool {
306321
if matches!(param.kind, Implicit | Starred | Named(_) | Numbered)
307322
&& let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind
308323
&& let [segment] = path.segments
@@ -317,8 +332,10 @@ fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut
317332
fixes.push((arg_span, String::new()));
318333
true // successful inlining, continue checking
319334
} else {
320-
// if we can't inline a numbered argument, we can't continue
321-
param.kind != Numbered
335+
// Do not continue inlining (return false) in case
336+
// * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)`
337+
// * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already
338+
param.kind != Numbered && (!ignore_mixed || matches!(param.kind, NamedInline(_)))
322339
}
323340
}
324341

clippy_lints/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -865,7 +865,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
865865
))
866866
});
867867
store.register_late_pass(move |_| Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks));
868-
store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv)));
868+
let allow_mixed_uninlined_format_args = conf.allow_mixed_uninlined_format_args;
869+
store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv, allow_mixed_uninlined_format_args)));
869870
store.register_late_pass(|_| Box::new(trailing_empty_array::TrailingEmptyArray));
870871
store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
871872
store.register_late_pass(|_| Box::new(needless_late_init::NeedlessLateInit));

clippy_lints/src/utils/conf.rs

Lines changed: 4 additions & 0 deletions
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.
Lines changed: 32 additions & 0 deletions
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+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
allow-mixed-uninlined-format-args = false
Lines changed: 182 additions & 0 deletions
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)