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](https://github.com/rust-lang/rust-clippy/pull/9865)
changelog: Moved [`uninlined_format_args`] to `style` (Now warn-by-default)
[#9865](https://github.com/rust-lang/rust-clippy/pull/9865)
This commit is contained in:
bors 2022-11-28 17:22:36 +00:00
commit 12074808c7
10 changed files with 156 additions and 70 deletions

View File

@ -1,6 +1,6 @@
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::is_diag_trait_item;
use clippy_utils::macros::FormatParamKind::{Implicit, Named, Numbered, Starred};
use clippy_utils::macros::FormatParamKind::{Implicit, Named, NamedInline, Numbered, Starred};
use clippy_utils::macros::{
is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
};
@ -106,19 +106,25 @@ declare_clippy_lint! {
/// format!("{var:.prec$}");
/// ```
///
/// ### Known Problems
///
/// There may be a false positive if the format string is expanded from certain proc macros:
///
/// ```ignore
/// println!(indoc!("{}"), var);
/// If allow-mixed-uninlined-format-args is set to false in clippy.toml,
/// the following code will also trigger the lint:
/// ```rust
/// # let var = 42;
/// format!("{} {}", var, 1+2);
/// ```
/// Use instead:
/// ```rust
/// # let var = 42;
/// format!("{var} {}", 1+2);
/// ```
///
/// ### Known Problems
///
/// If a format string contains a numbered argument that cannot be inlined
/// nothing will be suggested, e.g. `println!("{0}={1}", var, 1+2)`.
#[clippy::version = "1.65.0"]
pub UNINLINED_FORMAT_ARGS,
pedantic,
style,
"using non-inlined variables in `format!` calls"
}
@ -162,12 +168,16 @@ impl_lint_pass!(FormatArgs => [
pub struct FormatArgs {
msrv: Msrv,
ignore_mixed: bool,
}
impl FormatArgs {
#[must_use]
pub fn new(msrv: Msrv) -> Self {
Self { msrv }
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
Self {
msrv,
ignore_mixed: allow_mixed_uninlined_format_args,
}
}
}
@ -192,7 +202,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
check_to_string_in_format_args(cx, name, arg.param.value);
}
if self.msrv.meets(msrvs::FORMAT_ARGS_CAPTURE) {
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id);
check_uninlined_args(cx, &format_args, outermost_expn_data.call_site, macro_def_id, self.ignore_mixed);
}
}
}
@ -270,7 +280,13 @@ fn check_unused_format_specifier(cx: &LateContext<'_>, arg: &FormatArg<'_>) {
}
}
fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_site: Span, def_id: DefId) {
fn check_uninlined_args(
cx: &LateContext<'_>,
args: &FormatArgsExpn<'_>,
call_site: Span,
def_id: DefId,
ignore_mixed: bool,
) {
if args.format_string.span.from_expansion() {
return;
}
@ -285,7 +301,7 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
// we cannot remove any other arguments in the format string,
// because the index numbers might be wrong after inlining.
// Example of an un-inlinable format: print!("{}{1}", foo, 2)
if !args.params().all(|p| check_one_arg(args, &p, &mut fixes)) || fixes.is_empty() {
if !args.params().all(|p| check_one_arg(args, &p, &mut fixes, ignore_mixed)) || fixes.is_empty() {
return;
}
@ -309,7 +325,12 @@ fn check_uninlined_args(cx: &LateContext<'_>, args: &FormatArgsExpn<'_>, call_si
);
}
fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut Vec<(Span, String)>) -> bool {
fn check_one_arg(
args: &FormatArgsExpn<'_>,
param: &FormatParam<'_>,
fixes: &mut Vec<(Span, String)>,
ignore_mixed: bool,
) -> bool {
if matches!(param.kind, Implicit | Starred | Named(_) | Numbered)
&& let ExprKind::Path(QPath::Resolved(None, path)) = param.value.kind
&& let [segment] = path.segments
@ -324,8 +345,10 @@ fn check_one_arg(args: &FormatArgsExpn<'_>, param: &FormatParam<'_>, fixes: &mut
fixes.push((arg_span, String::new()));
true // successful inlining, continue checking
} else {
// if we can't inline a numbered argument, we can't continue
param.kind != Numbered
// Do not continue inlining (return false) in case
// * if we can't inline a numbered argument, e.g. `print!("{0} ...", foo.bar, ...)`
// * if allow_mixed_uninlined_format_args is false and this arg hasn't been inlined already
param.kind != Numbered && (!ignore_mixed || matches!(param.kind, NamedInline(_)))
}
}

View File

@ -828,7 +828,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
))
});
store.register_late_pass(move |_| Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks));
store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv())));
let allow_mixed_uninlined = conf.allow_mixed_uninlined_format_args;
store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv(), allow_mixed_uninlined)));
store.register_late_pass(|_| Box::new(trailing_empty_array::TrailingEmptyArray));
store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
store.register_late_pass(|_| Box::new(needless_late_init::NeedlessLateInit));

View File

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

View File

@ -0,0 +1 @@
allow-mixed-uninlined-format-args = false

View File

@ -0,0 +1,14 @@
// run-rustfix
#![warn(clippy::uninlined_format_args)]
fn main() {
let local_i32 = 1;
let local_f64 = 2.0;
let local_opt: Option<i32> = Some(3);
println!("val='{local_i32}'");
println!("Hello x is {local_f64:.local_i32$}");
println!("Hello {local_i32} is {local_f64:.*}", 5);
println!("Hello {local_i32} is {local_f64:.*}", 5);
println!("{local_i32}, {}", local_opt.unwrap());
}

View File

@ -0,0 +1,14 @@
// run-rustfix
#![warn(clippy::uninlined_format_args)]
fn main() {
let local_i32 = 1;
let local_f64 = 2.0;
let local_opt: Option<i32> = Some(3);
println!("val='{}'", local_i32);
println!("Hello {} is {:.*}", "x", local_i32, local_f64);
println!("Hello {} is {:.*}", local_i32, 5, local_f64);
println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
println!("{}, {}", local_i32, local_opt.unwrap());
}

View File

@ -0,0 +1,76 @@
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:9:5
|
LL | println!("val='{}'", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::uninlined-format-args` implied by `-D warnings`
help: change this to
|
LL - println!("val='{}'", local_i32);
LL + println!("val='{local_i32}'");
|
error: literal with an empty format string
--> $DIR/uninlined_format_args.rs:10:35
|
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
| ^^^
|
= note: `-D clippy::print-literal` implied by `-D warnings`
help: try this
|
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
LL + println!("Hello x is {:.*}", local_i32, local_f64);
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:10:5
|
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
LL + println!("Hello {} is {local_f64:.local_i32$}", "x");
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:11:5
|
LL | println!("Hello {} is {:.*}", local_i32, 5, local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("Hello {} is {:.*}", local_i32, 5, local_f64);
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:12:5
|
LL | println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:13:5
|
LL | println!("{}, {}", local_i32, local_opt.unwrap());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{}, {}", local_i32, local_opt.unwrap());
LL + println!("{local_i32}, {}", local_opt.unwrap());
|
error: aborting due to 6 previous errors

View File

@ -1,6 +1,7 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of
allow-dbg-in-tests
allow-expect-in-tests
allow-mixed-uninlined-format-args
allow-print-in-tests
allow-unwrap-in-tests
allowed-scripts

View File

@ -54,11 +54,11 @@ fn tester(fn_arg: i32) {
println!("{local_i32:<3}");
println!("{local_i32:#010x}");
println!("{local_f64:.1}");
println!("Hello {} is {local_f64:.local_i32$}", "x");
println!("Hello {local_i32} is {local_f64:.*}", 5);
println!("Hello {local_i32} is {local_f64:.*}", 5);
println!("Hello {} is {:.*}", "x", local_i32, local_f64);
println!("Hello {} is {:.*}", local_i32, 5, local_f64);
println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
println!("{local_i32} {local_f64}");
println!("{local_i32}, {}", local_opt.unwrap());
println!("{}, {}", local_i32, local_opt.unwrap());
println!("{val}");
println!("{val}");
println!("{} {1}", local_i32, 42);

View File

@ -177,42 +177,6 @@ LL - println!("{:.1}", local_f64);
LL + println!("{local_f64:.1}");
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:59:5
|
LL | println!("Hello {} is {:.*}", "x", local_i32, local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("Hello {} is {:.*}", "x", local_i32, local_f64);
LL + println!("Hello {} is {local_f64:.local_i32$}", "x");
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:60:5
|
LL | println!("Hello {} is {:.*}", local_i32, 5, local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("Hello {} is {:.*}", local_i32, 5, local_f64);
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:61:5
|
LL | println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("Hello {} is {2:.*}", local_i32, 5, local_f64);
LL + println!("Hello {local_i32} is {local_f64:.*}", 5);
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:62:5
|
@ -225,18 +189,6 @@ LL - println!("{} {}", local_i32, local_f64);
LL + println!("{local_i32} {local_f64}");
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:63:5
|
LL | println!("{}, {}", local_i32, local_opt.unwrap());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - println!("{}, {}", local_i32, local_opt.unwrap());
LL + println!("{local_i32}, {}", local_opt.unwrap());
|
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:64:5
|
@ -904,5 +856,5 @@ LL - println!("expand='{}'", local_i32);
LL + println!("expand='{local_i32}'");
|
error: aborting due to 76 previous errors
error: aborting due to 72 previous errors