Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allow-mixed-uninlined-format-args config #9865

Merged
merged 2 commits into from
Nov 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 39 additions & 16 deletions clippy_lints/src/format_args.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -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"
}

Expand Down Expand Up @@ -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,
}
}
}

Expand All @@ -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);
}
}
}
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}

Expand All @@ -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
Expand All @@ -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(_)))
}
}

Expand Down
3 changes: 2 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-mixed-uninlined-format-args = false
Original file line number Diff line number Diff line change
@@ -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());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// run-rustfix
nyurik marked this conversation as resolved.
Show resolved Hide resolved
#![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());
}
Original file line number Diff line number Diff line change
@@ -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

1 change: 1 addition & 0 deletions tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/uninlined_format_args.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
50 changes: 1 addition & 49 deletions tests/ui/uninlined_format_args.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
|
Expand All @@ -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
|
Expand Down Expand Up @@ -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