Skip to content

Commit

Permalink
Impl
Browse files Browse the repository at this point in the history
  • Loading branch information
nyurik committed Feb 12, 2024
1 parent b0e7640 commit b41c598
Show file tree
Hide file tree
Showing 11 changed files with 358 additions and 7 deletions.
5 changes: 5 additions & 0 deletions clippy_config/src/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,11 @@ define_Conf! {
/// 2. Paths with any segment that containing the word 'prelude'
/// are already allowed by default.
(allowed_wildcard_imports: FxHashSet<String> = FxHashSet::default()),
/// 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
6 changes: 4 additions & 2 deletions clippy_lints/src/format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,22 +171,24 @@ impl_lint_pass!(FormatArgs => [
pub struct FormatArgs {
msrv: Msrv,
ignore_mixed: bool,
include_custom: bool,
}

impl FormatArgs {
#[must_use]
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool, include_custom_format_macros: bool) -> Self {
Self {
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) = find_format_args(cx, expr, macro_call.expn)
{
for piece in &format_args.template {
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 @@ -101,12 +101,14 @@ struct FormatTraitNames {
pub struct FormatImpl {
// 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() -> Self {
pub fn new(include_custom: bool) -> Self {
Self {
format_trait_impl: None,
include_custom,
}
}
}
Expand All @@ -131,6 +133,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl {
cx,
expr,
format_trait_impl,
include_custom: self.include_custom,
};
linter.check_to_string_in_display();
linter.check_self_in_format_args();
Expand All @@ -143,6 +146,7 @@ struct FormatImplExpr<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
expr: &'tcx Expr<'tcx>,
format_trait_impl: FormatTraitNames,
include_custom: bool,
}

impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> {
Expand Down Expand Up @@ -174,7 +178,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) = find_format_args(self.cx, self.expr, outer_macro.expn)
{
for piece in &format_args.template {
Expand Down
12 changes: 9 additions & 3 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,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 @@ -817,7 +818,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(mut_key::MutableKeyType::new(ignore_interior_mutability.clone())));
store.register_early_pass(|| Box::new(reference::DerefAddrOf));
store.register_early_pass(|| Box::new(double_parens::DoubleParens));
store.register_late_pass(|_| Box::new(format_impl::FormatImpl::new()));
store.register_late_pass(move |_| Box::new(format_impl::FormatImpl::new(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 @@ -942,8 +943,13 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
accept_comment_above_attributes,
))
});
store
.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(msrv(), allow_mixed_uninlined_format_args)));
store.register_late_pass(move |_| {
Box::new(format_args::FormatArgs::new(
msrv(),
allow_mixed_uninlined_format_args,
include_custom_format_macros,
))
});
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
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,
// };
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:73:5
|
LL | println!("val='{}'", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::uninlined-format-args` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::uninlined_format_args)]`
help: change this to
|
LL - println!("val='{}'", local_i32);
LL + println!("val='{local_i32}'");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:77:5
|
LL | my_good_macro!("{}", local_i32);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - my_good_macro!("{}", local_i32);
LL + my_good_macro!("{local_i32}");
|

error: variables can be used directly in the `format!` string
--> $DIR/uninlined_format_args.rs:78:5
|
LL | my_good_macro!("{}", local_i32,);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: change this to
|
LL - my_good_macro!("{}", local_i32,);
LL + my_good_macro!("{local_i32}",);
|

error: aborting due to 3 previous errors

Loading

0 comments on commit b41c598

Please sign in to comment.