Skip to content

Fix FormatArgs storage when -Zthreads > 1 #12567

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

Merged
merged 1 commit into from
May 3, 2024
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
20 changes: 15 additions & 5 deletions clippy_lints/src/explicit_write.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::{find_format_args, format_args_inputs_span};
use clippy_utils::macros::{format_args_inputs_span, FormatArgsStorage};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{is_expn_of, path_def_id};
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::{BindingAnnotation, Block, BlockCheckMode, Expr, ExprKind, Node, PatKind, QPath, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;
use rustc_span::{sym, ExpnId};

declare_clippy_lint! {
Expand Down Expand Up @@ -38,7 +38,17 @@ declare_clippy_lint! {
"using the `write!()` family of functions instead of the `print!()` family of functions, when using the latter would work"
}

declare_lint_pass!(ExplicitWrite => [EXPLICIT_WRITE]);
pub struct ExplicitWrite {
format_args: FormatArgsStorage,
}

impl ExplicitWrite {
pub fn new(format_args: FormatArgsStorage) -> Self {
Self { format_args }
}
}

impl_lint_pass!(ExplicitWrite => [EXPLICIT_WRITE]);

impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
Expand All @@ -57,7 +67,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
Some(sym::io_stderr) => ("stderr", "e"),
_ => return,
};
let Some(format_args) = find_format_args(cx, write_arg, ExpnId::root()) else {
let Some(format_args) = self.format_args.get(cx, write_arg, ExpnId::root()) else {
return;
};

Expand All @@ -83,7 +93,7 @@ impl<'tcx> LateLintPass<'tcx> for ExplicitWrite {
};
let mut applicability = Applicability::MachineApplicable;
let inputs_snippet =
snippet_with_applicability(cx, format_args_inputs_span(&format_args), "..", &mut applicability);
snippet_with_applicability(cx, format_args_inputs_span(format_args), "..", &mut applicability);
span_lint_and_sugg(
cx,
EXPLICIT_WRITE,
Expand Down
19 changes: 15 additions & 4 deletions clippy_lints/src/format.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::{find_format_arg_expr, find_format_args, root_macro_call_first_node};
use clippy_utils::macros::{find_format_arg_expr, root_macro_call_first_node, FormatArgsStorage};
use clippy_utils::source::{snippet_opt, snippet_with_context};
use clippy_utils::sugg::Sugg;
use rustc_ast::{FormatArgsPiece, FormatOptions, FormatTrait};
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::declare_lint_pass;
use rustc_session::impl_lint_pass;
use rustc_span::{sym, Span};

declare_clippy_lint! {
Expand Down Expand Up @@ -39,13 +39,24 @@ declare_clippy_lint! {
"useless use of `format!`"
}

declare_lint_pass!(UselessFormat => [USELESS_FORMAT]);
#[allow(clippy::module_name_repetitions)]
pub struct UselessFormat {
format_args: FormatArgsStorage,
}

impl UselessFormat {
pub fn new(format_args: FormatArgsStorage) -> Self {
Self { format_args }
}
}

impl_lint_pass!(UselessFormat => [USELESS_FORMAT]);

impl<'tcx> LateLintPass<'tcx> for UselessFormat {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let Some(macro_call) = root_macro_call_first_node(cx, expr)
&& cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id)
&& let Some(format_args) = find_format_args(cx, expr, macro_call.expn)
&& let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn)
{
let mut applicability = Applicability::MachineApplicable;
let call_site = macro_call.span;
Expand Down
13 changes: 8 additions & 5 deletions clippy_lints/src/format_args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use clippy_config::msrvs::{self, Msrv};
use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
use clippy_utils::is_diag_trait_item;
use clippy_utils::macros::{
find_format_arg_expr, find_format_args, format_arg_removal_span, format_placeholder_format_span, is_assert_macro,
is_format_macro, is_panic, matching_root_macro_call, root_macro_call_first_node, FormatParamUsage, MacroCall,
find_format_arg_expr, format_arg_removal_span, format_placeholder_format_span, is_assert_macro, is_format_macro,
is_panic, matching_root_macro_call, root_macro_call_first_node, FormatArgsStorage, FormatParamUsage, MacroCall,
};
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{implements_trait, is_type_lang_item};
Expand Down Expand Up @@ -167,15 +167,18 @@ impl_lint_pass!(FormatArgs => [
UNUSED_FORMAT_SPECS,
]);

#[allow(clippy::struct_field_names)]
pub struct FormatArgs {
format_args: FormatArgsStorage,
msrv: Msrv,
ignore_mixed: bool,
}

impl FormatArgs {
#[must_use]
pub fn new(msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
pub fn new(format_args: FormatArgsStorage, msrv: Msrv, allow_mixed_uninlined_format_args: bool) -> Self {
Self {
format_args,
msrv,
ignore_mixed: allow_mixed_uninlined_format_args,
}
Expand All @@ -186,13 +189,13 @@ 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)
&& let Some(format_args) = find_format_args(cx, expr, macro_call.expn)
&& let Some(format_args) = self.format_args.get(cx, expr, macro_call.expn)
{
let linter = FormatArgsExpr {
cx,
expr,
macro_call: &macro_call,
format_args: &format_args,
format_args,
ignore_mixed: self.ignore_mixed,
};

Expand Down
10 changes: 7 additions & 3 deletions clippy_lints/src/format_impl.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
use clippy_utils::macros::{find_format_arg_expr, find_format_args, is_format_macro, root_macro_call_first_node};
use clippy_utils::macros::{find_format_arg_expr, is_format_macro, root_macro_call_first_node, FormatArgsStorage};
use clippy_utils::{get_parent_as_impl, is_diag_trait_item, path_to_local, peel_ref_operators};
use rustc_ast::{FormatArgsPiece, FormatTrait};
use rustc_errors::Applicability;
Expand Down Expand Up @@ -99,13 +99,15 @@ struct FormatTraitNames {

#[derive(Default)]
pub struct FormatImpl {
format_args: FormatArgsStorage,
// Whether we are inside Display or Debug trait impl - None for neither
format_trait_impl: Option<FormatTraitNames>,
}

impl FormatImpl {
pub fn new() -> Self {
pub fn new(format_args: FormatArgsStorage) -> Self {
Self {
format_args,
format_trait_impl: None,
}
}
Expand All @@ -129,6 +131,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl {
if let Some(format_trait_impl) = self.format_trait_impl {
let linter = FormatImplExpr {
cx,
format_args: &self.format_args,
expr,
format_trait_impl,
};
Expand All @@ -141,6 +144,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatImpl {

struct FormatImplExpr<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
format_args: &'a FormatArgsStorage,
expr: &'tcx Expr<'tcx>,
format_trait_impl: FormatTraitNames,
}
Expand Down Expand Up @@ -175,7 +179,7 @@ impl<'a, 'tcx> FormatImplExpr<'a, 'tcx> {
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)
&& let Some(format_args) = find_format_args(self.cx, self.expr, outer_macro.expn)
&& let Some(format_args) = self.format_args.get(self.cx, self.expr, outer_macro.expn)
{
for piece in &format_args.template {
if let FormatArgsPiece::Placeholder(placeholder) = piece
Expand Down
42 changes: 30 additions & 12 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ extern crate clippy_utils;
#[macro_use]
extern crate declare_clippy_lint;

use std::collections::BTreeMap;

use rustc_data_structures::fx::FxHashSet;
use rustc_lint::{Lint, LintId};

#[cfg(feature = "internal")]
pub mod deprecated_lints;
#[cfg_attr(feature = "internal", allow(clippy::missing_clippy_version_attribute))]
Expand Down Expand Up @@ -384,6 +379,10 @@ mod zero_sized_map_values;
// end lints modules, do not remove this comment, it’s used in `update_lints`

use clippy_config::{get_configuration_metadata, Conf};
use clippy_utils::macros::FormatArgsStorage;
use rustc_data_structures::fx::FxHashSet;
use rustc_lint::{Lint, LintId};
use std::collections::BTreeMap;

/// Register all pre expansion lints
///
Expand Down Expand Up @@ -614,6 +613,14 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
}
}

let format_args_storage = FormatArgsStorage::default();
let format_args = format_args_storage.clone();
store.register_early_pass(move || {
Box::new(utils::format_args_collector::FormatArgsCollector::new(
format_args.clone(),
))
});

// all the internal lints
#[cfg(feature = "internal")]
{
Expand Down Expand Up @@ -654,7 +661,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
.collect(),
))
});
store.register_early_pass(|| Box::<utils::format_args_collector::FormatArgsCollector>::default());
store.register_late_pass(|_| Box::new(utils::dump_hir::DumpHir));
store.register_late_pass(|_| Box::new(utils::author::Author));
store.register_late_pass(move |_| {
Expand Down Expand Up @@ -696,13 +702,15 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(non_octal_unix_permissions::NonOctalUnixPermissions));
store.register_early_pass(|| Box::new(unnecessary_self_imports::UnnecessarySelfImports));
store.register_late_pass(move |_| Box::new(approx_const::ApproxConstant::new(msrv())));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| {
Box::new(methods::Methods::new(
avoid_breaking_exported_api,
msrv(),
allow_expect_in_tests,
allow_unwrap_in_tests,
allowed_dotfiles.clone(),
format_args.clone(),
))
});
store.register_late_pass(move |_| Box::new(matches::Matches::new(msrv())));
Expand Down Expand Up @@ -766,7 +774,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::<regex::Regex>::default());
store.register_late_pass(move |_| Box::new(copies::CopyAndPaste::new(ignore_interior_mutability.clone())));
store.register_late_pass(|_| Box::new(copy_iterator::CopyIterator));
store.register_late_pass(|_| Box::new(format::UselessFormat));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| Box::new(format::UselessFormat::new(format_args.clone())));
store.register_late_pass(|_| Box::new(swap::Swap));
store.register_late_pass(|_| Box::new(overflow_check_conditional::OverflowCheckConditional));
store.register_late_pass(|_| Box::<new_without_default::NewWithoutDefault>::default());
Expand All @@ -790,7 +799,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(partialeq_ne_impl::PartialEqNeImpl));
store.register_late_pass(|_| Box::new(unused_io_amount::UnusedIoAmount));
store.register_late_pass(move |_| Box::new(large_enum_variant::LargeEnumVariant::new(enum_variant_size_threshold)));
store.register_late_pass(|_| Box::new(explicit_write::ExplicitWrite));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| Box::new(explicit_write::ExplicitWrite::new(format_args.clone())));
store.register_late_pass(|_| Box::new(needless_pass_by_value::NeedlessPassByValue));
store.register_late_pass(move |tcx| {
Box::new(pass_by_ref_or_value::PassByRefOrValue::new(
Expand Down Expand Up @@ -832,7 +842,8 @@ 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()));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| Box::new(format_impl::FormatImpl::new(format_args.clone())));
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 @@ -958,8 +969,14 @@ 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)));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| {
Box::new(format_args::FormatArgs::new(
format_args.clone(),
msrv(),
allow_mixed_uninlined_format_args,
))
});
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 All @@ -970,7 +987,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(|_| Box::new(default_union_representation::DefaultUnionRepresentation));
store.register_late_pass(|_| Box::<only_used_in_recursion::OnlyUsedInRecursion>::default());
store.register_late_pass(move |_| Box::new(dbg_macro::DbgMacro::new(allow_dbg_in_tests)));
store.register_late_pass(move |_| Box::new(write::Write::new(allow_print_in_tests)));
let format_args = format_args_storage.clone();
store.register_late_pass(move |_| Box::new(write::Write::new(format_args.clone(), allow_print_in_tests)));
store.register_late_pass(move |_| {
Box::new(cargo::Cargo {
ignore_publish: cargo_ignore_publish,
Expand Down
7 changes: 4 additions & 3 deletions clippy_lints/src/methods/expect_fun_call.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::macros::{find_format_args, format_args_inputs_span, root_macro_call_first_node};
use clippy_utils::macros::{format_args_inputs_span, root_macro_call_first_node, FormatArgsStorage};
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item};
use rustc_errors::Applicability;
Expand All @@ -16,6 +16,7 @@ use super::EXPECT_FUN_CALL;
#[allow(clippy::too_many_lines)]
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
format_args_storage: &FormatArgsStorage,
expr: &hir::Expr<'_>,
method_span: Span,
name: &str,
Expand Down Expand Up @@ -134,9 +135,9 @@ pub(super) fn check<'tcx>(
// Special handling for `format!` as arg_root
if let Some(macro_call) = root_macro_call_first_node(cx, arg_root) {
if cx.tcx.is_diagnostic_item(sym::format_macro, macro_call.def_id)
&& let Some(format_args) = find_format_args(cx, arg_root, macro_call.expn)
&& let Some(format_args) = format_args_storage.get(cx, arg_root, macro_call.expn)
{
let span = format_args_inputs_span(&format_args);
let span = format_args_inputs_span(format_args);
let sugg = snippet_with_applicability(cx, span, "..", &mut applicability);
span_lint_and_sugg(
cx,
Expand Down
15 changes: 14 additions & 1 deletion clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ use bind_instead_of_map::BindInsteadOfMap;
use clippy_config::msrvs::{self, Msrv};
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::macros::FormatArgsStorage;
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, peel_blocks, return_ty};
pub use path_ends_with_ext::DEFAULT_ALLOWED_DOTFILES;
Expand Down Expand Up @@ -4087,12 +4088,14 @@ declare_clippy_lint! {
suspicious,
"is_empty() called on strings known at compile time"
}

pub struct Methods {
avoid_breaking_exported_api: bool,
msrv: Msrv,
allow_expect_in_tests: bool,
allow_unwrap_in_tests: bool,
allowed_dotfiles: FxHashSet<String>,
format_args: FormatArgsStorage,
}

impl Methods {
Expand All @@ -4103,6 +4106,7 @@ impl Methods {
allow_expect_in_tests: bool,
allow_unwrap_in_tests: bool,
mut allowed_dotfiles: FxHashSet<String>,
format_args: FormatArgsStorage,
) -> Self {
allowed_dotfiles.extend(DEFAULT_ALLOWED_DOTFILES.iter().map(ToString::to_string));

Expand All @@ -4112,6 +4116,7 @@ impl Methods {
allow_expect_in_tests,
allow_unwrap_in_tests,
allowed_dotfiles,
format_args,
}
}
}
Expand Down Expand Up @@ -4281,7 +4286,15 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
ExprKind::MethodCall(method_call, receiver, args, _) => {
let method_span = method_call.ident.span;
or_fun_call::check(cx, expr, method_span, method_call.ident.as_str(), receiver, args);
expect_fun_call::check(cx, expr, method_span, method_call.ident.as_str(), receiver, args);
expect_fun_call::check(
cx,
&self.format_args,
expr,
method_span,
method_call.ident.as_str(),
receiver,
args,
);
clone_on_copy::check(cx, expr, method_call.ident.name, receiver, args);
clone_on_ref_ptr::check(cx, expr, method_call.ident.name, receiver, args);
inefficient_to_string::check(cx, expr, method_call.ident.name, receiver, args);
Expand Down
Loading