Skip to content

Commit

Permalink
Impl
Browse files Browse the repository at this point in the history
  • Loading branch information
nyurik committed May 26, 2024
1 parent 5aae5f6 commit 48385e7
Show file tree
Hide file tree
Showing 11 changed files with 362 additions and 5 deletions.
5 changes: 5 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,11 @@ define_Conf! {
///
/// Whether to also emit warnings for unsafe blocks with metavariable expansions in **private** macros.
(warn_unsafe_macro_metavars_in_private_macros: bool = false),
/// Lint: FORMAT_IN_FORMAT_ARGS, RECURSIVE_FORMAT_IMPL, TO_STRING_IN_FORMAT_ARGS, UNINLINED_FORMAT_ARGS, UNUSED_FORMAT_SPECS.
///
/// Recognize all macros that internally use `format_args!` and similar macros
/// as format macros, instead of just the ones in the standard library.
(include_custom_format_macros: bool = false),
}

/// Search for the configuration file.
Expand Down
11 changes: 9 additions & 2 deletions clippy_lints/src/format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,23 +172,30 @@ pub struct FormatArgs {
format_args: FormatArgsStorage,
msrv: Msrv,
ignore_mixed: bool,
include_custom: bool,
}

impl FormatArgs {
#[must_use]
pub fn new(format_args: FormatArgsStorage, msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
pub fn new(
format_args: FormatArgsStorage,
msrv: Msrv,
allow_mixed_uninlined_format_args: bool,
include_custom_format_macros: bool,
) -> Self {
Self {
format_args,
msrv,
ignore_mixed: allow_mixed_uninlined_format_args,
include_custom: include_custom_format_macros,
}
}
}

impl<'tcx> LateLintPass<'tcx> for FormatArgs {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
&& is_format_macro(cx, macro_call.def_id)
&& (self.include_custom || is_format_macro(cx, macro_call.def_id))
&& let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn)
{
let linter = FormatArgsExpr {
Expand Down
8 changes: 6 additions & 2 deletions clippy_lints/src/format_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,15 @@ pub struct FormatImpl {
format_args: FormatArgsStorage,
// Whether we are inside Display or Debug trait impl - None for neither
format_trait_impl: Option<FormatTraitNames>,
include_custom: bool,
}

impl FormatImpl {
pub fn new(format_args: FormatArgsStorage) -> Self {
pub fn new(format_args: FormatArgsStorage, include_custom: bool) -> Self {
Self {
format_args,
format_trait_impl: None,
include_custom,
}
}
}
Expand All @@ -134,6 +136,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl {
format_args: &self.format_args,
expr,
format_trait_impl,
include_custom: self.include_custom,
};
linter.check_to_string_in_display();
linter.check_self_in_format_args();
Expand All @@ -147,6 +150,7 @@ struct FormatImplExpr<'a, 'tcx> {
format_args: &'a FormatArgsStorage,
expr: &'tcx Expr<'tcx>,
format_trait_impl: FormatTraitNames,
include_custom: bool,
}

impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> {
Expand Down Expand Up @@ -178,7 +182,7 @@ impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> {
// Check each arg in format calls - do we ever use Display on self (directly or via deref)?
if let Some(outer_macro) = root_macro_call_first_node(self.cx, self.expr)
&& let macro_def_id = outer_macro.def_id
&& is_format_macro(self.cx, macro_def_id)
&& (self.include_custom || is_format_macro(self.cx, macro_def_id))
&& let Some(format_args) = self.format_args.get(self.cx, self.expr, outer_macro.expn)
{
for piece in &format_args.template {
Expand Down
9 changes: 8 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
excessive_nesting_threshold,
future_size_threshold,
ref ignore_interior_mutability,
include_custom_format_macros,
large_error_threshold,
literal_representation_threshold,
matches_for_let_else,
Expand Down Expand Up @@ -851,7 +852,12 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_early_pass(|| Box::new(reference::DerefAddrOf));
store.register_early_pass(|| Box::new(double_parens::DoubleParens));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| Box::new(format_impl::FormatImpl::new(format_args.clone())));
store.register_late_pass(move |_| {
Box::new(format_impl::FormatImpl::new(
format_args.clone(),
include_custom_format_macros,
))
});
store.register_early_pass(|| Box::new(unsafe_removed_from_name::UnsafeNameRemoval));
store.register_early_pass(|| Box::new(else_if_without_else::ElseIfWithoutElse));
store.register_early_pass(|| Box::new(int_plus_one::IntPlusOne));
Expand Down Expand Up @@ -983,6 +989,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
format_args.clone(),
msrv(),
allow_mixed_uninlined_format_args,
include_custom_format_macros,
))
});
store.register_late_pass(|_| Box::new(trailing_empty_array::TrailingEmptyArray));
Expand Down
1 change: 1 addition & 0 deletions tests/ui-toml/include_custom_format_macros/clippy.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
include-custom-format-macros = true
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#![warn(clippy::format_in_format_args, clippy::to_string_in_format_args)]
#![allow(clippy::assertions_on_constants, clippy::eq_op, clippy::uninlined_format_args)]

use std::io::{stdout, Error, ErrorKind, Write};
use std::ops::Deref;
use std::panic::Location;

macro_rules! _internal {
($($args:tt)*) => {
println!("{}", format_args!($($args)*))
};
}

macro_rules! my_println2 {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!($($args)+)
}
}};
}

macro_rules! my_println2_args {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!("foo: {}", format_args!($($args)+))
}
}};
}

fn main() {
let error = Error::new(ErrorKind::Other, "bad thing");

my_println2!(true, "error: {}", format!("something failed at {}", Location::caller()));
my_println2!(
true,
"{}: {}",
error,
format!("something failed at {}", Location::caller())
);

my_println2!(
true,
"error: {}",
format!("something failed at {}", Location::caller().to_string())
);
my_println2!(
true,
"{}: {}",
error,
format!("something failed at {}", Location::caller().to_string())
);

my_println2!(true, "error: {}", Location::caller().to_string());
my_println2!(true, "{}: {}", error, Location::caller().to_string());

my_println2_args!(true, "error: {}", format!("something failed at {}", Location::caller()));
my_println2_args!(
true,
"{}: {}",
error,
format!("something failed at {}", Location::caller())
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#![warn(clippy::uninlined_format_args)]
#![allow(named_arguments_used_positionally, unused_imports, unused_macros, unused_variables)]
#![allow(clippy::eq_op, clippy::format_in_format_args, clippy::print_literal)]

macro_rules! _internal {
($($args:tt)*) => {
println!("{}", format_args!($($args)*))
};
}

macro_rules! my_println2 {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!($($args)+)
}
}};
}

macro_rules! my_println2_args {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!("foo: {}", format_args!($($args)+))
}
}};
}

macro_rules! my_concat {
($fmt:literal $(, $e:expr)*) => {
println!(concat!("ERROR: ", $fmt), $($e,)*)
}
}

macro_rules! my_good_macro {
($fmt:literal $(, $e:expr)* $(,)?) => {
println!($fmt $(, $e)*)
}
}

macro_rules! my_bad_macro {
($fmt:literal, $($e:expr),*) => {
println!($fmt, $($e,)*)
}
}

macro_rules! my_bad_macro2 {
($fmt:literal) => {
let s = $fmt.clone();
println!("{}", s);
};
($fmt:literal, $($e:expr)+) => {
println!($fmt, $($e,)*)
};
}

// This abomination was suggested by @Alexendoo, may the Rust gods have mercy on their soul...
// https://github.com/rust-lang/rust-clippy/pull/9948#issuecomment-1327965962
macro_rules! used_twice {
(
large = $large:literal,
small = $small:literal,
$val:expr,
) => {
if $val < 5 {
println!($small, $val);
} else {
println!($large, $val);
}
};
}

fn main() {
let local_i32 = 1;
println!("val='{local_i32}'");
my_println2_args!(true, "{}", local_i32);
my_println2!(true, "{}", local_i32);
my_concat!("{}", local_i32);
my_good_macro!("{local_i32}");
my_good_macro!("{local_i32}",);

// FIXME: Broken false positives, currently unhandled
// my_bad_macro!("{}", local_i32);
// my_bad_macro2!("{}", local_i32);
// used_twice! {
// large = "large value: {}",
// small = "small value: {}",
// local_i32,
// };
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
#![warn(clippy::uninlined_format_args)]
#![allow(named_arguments_used_positionally, unused_imports, unused_macros, unused_variables)]
#![allow(clippy::eq_op, clippy::format_in_format_args, clippy::print_literal)]

macro_rules! _internal {
($($args:tt)*) => {
println!("{}", format_args!($($args)*))
};
}

macro_rules! my_println2 {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!($($args)+)
}
}};
}

macro_rules! my_println2_args {
($target:expr, $($args:tt)+) => {{
if $target {
_internal!("foo: {}", format_args!($($args)+))
}
}};
}

macro_rules! my_concat {
($fmt:literal $(, $e:expr)*) => {
println!(concat!("ERROR: ", $fmt), $($e,)*)
}
}

macro_rules! my_good_macro {
($fmt:literal $(, $e:expr)* $(,)?) => {
println!($fmt $(, $e)*)
}
}

macro_rules! my_bad_macro {
($fmt:literal, $($e:expr),*) => {
println!($fmt, $($e,)*)
}
}

macro_rules! my_bad_macro2 {
($fmt:literal) => {
let s = $fmt.clone();
println!("{}", s);
};
($fmt:literal, $($e:expr)+) => {
println!($fmt, $($e,)*)
};
}

// This abomination was suggested by @Alexendoo, may the Rust gods have mercy on their soul...
// https://github.com/rust-lang/rust-clippy/pull/9948#issuecomment-1327965962
macro_rules! used_twice {
(
large = $large:literal,
small = $small:literal,
$val:expr,
) => {
if $val < 5 {
println!($small, $val);
} else {
println!($large, $val);
}
};
}

fn main() {
let local_i32 = 1;
println!("val='{}'", local_i32);
my_println2_args!(true, "{}", local_i32);
my_println2!(true, "{}", local_i32);
my_concat!("{}", local_i32);
my_good_macro!("{}", local_i32);
my_good_macro!("{}", local_i32,);

// FIXME: Broken false positives, currently unhandled
// my_bad_macro!("{}", local_i32);
// my_bad_macro2!("{}", local_i32);
// used_twice! {
// large = "large value: {}",
// small = "small value: {}",
// local_i32,
// };
}
Loading

0 comments on commit 48385e7

Please sign in to comment.