From 53a1ff70e87ba92caddfbcc9feee8bc89e4a02fc Mon Sep 17 00:00:00 2001 From: Philipp Krones Date: Wed, 26 Feb 2025 19:46:01 +0100 Subject: [PATCH] Check os_str_display MSRV instead of feature This feature was stabilized, so the FormatArgs lints should check if the MSRV of the stabilization is met, rather than checking if the feature is enabled. --- clippy_lints/src/format_args.rs | 29 +++++++------- clippy_utils/src/msrvs.rs | 1 + .../ui/unnecessary_os_str_debug_formatting.rs | 1 - ...unnecessary_os_str_debug_formatting.stderr | 12 +++--- tests/ui/unnecessary_path_debug_formatting.rs | 7 ++-- .../unnecessary_path_debug_formatting.stderr | 38 ++++++++++++++----- 6 files changed, 54 insertions(+), 34 deletions(-) diff --git a/clippy_lints/src/format_args.rs b/clippy_lints/src/format_args.rs index ea617dd1bab8..1ba355938b68 100644 --- a/clippy_lints/src/format_args.rs +++ b/clippy_lints/src/format_args.rs @@ -15,6 +15,7 @@ use rustc_ast::{ FormatArgPosition, FormatArgPositionKind, FormatArgsPiece, FormatArgumentKind, FormatCount, FormatOptions, FormatPlaceholder, FormatTrait, }; +use rustc_attr_parsing::RustcVersion; use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; use rustc_errors::SuggestionStyle::{CompletelyHidden, ShowCode}; @@ -206,17 +207,17 @@ pub struct FormatArgs<'tcx> { format_args: FormatArgsStorage, msrv: Msrv, ignore_mixed: bool, - ty_feature_map: FxHashMap, Option>, + ty_msrv_map: FxHashMap, Option>, } impl<'tcx> FormatArgs<'tcx> { pub fn new(tcx: TyCtxt<'tcx>, conf: &'static Conf, format_args: FormatArgsStorage) -> Self { - let ty_feature_map = make_ty_feature_map(tcx); + let ty_msrv_map = make_ty_msrv_map(tcx); Self { format_args, msrv: conf.msrv.clone(), ignore_mixed: conf.allow_mixed_uninlined_format_args, - ty_feature_map, + ty_msrv_map, } } } @@ -233,7 +234,8 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs<'tcx> { macro_call: ¯o_call, format_args, ignore_mixed: self.ignore_mixed, - ty_feature_map: &self.ty_feature_map, + msrv: &self.msrv, + ty_msrv_map: &self.ty_msrv_map, }; linter.check_templates(); @@ -253,7 +255,8 @@ struct FormatArgsExpr<'a, 'tcx> { macro_call: &'a MacroCall, format_args: &'a rustc_ast::FormatArgs, ignore_mixed: bool, - ty_feature_map: &'a FxHashMap, Option>, + msrv: &'a Msrv, + ty_msrv_map: &'a FxHashMap, Option>, } impl<'tcx> FormatArgsExpr<'_, 'tcx> { @@ -538,19 +541,19 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> { fn can_display_format(&self, ty: Ty<'tcx>) -> bool { let ty = ty.peel_refs(); - if let Some(feature) = self.ty_feature_map.get(&ty) - && feature.is_none_or(|feature| self.cx.tcx.features().enabled(feature)) + if let Some(msrv) = self.ty_msrv_map.get(&ty) + && msrv.is_none_or(|msrv| self.msrv.meets(msrv)) { return true; } - // Even if `ty` is not in `self.ty_feature_map`, check whether `ty` implements `Deref` with - // a `Target` that is in `self.ty_feature_map`. + // Even if `ty` is not in `self.ty_msrv_map`, check whether `ty` implements `Deref` with + // a `Target` that is in `self.ty_msrv_map`. if let Some(deref_trait_id) = self.cx.tcx.lang_items().deref_trait() && implements_trait(self.cx, ty, deref_trait_id, &[]) && let Some(target_ty) = self.cx.get_associated_type(ty, deref_trait_id, "Target") - && let Some(feature) = self.ty_feature_map.get(&target_ty) - && feature.is_none_or(|feature| self.cx.tcx.features().enabled(feature)) + && let Some(msrv) = self.ty_msrv_map.get(&target_ty) + && msrv.is_none_or(|msrv| self.msrv.meets(msrv)) { return true; } @@ -559,8 +562,8 @@ impl<'tcx> FormatArgsExpr<'_, 'tcx> { } } -fn make_ty_feature_map(tcx: TyCtxt<'_>) -> FxHashMap, Option> { - [(sym::OsStr, Some(Symbol::intern("os_str_display"))), (sym::Path, None)] +fn make_ty_msrv_map(tcx: TyCtxt<'_>) -> FxHashMap, Option> { + [(sym::OsStr, Some(msrvs::OS_STR_DISPLAY)), (sym::Path, None)] .into_iter() .filter_map(|(name, feature)| { tcx.get_diagnostic_item(name).map(|def_id| { diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 8c9832af0a19..8a7a7d006352 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -19,6 +19,7 @@ macro_rules! msrv_aliases { // names may refer to stabilized feature flags or library items msrv_aliases! { + 1,87,0 { OS_STR_DISPLAY } 1,84,0 { CONST_OPTION_AS_SLICE } 1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP } 1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP } diff --git a/tests/ui/unnecessary_os_str_debug_formatting.rs b/tests/ui/unnecessary_os_str_debug_formatting.rs index 3c264e5fc598..12663ec9a528 100644 --- a/tests/ui/unnecessary_os_str_debug_formatting.rs +++ b/tests/ui/unnecessary_os_str_debug_formatting.rs @@ -1,4 +1,3 @@ -#![feature(os_str_display)] #![warn(clippy::unnecessary_debug_formatting)] use std::ffi::{OsStr, OsString}; diff --git a/tests/ui/unnecessary_os_str_debug_formatting.stderr b/tests/ui/unnecessary_os_str_debug_formatting.stderr index 382e59b04619..001309ab817a 100644 --- a/tests/ui/unnecessary_os_str_debug_formatting.stderr +++ b/tests/ui/unnecessary_os_str_debug_formatting.stderr @@ -1,5 +1,5 @@ error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_os_str_debug_formatting.rs:15:22 + --> tests/ui/unnecessary_os_str_debug_formatting.rs:14:22 | LL | println!("{:?}", os_str); | ^^^^^^ @@ -10,7 +10,7 @@ LL | println!("{:?}", os_str); = help: to override `-D warnings` add `#[allow(clippy::unnecessary_debug_formatting)]` error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_os_str_debug_formatting.rs:16:22 + --> tests/ui/unnecessary_os_str_debug_formatting.rs:15:22 | LL | println!("{:?}", os_string); | ^^^^^^^^^ @@ -19,7 +19,7 @@ LL | println!("{:?}", os_string); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_os_str_debug_formatting.rs:18:16 + --> tests/ui/unnecessary_os_str_debug_formatting.rs:17:16 | LL | println!("{os_str:?}"); | ^^^^^^ @@ -28,7 +28,7 @@ LL | println!("{os_str:?}"); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_os_str_debug_formatting.rs:19:16 + --> tests/ui/unnecessary_os_str_debug_formatting.rs:18:16 | LL | println!("{os_string:?}"); | ^^^^^^^^^ @@ -37,7 +37,7 @@ LL | println!("{os_string:?}"); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `format!` args - --> tests/ui/unnecessary_os_str_debug_formatting.rs:21:37 + --> tests/ui/unnecessary_os_str_debug_formatting.rs:20:37 | LL | let _: String = format!("{:?}", os_str); | ^^^^^^ @@ -46,7 +46,7 @@ LL | let _: String = format!("{:?}", os_str); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `format!` args - --> tests/ui/unnecessary_os_str_debug_formatting.rs:22:37 + --> tests/ui/unnecessary_os_str_debug_formatting.rs:21:37 | LL | let _: String = format!("{:?}", os_string); | ^^^^^^^^^ diff --git a/tests/ui/unnecessary_path_debug_formatting.rs b/tests/ui/unnecessary_path_debug_formatting.rs index 96ad2ee2bf23..02adeece2809 100644 --- a/tests/ui/unnecessary_path_debug_formatting.rs +++ b/tests/ui/unnecessary_path_debug_formatting.rs @@ -25,11 +25,10 @@ fn main() { println!("{}", path.display()); println!("{}", path_buf.display()); - // should not fire because feature `os_str_display` is not enabled - println!("{:?}", os_str); - println!("{:?}", os_string); - // positive tests + println!("{:?}", os_str); //~ unnecessary_debug_formatting + println!("{:?}", os_string); //~ unnecessary_debug_formatting + println!("{:?}", path); //~ unnecessary_debug_formatting println!("{:?}", path_buf); //~ unnecessary_debug_formatting diff --git a/tests/ui/unnecessary_path_debug_formatting.stderr b/tests/ui/unnecessary_path_debug_formatting.stderr index c9f649dbc0a1..f12fa72c84b3 100644 --- a/tests/ui/unnecessary_path_debug_formatting.stderr +++ b/tests/ui/unnecessary_path_debug_formatting.stderr @@ -1,16 +1,34 @@ error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_path_debug_formatting.rs:33:22 + --> tests/ui/unnecessary_path_debug_formatting.rs:29:22 + | +LL | println!("{:?}", os_str); + | ^^^^^^ + | + = help: use `Display` formatting and change this to `os_str.display()` + = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed + = note: `-D clippy::unnecessary-debug-formatting` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_debug_formatting)]` + +error: unnecessary `Debug` formatting in `println!` args + --> tests/ui/unnecessary_path_debug_formatting.rs:30:22 + | +LL | println!("{:?}", os_string); + | ^^^^^^^^^ + | + = help: use `Display` formatting and change this to `os_string.display()` + = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed + +error: unnecessary `Debug` formatting in `println!` args + --> tests/ui/unnecessary_path_debug_formatting.rs:32:22 | LL | println!("{:?}", path); | ^^^^ | = help: use `Display` formatting and change this to `path.display()` = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed - = note: `-D clippy::unnecessary-debug-formatting` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::unnecessary_debug_formatting)]` error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_path_debug_formatting.rs:34:22 + --> tests/ui/unnecessary_path_debug_formatting.rs:33:22 | LL | println!("{:?}", path_buf); | ^^^^^^^^ @@ -19,7 +37,7 @@ LL | println!("{:?}", path_buf); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_path_debug_formatting.rs:36:16 + --> tests/ui/unnecessary_path_debug_formatting.rs:35:16 | LL | println!("{path:?}"); | ^^^^ @@ -28,7 +46,7 @@ LL | println!("{path:?}"); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_path_debug_formatting.rs:37:16 + --> tests/ui/unnecessary_path_debug_formatting.rs:36:16 | LL | println!("{path_buf:?}"); | ^^^^^^^^ @@ -37,7 +55,7 @@ LL | println!("{path_buf:?}"); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `format!` args - --> tests/ui/unnecessary_path_debug_formatting.rs:39:37 + --> tests/ui/unnecessary_path_debug_formatting.rs:38:37 | LL | let _: String = format!("{:?}", path); | ^^^^ @@ -46,7 +64,7 @@ LL | let _: String = format!("{:?}", path); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `format!` args - --> tests/ui/unnecessary_path_debug_formatting.rs:40:37 + --> tests/ui/unnecessary_path_debug_formatting.rs:39:37 | LL | let _: String = format!("{:?}", path_buf); | ^^^^^^^^ @@ -55,7 +73,7 @@ LL | let _: String = format!("{:?}", path_buf); = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed error: unnecessary `Debug` formatting in `println!` args - --> tests/ui/unnecessary_path_debug_formatting.rs:43:22 + --> tests/ui/unnecessary_path_debug_formatting.rs:42:22 | LL | println!("{:?}", &*deref_path); | ^^^^^^^^^^^^ @@ -63,5 +81,5 @@ LL | println!("{:?}", &*deref_path); = help: use `Display` formatting and change this to `&*deref_path.display()` = note: switching to `Display` formatting will change how the value is shown; escaped characters will no longer be escaped and surrounding quotes will be removed -error: aborting due to 7 previous errors +error: aborting due to 9 previous errors