feat: Implement concealed_obvious_default lint

Co-authored-by: Samuel Tardieu <[email protected]>

Author: nik-rev

CHANGELOG.md

@@ -5694,6 +5694,7 @@ Released 2018-09-13
 [`collection_is_never_read`]: https://rust-lang.github.io/rust-clippy/master/index.html#collection_is_never_read
 [`comparison_chain`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_chain
 [`comparison_to_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#comparison_to_empty
+[`concealed_obvious_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#concealed_obvious_default
 [`confusing_method_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#confusing_method_to_numeric_cast
 [`const_is_empty`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_is_empty
 [`const_static_lifetime`]: https://rust-lang.github.io/rust-clippy/master/index.html#const_static_lifetime

clippy_config/src/conf.rs

@@ -1097,7 +1097,7 @@ impl serde::de::Error for FieldError {
             writeln!(msg).unwrap();
             for (column, column_width) in column_widths.iter().copied().enumerate() {
                 let index = column * rows + row;
-                let field = expected.get(index).copied().unwrap_or_default();
+                let field = expected.get(index).copied().unwrap_or("");
                 write!(msg, "{:SEPARATOR_WIDTH$}{field:column_width$}", " ").unwrap();
             }
         }
@@ -1128,7 +1128,7 @@ fn calculate_dimensions(fields: &[&str]) -> (usize, Vec<usize>) {
                 (0..rows)
                     .map(|row| {
                         let index = column * rows + row;
-                        let field = fields.get(index).copied().unwrap_or_default();
+                        let field = fields.get(index).copied().unwrap_or("");
                         field.len()
                     })
                     .max()

clippy_lints/src/arbitrary_source_item_ordering.rs

@@ -414,7 +414,7 @@ impl<'tcx> LateLintPass<'tcx> for ArbitrarySourceItemOrdering {
             let module_level_order = self
                 .module_item_order_groupings
                 .module_level_order_of(&item_kind)
-                .unwrap_or_default();
+                .unwrap_or(0);
 
             if let Some(cur_t) = cur_t.as_ref() {
                 use std::cmp::Ordering; // Better legibility.

clippy_lints/src/casts/cast_possible_truncation.rs

@@ -49,7 +49,7 @@ fn apply_reductions(cx: &LateContext<'_>, nbits: u64, expr: &Expr<'_>, signed: b
                 .min(apply_reductions(cx, nbits, right, signed))
                 .min(apply_reductions(cx, nbits, left, signed)),
             BinOpKind::Shr => apply_reductions(cx, nbits, left, signed)
-                .saturating_sub(constant_int(cx, right).map_or(0, |s| u64::try_from(s).unwrap_or_default())),
+                .saturating_sub(constant_int(cx, right).map_or(0, |s| u64::try_from(s).unwrap_or(0))),
             _ => nbits,
         },
         ExprKind::MethodCall(method, left, [right], _) => {

clippy_lints/src/declared_lints.rs

@@ -359,6 +359,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::CLONE_ON_COPY_INFO,
     crate::methods::CLONE_ON_REF_PTR_INFO,
     crate::methods::COLLAPSIBLE_STR_REPLACE_INFO,
+    crate::methods::CONCEALED_OBVIOUS_DEFAULT_INFO,
     crate::methods::CONST_IS_EMPTY_INFO,
     crate::methods::DOUBLE_ENDED_ITERATOR_LAST_INFO,
     crate::methods::DRAIN_COLLECT_INFO,

clippy_lints/src/manual_strip.rs

@@ -269,7 +269,7 @@ fn find_stripping<'tcx>(
 
         fn visit_pat(&mut self, pat: &'tcx rustc_hir::Pat<'tcx>) -> Self::Result {
             if let PatKind::Binding(_, _, ident, _) = pat.kind {
-                *self.bindings.entry(ident.name).or_default() += 1;
+                *self.bindings.entry(ident.name).or_insert(0) += 1;
             }
             walk_pat(self, pat);
         }

clippy_lints/src/methods/concealed_obvious_default.rs

@@ -0,0 +1,133 @@
+use rustc_span::Symbol;
+
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::sym;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_middle::ty::{self, Ty};
+use rustc_span::Span;
+
+use super::CONCEALED_OBVIOUS_DEFAULT;
+
+pub(super) fn check(cx: &LateContext<'_>, recv: &hir::Expr<'_>, method_name: Symbol, call_span: Span) {
+    // Type of the expression which invoked the method
+    let recv_ty = cx.typeck_results().expr_ty(recv);
+
+    // Only consider algebraic data types e.g. an `Option`.
+    // Their generics are represented by `generic_args`
+    if let ty::Adt(adt, generic_args) = recv_ty.kind()
+        // `name_of_generic`, is e.g. a `sym::Option`
+        && let Some(name_of_generic) = cx.tcx.get_diagnostic_name(adt.did())
+        && let Some((message, suggestion)) = CONCEALING_METHODS.into_iter().find(|concealing| {
+            name_of_generic == concealing.ty && method_name == concealing.method
+        }).and_then(|concealing| {
+            let ty = generic_args.type_at(concealing.generic_index);
+            extract_obvious_default(cx, ty).map(|(default, ty)| {
+                let method = (concealing.fmt_msg)(ty);
+                (
+                    format!("method {method} conceals the underlying type"),
+                    (concealing.fmt_sugg)(default),
+                )
+            })
+        })
+    {
+        span_lint_and_sugg(
+            cx,
+            CONCEALED_OBVIOUS_DEFAULT,
+            call_span,
+            message,
+            "write the default type explicitly".to_string(),
+            suggestion,
+            Applicability::MachineApplicable,
+        );
+    }
+}
+
+/// Method which conceals an underlying type of a generic
+struct ConcealingMethod {
+    /// Generic which contains the concealing method, e.g. `Option<T>`
+    ty: Symbol,
+    /// Index of the concealed generic type, e.g. `0` for `Option<T>`
+    generic_index: usize,
+    /// The concealing method, e.g. `unwrap_or_default`
+    method: Symbol,
+    /// Format the lint's message
+    ///
+    /// Receives the concealed type, e.g. for `Option<bool>` receives `bool`
+    fmt_msg: fn(&'static str) -> String,
+    /// Format the lint's suggestion
+    ///
+    /// Receives the default of the concealed type, e.g. for `Option<bool>` receives `false`,
+    /// as `bool::default()` is `false`
+    fmt_sugg: fn(&'static str) -> String,
+}
+
+/// List of methods which use `Default` trait under the hood,
+/// but they have an alternative non-`Default` method
+///
+/// For example, there is `Option::unwrap_or_default` which is almost the same
+/// as `Option::unwrap_or`, but the type does not have to be provided and the
+/// `Default` implementation is used.
+const CONCEALING_METHODS: [ConcealingMethod; 4] = [
+    ConcealingMethod {
+        ty: sym::Result,
+        // Result<T, E>
+        //        ^ want
+        generic_index: 0,
+        method: sym::unwrap_or_default,
+        fmt_msg: |ty| format!("Result::<{ty}, _>::unwrap_or_default()"),
+        fmt_sugg: |val| format!("unwrap_or({val})"),
+    },
+    ConcealingMethod {
+        ty: sym::Option,
+        // Option<T>
+        //        ^ want
+        generic_index: 0,
+        method: sym::unwrap_or_default,
+        fmt_msg: |ty| format!("Option::<{ty}>::unwrap_or_default()"),
+        fmt_sugg: |val| format!("unwrap_or({val})"),
+    },
+    ConcealingMethod {
+        ty: sym::HashMapEntry,
+        // Entry<'a, K, V, A = Global>
+        //              ^ want
+        generic_index: 2,
+        method: sym::or_default,
+        fmt_msg: |ty| format!("hash_map::Entry::<'_, _, {ty}>::or_default()"),
+        fmt_sugg: |val| format!("or_insert({val})"),
+    },
+    ConcealingMethod {
+        ty: sym::BTreeEntry,
+        // Entry<'a, K, V, A = Global>
+        //              ^ want
+        generic_index: 2,
+        method: sym::or_default,
+        fmt_msg: |ty| format!("btree_map::Entry::<'_, _, {ty}>::or_default()"),
+        fmt_sugg: |val| format!("or_insert({val})"),
+    },
+];
+
+/// Get default value of a type with an obvious default.
+///
+/// # Returns
+///
+/// If the type has an obvious default:
+///
+/// - Default for the type
+/// - The type as it should be displayed in the lint message
+///
+/// If the type is not considered to have an obvious default, return `None`.
+fn extract_obvious_default(cx: &LateContext<'_>, ty: Ty<'_>) -> Option<(&'static str, &'static str)> {
+    match ty.peel_refs().kind() {
+        ty::Int(ty) => Some(("0", ty.name_str())),
+        ty::Uint(ty) => Some(("0", ty.name_str())),
+        ty::Float(ty) => Some(("0.0", ty.name_str())),
+        ty::Char => Some((r"'\0'", "char")),
+        ty::Str => Some((r#""""#, "&str")),
+        ty::Bool => Some(("false", "bool")),
+        ty::Tuple(tys) if tys.is_empty() => Some(("()", "()")),
+        ty::Adt(def, _) if cx.tcx.get_diagnostic_name(def.did()) == Some(sym::Option) => Some(("None", "Option<_>")),
+        _ => None,
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -14,6 +14,7 @@ mod clone_on_copy;
 mod clone_on_ref_ptr;
 mod cloned_instead_of_copied;
 mod collapsible_str_replace;
+mod concealed_obvious_default;
 mod double_ended_iterator_last;
 mod drain_collect;
 mod err_expect;
@@ -4565,6 +4566,46 @@ declare_clippy_lint! {
     "hardcoded localhost IP address"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for usages of methods like `Option::unwrap_or_default`,
+    /// for types where the `Default` implementation is obvious and can be written literally
+    ///
+    /// ### Why is this bad?
+    ///
+    /// You have to know what the underlying type is, instead of it being obvious from a glance
+    ///
+    /// ### Example
+    /// ```no_run
+    /// fn f(
+    ///     a: Option<&str>,
+    ///     b: Option<usize>,
+    ///     c: Option<Option<bool>>
+    /// ) {
+    ///     let a = a.unwrap_or_default();
+    ///     let b = b.unwrap_or_default();
+    ///     let c = c.unwrap_or_default();
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// fn f(
+    ///     a: Option<&str>,
+    ///     b: Option<usize>,
+    ///     c: Option<Option<bool>>
+    /// ) {
+    ///     let a = a.unwrap_or("");
+    ///     let b = b.unwrap_or(0);
+    ///     let c = c.unwrap_or(None);
+    /// }
+    /// ```
+    #[clippy::version = "1.89.0"]
+    pub CONCEALED_OBVIOUS_DEFAULT,
+    complexity,
+    ".*or_default() obscures the underlying type"
+}
+
 #[expect(clippy::struct_excessive_bools)]
 pub struct Methods {
     avoid_breaking_exported_api: bool,
@@ -4744,6 +4785,7 @@ impl_lint_pass!(Methods => [
     IO_OTHER_ERROR,
     SWAP_WITH_TEMPORARY,
     IP_CONSTANT,
+    CONCEALED_OBVIOUS_DEFAULT
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -5505,6 +5547,10 @@ impl Methods {
                         _ => {},
                     }
                     unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
+                    concealed_obvious_default::check(cx, recv, name, call_span);
+                },
+                (sym::or_default, []) => {
+                    concealed_obvious_default::check(cx, recv, name, call_span);
                 },
                 (sym::unwrap_or_else, [u_arg]) => {
                     match method_call(recv) {

clippy_lints/src/operators/arithmetic_side_effects.rs

@@ -64,9 +64,9 @@ impl ArithmeticSideEffects {
     /// "multiplication" are present in the inner set of allowed types.
     fn has_allowed_binary(&self, lhs_ty: Ty<'_>, rhs_ty: Ty<'_>) -> bool {
         let lhs_ty_string = lhs_ty.to_string();
-        let lhs_ty_string_elem = lhs_ty_string.split('<').next().unwrap_or_default();
+        let lhs_ty_string_elem = lhs_ty_string.split('<').next().unwrap_or("");
         let rhs_ty_string = rhs_ty.to_string();
-        let rhs_ty_string_elem = rhs_ty_string.split('<').next().unwrap_or_default();
+        let rhs_ty_string_elem = rhs_ty_string.split('<').next().unwrap_or("");
         if let Some(rhs_from_specific) = self.allowed_binary.get(lhs_ty_string_elem)
             && {
                 let rhs_has_allowed_ty = rhs_from_specific.contains(rhs_ty_string_elem);
@@ -85,7 +85,7 @@ impl ArithmeticSideEffects {
     /// allowed types.
     fn has_allowed_unary(&self, ty: Ty<'_>) -> bool {
         let ty_string = ty.to_string();
-        let ty_string_elem = ty_string.split('<').next().unwrap_or_default();
+        let ty_string_elem = ty_string.split('<').next().unwrap_or("");
         self.allowed_unary.contains(ty_string_elem)
     }
 

clippy_lints/src/utils/author.rs

@@ -216,7 +216,7 @@ impl<'a, 'tcx> PrintVisitor<'a, 'tcx> {
 
     fn next(&self, s: &'static str) -> String {
         let mut ids = self.ids.take();
-        let out = match *ids.entry(s).and_modify(|n| *n += 1).or_default() {
+        let out = match *ids.entry(s).and_modify(|n| *n += 1).or_insert(0) {
             // first usage of the name, use it as is
             0 => s.to_string(),
             // append a number starting with 1

clippy_utils/src/lib.rs

@@ -2770,7 +2770,7 @@ pub fn tokenize_with_text(s: &str) -> impl Iterator<Item = (TokenKind, &str, Inn
         let range = pos as usize..end as usize;
         let inner = InnerSpan::new(range.start, range.end);
         pos = end;
-        (t.kind, s.get(range).unwrap_or_default(), inner)
+        (t.kind, s.get(range).unwrap_or(""), inner)
     })
 }
 

clippy_utils/src/ty/mod.rs

@@ -934,7 +934,7 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
     match (cx.layout_of(ty).map(|layout| layout.size.bytes()), ty.kind()) {
         (Ok(size), _) => size,
         (Err(_), ty::Tuple(list)) => list.iter().map(|t| approx_ty_size(cx, t)).sum(),
-        (Err(_), ty::Array(t, n)) => n.try_to_target_usize(cx.tcx).unwrap_or_default() * approx_ty_size(cx, *t),
+        (Err(_), ty::Array(t, n)) => n.try_to_target_usize(cx.tcx).unwrap_or(0) * approx_ty_size(cx, *t),
         (Err(_), ty::Adt(def, subst)) if def.is_struct() => def
             .variants()
             .iter()
@@ -955,7 +955,7 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
                     .sum::<u64>()
             })
             .max()
-            .unwrap_or_default(),
+            .unwrap_or(0),
         (Err(_), ty::Adt(def, subst)) if def.is_union() => def
             .variants()
             .iter()
@@ -964,10 +964,10 @@ pub fn approx_ty_size<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> u64 {
                     .iter()
                     .map(|field| approx_ty_size(cx, field.ty(cx.tcx, subst)))
                     .max()
-                    .unwrap_or_default()
+                    .unwrap_or(0)
             })
             .max()
-            .unwrap_or_default(),
+            .unwrap_or(0),
         (Err(_), _) => 0,
     }
 }

src/driver.rs

@@ -267,7 +267,7 @@ pub fn main() {
         let clippy_args_var = env::var("CLIPPY_ARGS").ok();
         let clippy_args = clippy_args_var
             .as_deref()
-            .unwrap_or_default()
+            .unwrap_or("")
             .split("__CLIPPY_HACKERY__")
             .filter_map(|s| match s {
                 "" => None,