Add unnecessary_debug_formatting lint

Address review comments

Fix adjacent code

Required now that the lint is pedantic

Add inline formatting tests

Add note re formatting changes

Address `unnecessary_map_or` warnings

Address additional review comments

Typo

Update Clippy version

clippy_lints/src/declared_lints.rs

@@ -191,6 +191,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::format_args::FORMAT_IN_FORMAT_ARGS_INFO,
     crate::format_args::TO_STRING_IN_FORMAT_ARGS_INFO,
     crate::format_args::UNINLINED_FORMAT_ARGS_INFO,
+    crate::format_args::UNNECESSARY_DEBUG_FORMATTING_INFO,
     crate::format_args::UNUSED_FORMAT_SPECS_INFO,
     crate::format_impl::PRINT_IN_FORMAT_IMPL_INFO,
     crate::format_impl::RECURSIVE_FORMAT_IMPL_INFO,

clippy_lints/src/format_args.rs

@@ -1,26 +1,27 @@
 use arrayvec::ArrayVec;
 use clippy_config::Conf;
 use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
-use clippy_utils::is_diag_trait_item;
 use clippy_utils::macros::{
     FormatArgsStorage, 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,
 };
 use clippy_utils::msrvs::{self, Msrv};
-use clippy_utils::source::SpanRangeExt;
+use clippy_utils::source::{SpanRangeExt, snippet};
 use clippy_utils::ty::{implements_trait, is_type_lang_item};
+use clippy_utils::{is_diag_trait_item, is_from_proc_macro};
 use itertools::Itertools;
 use rustc_ast::{
     FormatArgPosition, FormatArgPositionKind, FormatArgsPiece, FormatArgumentKind, FormatCount, FormatOptions,
     FormatPlaceholder, FormatTrait,
 };
+use rustc_data_structures::fx::FxHashMap;
 use rustc_errors::Applicability;
 use rustc_errors::SuggestionStyle::{CompletelyHidden, ShowCode};
 use rustc_hir::{Expr, ExprKind, LangItem};
 use rustc_lint::{LateContext, LateLintPass, LintContext};
-use rustc_middle::ty::Ty;
 use rustc_middle::ty::adjustment::{Adjust, Adjustment};
+use rustc_middle::ty::{List, Ty, TyCtxt};
 use rustc_session::impl_lint_pass;
 use rustc_span::edition::Edition::Edition2021;
 use rustc_span::{Span, Symbol, sym};
@@ -50,6 +51,36 @@ declare_clippy_lint! {
     "`format!` used in a macro that does formatting"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `Debug` formatting (`{:?}`) applied to an `OsStr` or `Path`.
+    ///
+    /// ### Why is this bad?
+    /// Rust doesn't guarantee what `Debug` formatting looks like, and it could
+    /// change in the future. `OsStr`s and `Path`s can be `Display` formatted
+    /// using their `display` methods.
+    ///
+    /// Furthermore, with `Debug` formatting, certain characters are escaped.
+    /// Thus, a `Debug` formatted `Path` is less likely to be clickable.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// # use std::path::Path;
+    /// let path = Path::new("...");
+    /// println!("The path is {:?}", path);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// # use std::path::Path;
+    /// let path = Path::new("…");
+    /// println!("The path is {}", path.display());
+    /// ```
+    #[clippy::version = "1.87.0"]
+    pub UNNECESSARY_DEBUG_FORMATTING,
+    pedantic,
+    "`Debug` formatting applied to an `OsStr` or `Path` when `.display()` is available"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for [`ToString::to_string`](https://doc.rust-lang.org/std/string/trait.ToString.html#tymethod.to_string)
@@ -162,31 +193,35 @@ declare_clippy_lint! {
     "use of a format specifier that has no effect"
 }
 
-impl_lint_pass!(FormatArgs => [
+impl_lint_pass!(FormatArgs<'_> => [
     FORMAT_IN_FORMAT_ARGS,
     TO_STRING_IN_FORMAT_ARGS,
     UNINLINED_FORMAT_ARGS,
+    UNNECESSARY_DEBUG_FORMATTING,
     UNUSED_FORMAT_SPECS,
 ]);
 
 #[allow(clippy::struct_field_names)]
-pub struct FormatArgs {
+pub struct FormatArgs<'tcx> {
     format_args: FormatArgsStorage,
     msrv: Msrv,
     ignore_mixed: bool,
+    ty_feature_map: FxHashMap<Ty<'tcx>, Option<Symbol>>,
 }
 
-impl FormatArgs {
-    pub fn new(conf: &'static Conf, format_args: FormatArgsStorage) -> Self {
+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);
         Self {
             format_args,
             msrv: conf.msrv.clone(),
             ignore_mixed: conf.allow_mixed_uninlined_format_args,
+            ty_feature_map,
         }
     }
 }
 
-impl<'tcx> LateLintPass<'tcx> for FormatArgs {
+impl<'tcx> LateLintPass<'tcx> for FormatArgs<'tcx> {
     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)
@@ -198,6 +233,7 @@ impl<'tcx> LateLintPass<'tcx> for FormatArgs {
                 macro_call: &macro_call,
                 format_args,
                 ignore_mixed: self.ignore_mixed,
+                ty_feature_map: &self.ty_feature_map,
             };
 
             linter.check_templates();
@@ -217,9 +253,10 @@ struct FormatArgsExpr<'a, 'tcx> {
     macro_call: &'a MacroCall,
     format_args: &'a rustc_ast::FormatArgs,
     ignore_mixed: bool,
+    ty_feature_map: &'a FxHashMap<Ty<'tcx>, Option<Symbol>>,
 }
 
-impl FormatArgsExpr<'_, '_> {
+impl<'tcx> FormatArgsExpr<'_, 'tcx> {
     fn check_templates(&self) {
         for piece in &self.format_args.template {
             if let FormatArgsPiece::Placeholder(placeholder) = piece
@@ -237,6 +274,11 @@ impl FormatArgsExpr<'_, '_> {
                     self.check_format_in_format_args(name, arg_expr);
                     self.check_to_string_in_format_args(name, arg_expr);
                 }
+
+                if placeholder.format_trait == FormatTrait::Debug {
+                    let name = self.cx.tcx.item_name(self.macro_call.def_id);
+                    self.check_unnecessary_debug_formatting(name, arg_expr);
+                }
             }
         }
     }
@@ -439,6 +481,33 @@ impl FormatArgsExpr<'_, '_> {
         }
     }
 
+    fn check_unnecessary_debug_formatting(&self, name: Symbol, value: &Expr<'tcx>) {
+        let cx = self.cx;
+        if !value.span.from_expansion()
+            && !is_from_proc_macro(cx, value)
+            && let ty = cx.typeck_results().expr_ty(value)
+            && self.can_display_format(ty)
+        {
+            let snippet = snippet(cx.sess(), value.span, "..");
+            span_lint_and_then(
+                cx,
+                UNNECESSARY_DEBUG_FORMATTING,
+                value.span,
+                format!("unnecessary `Debug` formatting in `{name}!` args"),
+                |diag| {
+                    diag.help(format!(
+                        "use `Display` formatting and change this to `{snippet}.display()`"
+                    ));
+                    diag.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",
+                    );
+                },
+            );
+        }
+    }
+
     fn format_arg_positions(&self) -> impl Iterator<Item = (&FormatArgPosition, FormatParamUsage)> {
         self.format_args.template.iter().flat_map(|piece| match piece {
             FormatArgsPiece::Placeholder(placeholder) => {
@@ -465,6 +534,41 @@ impl FormatArgsExpr<'_, '_> {
             .at_most_one()
             .is_err()
     }
+
+    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))
+        {
+            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`.
+        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))
+        {
+            return true;
+        }
+
+        false
+    }
+}
+
+fn make_ty_feature_map(tcx: TyCtxt<'_>) -> FxHashMap<Ty<'_>, Option<Symbol>> {
+    [(sym::OsStr, Some(Symbol::intern("os_str_display"))), (sym::Path, None)]
+        .into_iter()
+        .filter_map(|(name, feature)| {
+            tcx.get_diagnostic_item(name).map(|def_id| {
+                let ty = Ty::new_adt(tcx, tcx.adt_def(def_id), List::empty());
+                (ty, feature)
+            })
+        })
+        .collect()
 }
 
 fn count_needed_derefs<'tcx, I>(mut ty: Ty<'tcx>, mut iter: I) -> (usize, Ty<'tcx>)

clippy_lints/src/lib.rs

@@ -835,7 +835,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| Box::new(non_send_fields_in_send_ty::NonSendFieldInSendTy::new(conf)));
     store.register_late_pass(move |_| Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::new(conf)));
     let format_args = format_args_storage.clone();
-    store.register_late_pass(move |_| Box::new(format_args::FormatArgs::new(conf, format_args.clone())));
+    store.register_late_pass(move |tcx| Box::new(format_args::FormatArgs::new(tcx, conf, format_args.clone())));
     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));