Skip to content

Commit 6249c11

Browse files
nyuriknyurik
committed
implement argument, more unit tests
1 parent 39fadaf commit 6249c11

22 files changed

+872
-112
lines changed

‎clippy_lints/src/format_args.rs

+9-6
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
22
use clippy_utils::is_diag_trait_item;
33
use clippy_utils::macros::FormatParamKind::{Implicit, Named, NamedInline, Numbered, Starred};
44
use clippy_utils::macros::{
5-
is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
5+
is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
66
};
77
use clippy_utils::msrvs::{self, Msrv};
88
use clippy_utils::source::snippet_opt;
@@ -169,24 +169,27 @@ impl_lint_pass!(FormatArgs => [
169169
pub struct FormatArgs {
170170
msrv: Msrv,
171171
ignore_mixed: bool,
172+
include_custom: bool,
172173
}
173174

174175
impl FormatArgs {
175176
#[must_use]
176-
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
177+
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool, include_custom_format_macros: bool) -> Self {
177178
Self {
178179
msrv,
179180
ignore_mixed: allow_mixed_uninlined_format_args,
181+
include_custom: include_custom_format_macros,
180182
}
181183
}
182184
}
183185

184186
impl<'tcx> LateLintPass<'tcx> for FormatArgs {
185187
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
186-
if let Some(format_args) = FormatArgsExpn::parse(cx, expr)
187-
&& let expr_expn_data = expr.span.ctxt().outer_expn_data()
188-
&& let outermost_expn_data = outermost_expn_data(expr_expn_data)
189-
&& let Some(macro_def_id) = outermost_expn_data.macro_def_id
188+
let expr_expn_data = expr.span.ctxt().outer_expn_data();
189+
let outermost_expn_data = outermost_expn_data(expr_expn_data);
190+
if let Some(macro_def_id) = outermost_expn_data.macro_def_id
191+
&& (self.include_custom || is_format_macro(cx, macro_def_id))
192+
&& let Some(format_args) = FormatArgsExpn::parse(cx, expr)
190193
&& let ExpnKind::Macro(_, name) = outermost_expn_data.kind
191194
{
192195
for arg in &format_args.args {

‎clippy_lints/src/format_impl.rs

+12-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
2-
use clippy_utils::macros::{root_macro_call_first_node, FormatArg, FormatArgsExpn};
2+
use clippy_utils::macros::{is_format_macro, root_macro_call_first_node, FormatArg, FormatArgsExpn};
33
use clippy_utils::{get_parent_as_impl, is_diag_trait_item, path_to_local, peel_ref_operators};
44
use if_chain::if_chain;
55
use rustc_errors::Applicability;
@@ -100,12 +100,14 @@ struct FormatTrait {
100100
pub struct FormatImpl {
101101
// Whether we are inside Display or Debug trait impl - None for neither
102102
format_trait_impl: Option<FormatTrait>,
103+
include_custom: bool,
103104
}
104105

105106
impl FormatImpl {
106-
pub fn new() -> Self {
107+
pub fn new(include_custom: bool) -> Self {
107108
Self {
108109
format_trait_impl: None,
110+
include_custom,
109111
}
110112
}
111113
}
@@ -131,7 +133,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl {
131133
check_to_string_in_display(cx, expr);
132134
}
133135

134-
check_self_in_format_args(cx, expr, format_trait_impl);
136+
self.check_self_in_format_args(cx, expr, format_trait_impl);
135137
check_print_in_format_impl(cx, expr, format_trait_impl);
136138
}
137139
}
@@ -161,12 +163,13 @@ fn check_to_string_in_display(cx: &LateContext<'_>, expr: &Expr<'_>) {
161163
}
162164
}
163165

164-
fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, impl_trait: FormatTrait) {
165-
// Check each arg in format calls - do we ever use Display on self (directly or via deref)?
166-
if_chain! {
167-
if let Some(outer_macro) = root_macro_call_first_node(cx, expr);
168-
if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn);
169-
then {
166+
impl FormatImpl {
167+
fn check_self_in_format_args<'tcx>(&self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, impl_trait: FormatTrait) {
168+
// Check each arg in format calls - do we ever use Display on self (directly or via deref)?
169+
if let Some(outer_macro) = root_macro_call_first_node(cx, expr)
170+
&& (self.include_custom || is_format_macro(cx, outer_macro.def_id))
171+
&& let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn)
172+
{
170173
for arg in format_args.args {
171174
if arg.format.r#trait != impl_trait.name {
172175
continue;

‎clippy_lints/src/lib.rs

+11-3
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
703703
store.register_late_pass(move |_| Box::new(mut_key::MutableKeyType::new(ignore_interior_mutability.clone())));
704704
store.register_early_pass(|| Box::new(reference::DerefAddrOf));
705705
store.register_early_pass(|| Box::new(double_parens::DoubleParens));
706-
store.register_late_pass(|_| Box::new(format_impl::FormatImpl::new()));
706+
let include_custom_format_macros = conf.include_custom_format_macros;
707+
store.register_late_pass(move |_| Box::new(format_impl::FormatImpl::new(include_custom_format_macros)));
707708
store.register_early_pass(|| Box::new(unsafe_removed_from_name::UnsafeNameRemoval));
708709
store.register_early_pass(|| Box::new(else_if_without_else::ElseIfWithoutElse));
709710
store.register_early_pass(|| Box::new(int_plus_one::IntPlusOne));
@@ -832,8 +833,15 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
832833
))
833834
});
834835
store.register_late_pass(move |_| Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks));
835-
let allow_mixed_uninlined = conf.allow_mixed_uninlined_format_args;
836-
store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv(), allow_mixed_uninlined)));
836+
let allow_mixed_uninlined_format_args = conf.allow_mixed_uninlined_format_args;
837+
let include_custom_format_macros = conf.include_custom_format_macros;
838+
store.register_late_pass(move |_| {
839+
Box::new(format_args::FormatArgs::new(
840+
msrv(),
841+
allow_mixed_uninlined_format_args,
842+
include_custom_format_macros,
843+
))
844+
});
837845
store.register_late_pass(|_| Box::new(trailing_empty_array::TrailingEmptyArray));
838846
store.register_early_pass(|| Box::new(octal_escapes::OctalEscapes));
839847
store.register_late_pass(|_| Box::new(needless_late_init::NeedlessLateInit));

‎clippy_lints/src/utils/conf.rs

+5
Original file line numberDiff line numberDiff line change
@@ -414,6 +414,11 @@ define_Conf! {
414414
/// configuration will cause restriction lints to trigger even
415415
/// if no suggestion can be made.
416416
(suppress_restriction_lint_in_const: bool = false),
417+
/// Lint: FORMAT_IN_FORMAT_ARGS, RECURSIVE_FORMAT_IMPL, TO_STRING_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, UNUSED_FORMAT_SPECS.
418+
///
419+
/// Recognize all macros that internally use `format_args!` and similar macros
420+
/// as format macros, instead of just the ones in the standard library.
421+
(include_custom_format_macros: bool = false),
417422
}
418423

419424
/// Search for the configuration file.

‎clippy_utils/src/macros.rs

+27
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,33 @@ use rustc_span::{sym, BytePos, ExpnData, ExpnId, ExpnKind, Pos, Span, SpanData,
1919
use std::iter::{once, zip};
2020
use std::ops::ControlFlow;
2121

22+
const FORMAT_MACRO_DIAG_ITEMS: &[Symbol] = &[
23+
sym::assert_eq_macro,
24+
sym::assert_macro,
25+
sym::assert_ne_macro,
26+
sym::debug_assert_eq_macro,
27+
sym::debug_assert_macro,
28+
sym::debug_assert_ne_macro,
29+
sym::eprint_macro,
30+
sym::eprintln_macro,
31+
sym::format_args_macro,
32+
sym::format_macro,
33+
sym::print_macro,
34+
sym::println_macro,
35+
sym::std_panic_macro,
36+
sym::write_macro,
37+
sym::writeln_macro,
38+
];
39+
40+
/// Returns true if a given Macro `DefId` is a format macro (e.g. `println!`)
41+
pub fn is_format_macro(cx: &LateContext<'_>, macro_def_id: DefId) -> bool {
42+
if let Some(name) = cx.tcx.get_diagnostic_name(macro_def_id) {
43+
FORMAT_MACRO_DIAG_ITEMS.contains(&name)
44+
} else {
45+
false
46+
}
47+
}
48+
2249
/// A macro call, like `vec![1, 2, 3]`.
2350
///
2451
/// Use `tcx.item_name(macro_call.def_id)` to get the macro name.

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

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
include-custom-format-macros = true

‎tests/ui-toml/include_custom_format_macros/format_args_unfixable.rs

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
#![warn(clippy::format_in_format_args, clippy::to_string_in_format_args)]
2+
#![allow(clippy::assertions_on_constants, clippy::eq_op, clippy::uninlined_format_args)]
3+
4+
use std::io::{stdout, Error, ErrorKind, Write};
5+
use std::ops::Deref;
6+
use std::panic::Location;
7+
8+
macro_rules! _internal {
9+
($($args:tt)*) => {
10+
println!("{}", format_args!($($args)*))
11+
};
12+
}
13+
14+
macro_rules! my_println2 {
15+
($target:expr, $($args:tt)+) => {{
16+
if $target {
17+
_internal!($($args)+)
18+
}
19+
}};
20+
}
21+
22+
macro_rules! my_println2_args {
23+
($target:expr, $($args:tt)+) => {{
24+
if $target {
25+
_internal!("foo: {}", format_args!($($args)+))
26+
}
27+
}};
28+
}
29+
30+
fn main() {
31+
let error = Error::new(ErrorKind::Other, "bad thing");
32+
33+
my_println2!(true, "error: {}", format!("something failed at {}", Location::caller()));
34+
my_println2!(
35+
true,
36+
"{}: {}",
37+
error,
38+
format!("something failed at {}", Location::caller())
39+
);
40+
41+
my_println2!(
42+
true,
43+
"error: {}",
44+
format!("something failed at {}", Location::caller().to_string())
45+
);
46+
my_println2!(
47+
true,
48+
"{}: {}",
49+
error,
50+
format!("something failed at {}", Location::caller().to_string())
51+
);
52+
53+
my_println2!(true, "error: {}", Location::caller().to_string());
54+
my_println2!(true, "{}: {}", error, Location::caller().to_string());
55+
56+
my_println2_args!(true, "error: {}", format!("something failed at {}", Location::caller()));
57+
my_println2_args!(
58+
true,
59+
"{}: {}",
60+
error,
61+
format!("something failed at {}", Location::caller())
62+
);
63+
}

‎tests/ui-toml/include_custom_format_macros/format_args_unfixable.stderr

+102
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
error: `format!` in `my_println2!` args
2+
--> $DIR/format_args_unfixable.rs:33:5
3+
|
4+
LL | my_println2!(true, "error: {}", format!("something failed at {}", Location::caller()));
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
= help: combine the `format!(..)` arguments with the outer `my_println2!(..)` call
8+
= help: or consider changing `format!` to `format_args!`
9+
= note: `-D clippy::format-in-format-args` implied by `-D warnings`
10+
11+
error: `format!` in `my_println2!` args
12+
--> $DIR/format_args_unfixable.rs:34:5
13+
|
14+
LL | / my_println2!(
15+
LL | | true,
16+
LL | | "{}: {}",
17+
LL | | error,
18+
LL | | format!("something failed at {}", Location::caller())
19+
LL | | );
20+
| |_____^
21+
|
22+
= help: combine the `format!(..)` arguments with the outer `my_println2!(..)` call
23+
= help: or consider changing `format!` to `format_args!`
24+
25+
error: `format!` in `my_println2!` args
26+
--> $DIR/format_args_unfixable.rs:41:5
27+
|
28+
LL | / my_println2!(
29+
LL | | true,
30+
LL | | "error: {}",
31+
LL | | format!("something failed at {}", Location::caller().to_string())
32+
LL | | );
33+
| |_____^
34+
|
35+
= help: combine the `format!(..)` arguments with the outer `my_println2!(..)` call
36+
= help: or consider changing `format!` to `format_args!`
37+
38+
error: `to_string` applied to a type that implements `Display` in `format!` args
39+
--> $DIR/format_args_unfixable.rs:44:61
40+
|
41+
LL | format!("something failed at {}", Location::caller().to_string())
42+
| ^^^^^^^^^^^^ help: remove this
43+
|
44+
= note: `-D clippy::to-string-in-format-args` implied by `-D warnings`
45+
46+
error: `format!` in `my_println2!` args
47+
--> $DIR/format_args_unfixable.rs:46:5
48+
|
49+
LL | / my_println2!(
50+
LL | | true,
51+
LL | | "{}: {}",
52+
LL | | error,
53+
LL | | format!("something failed at {}", Location::caller().to_string())
54+
LL | | );
55+
| |_____^
56+
|
57+
= help: combine the `format!(..)` arguments with the outer `my_println2!(..)` call
58+
= help: or consider changing `format!` to `format_args!`
59+
60+
error: `to_string` applied to a type that implements `Display` in `format!` args
61+
--> $DIR/format_args_unfixable.rs:50:61
62+
|
63+
LL | format!("something failed at {}", Location::caller().to_string())
64+
| ^^^^^^^^^^^^ help: remove this
65+
66+
error: `to_string` applied to a type that implements `Display` in `my_println2!` args
67+
--> $DIR/format_args_unfixable.rs:53:55
68+
|
69+
LL | my_println2!(true, "error: {}", Location::caller().to_string());
70+
| ^^^^^^^^^^^^ help: remove this
71+
72+
error: `to_string` applied to a type that implements `Display` in `my_println2!` args
73+
--> $DIR/format_args_unfixable.rs:54:59
74+
|
75+
LL | my_println2!(true, "{}: {}", error, Location::caller().to_string());
76+
| ^^^^^^^^^^^^ help: remove this
77+
78+
error: `format!` in `my_println2_args!` args
79+
--> $DIR/format_args_unfixable.rs:56:5
80+
|
81+
LL | my_println2_args!(true, "error: {}", format!("something failed at {}", Location::caller()));
82+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
83+
|
84+
= help: combine the `format!(..)` arguments with the outer `my_println2_args!(..)` call
85+
= help: or consider changing `format!` to `format_args!`
86+
87+
error: `format!` in `my_println2_args!` args
88+
--> $DIR/format_args_unfixable.rs:57:5
89+
|
90+
LL | / my_println2_args!(
91+
LL | | true,
92+
LL | | "{}: {}",
93+
LL | | error,
94+
LL | | format!("something failed at {}", Location::caller())
95+
LL | | );
96+
| |_____^
97+
|
98+
= help: combine the `format!(..)` arguments with the outer `my_println2_args!(..)` call
99+
= help: or consider changing `format!` to `format_args!`
100+
101+
error: aborting due to 10 previous errors
102+

0 commit comments

Comments
 (0)