Skip to content

Commit 029714a

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 6d0b4e3 commit 029714a

13 files changed

+1361
-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
};
@@ -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

@@ -305,11 +321,23 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
305321
Applicability::MachineApplicable,
306322
if multiline_fix { CompletelyHidden } else { ShowCode },
307323
);
324+
if ignore_mixed {
325+
// Improve lint config discoverability
326+
diag.note_once(
327+
"this lint can also fix mixed format arg inlining if \
328+
`allow-mixed-uninlined-format-args = false` is set in the `clippy.toml` file",
329+
);
330+
}
308331
},
309332
);
310333
}
311334

312-
fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut Vec<(Span, String)>) -> bool {
335+
fn check_one_arg(
336+
args: &FormatArgsExpn<'_>,
337+
param: &FormatParam<'_>,
338+
fixes: &mut Vec<(Span, String)>,
339+
ignore_mixed: bool,
340+
) -> bool {
313341
if matches!(param.kind, Implicit | Starred | Named(_) | Numbered)
314342
&& let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind
315343
&& let [segment] = path.segments
@@ -324,8 +352,10 @@ fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut
324352
fixes.push((arg_span, String::new()));
325353
true // successful inlining, continue checking
326354
} else {
327-
// if we can't inline a numbered argument, we can't continue
328-
param.kind != Numbered
355+
// Do not continue inlining (return false) in case
356+
// * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)`
357+
// * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already
358+
param.kind != Numbered && (!ignore_mixed || matches!(param.kind, NamedInline(_)))
329359
}
330360
}
331361

‎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

+177
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
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='{local_i32}'"
48+
);
49+
println!("{local_i32}");
50+
println!("{fn_arg}");
51+
println!("{local_i32:?}");
52+
println!("{local_i32:#?}");
53+
println!("{local_i32:4}");
54+
println!("{local_i32:04}");
55+
println!("{local_i32:<3}");
56+
println!("{local_i32:#010x}");
57+
println!("{local_f64:.1}");
58+
println!("Hello {} is {local_f64:.local_i32$}", "x");
59+
println!("Hello {local_i32} is {local_f64:.*}", 5);
60+
println!("Hello {local_i32} is {local_f64:.*}", 5);
61+
println!("{local_i32} {local_f64}");
62+
println!("{local_i32}, {}", local_opt.unwrap());
63+
println!("{val}");
64+
println!("{val}");
65+
println!("{} {1}", local_i32, 42);
66+
println!("val='{local_i32}'");
67+
println!("val='{local_i32}'");
68+
println!("val='{local_i32}'");
69+
println!("val='{fn_arg}'");
70+
println!("{local_i32}");
71+
println!("{local_i32:?}");
72+
println!("{local_i32:#?}");
73+
println!("{local_i32:04}");
74+
println!("{local_i32:<3}");
75+
println!("{local_i32:#010x}");
76+
println!("{local_f64:.1}");
77+
println!("{local_i32} {local_i32}");
78+
println!("{local_f64} {local_i32} {local_i32} {local_f64}");
79+
println!("{local_i32} {local_f64}");
80+
println!("{local_f64} {local_i32}");
81+
println!("{local_f64} {local_i32} {local_f64} {local_i32}");
82+
println!("{1} {0}", "str", local_i32);
83+
println!("{local_i32}");
84+
println!("{local_i32:width$}");
85+
println!("{local_i32:width$}");
86+
println!("{local_i32:.prec$}");
87+
println!("{local_i32:.prec$}");
88+
println!("{val:val$}");
89+
println!("{val:val$}");
90+
println!("{val:val$.val$}");
91+
println!("{val: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!("{width:width$}");
99+
println!("{local_i32:width$}");
100+
println!("{width:width$}");
101+
println!("{local_i32:width$}");
102+
println!("{prec:.prec$}");
103+
println!("{local_i32:.prec$}");
104+
println!("{prec:.prec$}");
105+
println!("{local_i32:.prec$}");
106+
println!("{width:width$.prec$}");
107+
println!("{width:width$.prec$}");
108+
println!("{local_f64:width$.prec$}");
109+
println!("{local_f64:width$.prec$} {local_f64} {width} {prec}");
110+
println!(
111+
"{local_i32:width$.prec$} {local_i32:prec$.width$} {width:local_i32$.prec$} {width:prec$.local_i32$} {prec:local_i32$.width$} {prec:width$.local_i32$}",
112+
);
113+
println!(
114+
"{0:1$.2$} {0:2$.1$} {1:0$.2$} {1:2$.0$} {2:0$.1$} {2:1$.0$} {3}",
115+
local_i32,
116+
width,
117+
prec,
118+
1 + 2
119+
);
120+
println!("Width = {local_i32}, value with width = {local_f64:local_i32$}");
121+
println!("{local_i32:width$.prec$}");
122+
println!("{width:width$.prec$}");
123+
println!("{}", format!("{local_i32}"));
124+
my_println!("{}", local_i32);
125+
my_println_args!("{}", local_i32);
126+
127+
// these should NOT be modified by the lint
128+
println!(concat!("nope ", "{}"), local_i32);
129+
println!("val='{local_i32}'");
130+
println!("val='{local_i32 }'");
131+
println!("val='{local_i32 }'"); // with tab
132+
println!("val='{local_i32\n}'");
133+
println!("{}", usize::MAX);
134+
println!("{}", local_opt.unwrap());
135+
println!(
136+
"val='{local_i32
137+
}'"
138+
);
139+
println!(no_param_str!(), local_i32);
140+
141+
println!(
142+
"{val}",
143+
);
144+
println!("{val}");
145+
146+
println!(with_span!("{0} {1}" "{1} {0}"), local_i32, local_f64);
147+
println!("{}", with_span!(span val));
148+
149+
if local_i32 > 0 {
150+
panic!("p1 {local_i32}");
151+
}
152+
if local_i32 > 0 {
153+
panic!("p2 {local_i32}");
154+
}
155+
if local_i32 > 0 {
156+
panic!("p3 {local_i32}");
157+
}
158+
if local_i32 > 0 {
159+
panic!("p4 {local_i32}");
160+
}
161+
}
162+
163+
fn main() {
164+
tester(42);
165+
}
166+
167+
fn _under_msrv() {
168+
#![clippy::msrv = "1.57"]
169+
let local_i32 = 1;
170+
println!("don't expand='{}'", local_i32);
171+
}
172+
173+
fn _meets_msrv() {
174+
#![clippy::msrv = "1.58"]
175+
let local_i32 = 1;
176+
println!("expand='{local_i32}'");
177+
}

0 commit comments

Comments
 (0)