Skip to content

Commit 7239dd6

Browse files
nyuriknyurik
committed
Process all format-like macros
Remove the `is_format_macro()` function and calls to it because now it seems to be working just fine without it, properly handling all other use cases, such as `log::warn!(...)`
1 parent 6d0b4e3 commit 7239dd6

File tree

8 files changed

+267
-59
lines changed

8 files changed

+267
-59
lines changed

‎clippy_dev/src/new_lint.rs

+2-6
Original file line numberDiff line numberDiff line change
@@ -368,9 +368,7 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R
368368
}}
369369
todo!();
370370
}}
371-
"#,
372-
context_import = context_import,
373-
name_upper = name_upper,
371+
"#
374372
);
375373
} else {
376374
let _ = writedoc!(
@@ -384,9 +382,7 @@ fn create_lint_for_ty(lint: &LintData<'_>, enable_msrv: bool, ty: &str) -> io::R
384382
pub(super) fn check(cx: &{context_import}) {{
385383
todo!();
386384
}}
387-
"#,
388-
context_import = context_import,
389-
name_upper = name_upper,
385+
"#
390386
);
391387
}
392388

‎clippy_dev/src/update_lints.rs

+5-9
Original file line numberDiff line numberDiff line change
@@ -537,17 +537,13 @@ fn declare_deprecated(name: &str, path: &Path, reason: &str) -> io::Result<()> {
537537
/// Nothing. This lint has been deprecated.
538538
///
539539
/// ### Deprecation reason
540-
/// {}
541-
#[clippy::version = \"{}\"]
542-
pub {},
543-
\"{}\"
540+
/// {deprecation_reason}
541+
#[clippy::version = \"{version}\"]
542+
pub {name},
543+
\"{reason}\"
544544
}}
545545
546-
",
547-
deprecation_reason,
548-
version,
549-
name,
550-
reason,
546+
"
551547
)
552548
}
553549

‎clippy_lints/src/format_args.rs

+1-2
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, Numbered, Starred};
44
use clippy_utils::macros::{
5-
is_format_macro, is_panic, root_macro_call, Count, FormatArg, FormatArgsExpn, FormatParam, FormatParamUsage,
5+
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;
@@ -177,7 +177,6 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
177177
&& let expr_expn_data = expr.span.ctxt().outer_expn_data()
178178
&& let outermost_expn_data = outermost_expn_data(expr_expn_data)
179179
&& let Some(macro_def_id) = outermost_expn_data.macro_def_id
180-
&& is_format_macro(cx, macro_def_id)
181180
&& let ExpnKind::Macro(_, name) = outermost_expn_data.kind
182181
{
183182
for arg in &format_args.args {

‎clippy_lints/src/format_impl.rs

+1-3
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::{is_format_macro, root_macro_call_first_node, FormatArg, FormatArgsExpn};
2+
use clippy_utils::macros::{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;
@@ -165,9 +165,7 @@ fn check_self_in_format_args<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>,
165165
// Check each arg in format calls - do we ever use Display on self (directly or via deref)?
166166
if_chain! {
167167
if let Some(outer_macro) = root_macro_call_first_node(cx, expr);
168-
if let macro_def_id = outer_macro.def_id;
169168
if let Some(format_args) = FormatArgsExpn::find_nested(cx, expr, outer_macro.expn);
170-
if is_format_macro(cx, macro_def_id);
171169
then {
172170
for arg in format_args.args {
173171
if arg.format.r#trait != impl_trait.name {

‎clippy_utils/src/macros.rs

-27
Original file line numberDiff line numberDiff line change
@@ -19,33 +19,6 @@ 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-
4922
/// A macro call, like `vec![1, 2, 3]`.
5023
///
5124
/// Use `tcx.item_name(macro_call.def_id)` to get the macro name.

‎tests/ui/uninlined_format_args.fixed

+93-6
Original file line numberDiff line numberDiff line change
@@ -121,8 +121,8 @@ fn tester(fn_arg: i32) {
121121
println!("{local_i32:width$.prec$}");
122122
println!("{width:width$.prec$}");
123123
println!("{}", format!("{local_i32}"));
124-
my_println!("{}", local_i32);
125-
my_println_args!("{}", local_i32);
124+
my_println!("{local_i32}");
125+
my_println_args!("{local_i32}");
126126

127127
// these should NOT be modified by the lint
128128
println!(concat!("nope ", "{}"), local_i32);
@@ -160,10 +160,6 @@ fn tester(fn_arg: i32) {
160160
}
161161
}
162162

163-
fn main() {
164-
tester(42);
165-
}
166-
167163
fn _under_msrv() {
168164
#![clippy::msrv = "1.57"]
169165
let local_i32 = 1;
@@ -175,3 +171,94 @@ fn _meets_msrv() {
175171
let local_i32 = 1;
176172
println!("expand='{local_i32}'");
177173
}
174+
175+
macro_rules! _internal {
176+
($($args:tt)*) => {
177+
println!("{}", format_args!($($args)*))
178+
};
179+
}
180+
181+
macro_rules! my_println2 {
182+
($target:expr, $($args:tt)+) => {{
183+
if $target {
184+
_internal!($($args)+)
185+
}
186+
}};
187+
}
188+
189+
macro_rules! my_println2_args {
190+
($target:expr, $($args:tt)+) => {{
191+
if $target {
192+
_internal!("foo: {}", format_args!($($args)+))
193+
}
194+
}};
195+
}
196+
197+
macro_rules! my_concat {
198+
($fmt:literal $(, $e:expr)*) => {
199+
println!(concat!("ERROR: ", $fmt), $($e,)*)
200+
}
201+
}
202+
203+
macro_rules! my_good_macro {
204+
($fmt:literal $(, $e:expr)* $(,)?) => {
205+
println!($fmt $(, $e)*)
206+
}
207+
}
208+
209+
macro_rules! my_bad_macro {
210+
($fmt:literal, $($e:expr),*) => {
211+
println!($fmt, $($e,)*)
212+
}
213+
}
214+
215+
macro_rules! my_bad_macro2 {
216+
($fmt:literal) => {
217+
let s = $fmt.clone();
218+
println!("{}", s);
219+
};
220+
($fmt:literal, $($e:expr)+) => {
221+
println!($fmt, $($e,)*)
222+
};
223+
}
224+
225+
// This abomination was suggested by @Alexendoo, may the Rust gods have mercy on their soul...
226+
// https://github.com/rust-lang/rust-clippy/pull/9948#issuecomment-1327965962
227+
macro_rules! used_twice {
228+
(
229+
large = $large:literal,
230+
small = $small:literal,
231+
$val:expr,
232+
) => {
233+
if $val < 5 {
234+
println!($small, $val);
235+
} else {
236+
println!($large, $val);
237+
}
238+
};
239+
}
240+
241+
fn tester2() {
242+
let local_i32 = 1;
243+
my_println2_args!(true, "{local_i32}");
244+
my_println2!(true, "{local_i32}");
245+
my_concat!("{}", local_i32);
246+
my_good_macro!("{local_i32}");
247+
my_good_macro!("{local_i32}",);
248+
249+
// FIXME: Broken false positives, currently unhandled
250+
// my_bad_macro!("{}", local_i32);
251+
// my_bad_macro2!("{}", local_i32);
252+
// used_twice! {
253+
// large = "large value: {}",
254+
// small = "small value: {}",
255+
// local_i32,
256+
// };
257+
}
258+
259+
// Add new tests right above this line to keep existing test line numbers intact.
260+
// The main function is the only one that be kept at the end.
261+
fn main() {
262+
tester(42);
263+
tester2();
264+
}

‎tests/ui/uninlined_format_args.rs

+91-4
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,6 @@ fn tester(fn_arg: i32) {
165165
}
166166
}
167167

168-
fn main() {
169-
tester(42);
170-
}
171-
172168
fn _under_msrv() {
173169
#![clippy::msrv = "1.57"]
174170
let local_i32 = 1;
@@ -180,3 +176,94 @@ fn _meets_msrv() {
180176
let local_i32 = 1;
181177
println!("expand='{}'", local_i32);
182178
}
179+
180+
macro_rules! _internal {
181+
($($args:tt)*) => {
182+
println!("{}", format_args!($($args)*))
183+
};
184+
}
185+
186+
macro_rules! my_println2 {
187+
($target:expr, $($args:tt)+) => {{
188+
if $target {
189+
_internal!($($args)+)
190+
}
191+
}};
192+
}
193+
194+
macro_rules! my_println2_args {
195+
($target:expr, $($args:tt)+) => {{
196+
if $target {
197+
_internal!("foo: {}", format_args!($($args)+))
198+
}
199+
}};
200+
}
201+
202+
macro_rules! my_concat {
203+
($fmt:literal $(, $e:expr)*) => {
204+
println!(concat!("ERROR: ", $fmt), $($e,)*)
205+
}
206+
}
207+
208+
macro_rules! my_good_macro {
209+
($fmt:literal $(, $e:expr)* $(,)?) => {
210+
println!($fmt $(, $e)*)
211+
}
212+
}
213+
214+
macro_rules! my_bad_macro {
215+
($fmt:literal, $($e:expr),*) => {
216+
println!($fmt, $($e,)*)
217+
}
218+
}
219+
220+
macro_rules! my_bad_macro2 {
221+
($fmt:literal) => {
222+
let s = $fmt.clone();
223+
println!("{}", s);
224+
};
225+
($fmt:literal, $($e:expr)+) => {
226+
println!($fmt, $($e,)*)
227+
};
228+
}
229+
230+
// This abomination was suggested by @Alexendoo, may the Rust gods have mercy on their soul...
231+
// https://github.com/rust-lang/rust-clippy/pull/9948#issuecomment-1327965962
232+
macro_rules! used_twice {
233+
(
234+
large = $large:literal,
235+
small = $small:literal,
236+
$val:expr,
237+
) => {
238+
if $val < 5 {
239+
println!($small, $val);
240+
} else {
241+
println!($large, $val);
242+
}
243+
};
244+
}
245+
246+
fn tester2() {
247+
let local_i32 = 1;
248+
my_println2_args!(true, "{}", local_i32);
249+
my_println2!(true, "{}", local_i32);
250+
my_concat!("{}", local_i32);
251+
my_good_macro!("{}", local_i32);
252+
my_good_macro!("{}", local_i32,);
253+
254+
// FIXME: Broken false positives, currently unhandled
255+
// my_bad_macro!("{}", local_i32);
256+
// my_bad_macro2!("{}", local_i32);
257+
// used_twice! {
258+
// large = "large value: {}",
259+
// small = "small value: {}",
260+
// local_i32,
261+
// };
262+
}
263+
264+
// Add new tests right above this line to keep existing test line numbers intact.
265+
// The main function is the only one that be kept at the end.
266+
fn main() {
267+
tester(42);
268+
tester2();
269+
}

0 commit comments

Comments
 (0)