feat: new lint concealed_obvious_default

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_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/methods/concealed_obvious_default.rs

@@ -0,0 +1,140 @@
+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);
+
+    // Option::<bool>::unwrap_or_default()
+    //                 ^^^^^^^^^^^^^^^^^^^
+    // if the call comes from expansion, bail
+    if call_span.from_expansion() {
+        return;
+    }
+
+    // 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.
@@ -5544,7 +5586,7 @@ impl Methods {
             }
         }
         // Handle method calls whose receiver and arguments may come from expansion
-        if let ExprKind::MethodCall(path, recv, args, _call_span) = expr.kind {
+        if let ExprKind::MethodCall(path, recv, args, call_span) = expr.kind {
             match (path.ident.name, args) {
                 (sym::expect, [_]) if !matches!(method_call(recv), Some((sym::ok | sym::err, _, [], _, _))) => {
                     unwrap_expect_used::check(
@@ -5579,6 +5621,9 @@ impl Methods {
                         unwrap_expect_used::Variant::Unwrap,
                     );
                 },
+                (sym::or_default | sym::unwrap_or_default, []) => {
+                    concealed_obvious_default::check(cx, recv, path.ident.name, call_span);
+                },
                 (sym::unwrap_err, []) => {
                     unwrap_expect_used::check(
                         cx,

tests/ui/concealed_obvious_default.fixed

@@ -0,0 +1,86 @@
+//@aux-build:proc_macros.rs
+#![warn(clippy::concealed_obvious_default)]
+#![allow(dead_code)]
+#![allow(clippy::too_many_arguments)]
+
+extern crate proc_macros;
+
+use std::collections::{btree_map, hash_map};
+
+fn test<'e, 'f, 'g, 'h>(
+    a: Option<bool>,
+    b: Option<Option<bool>>,
+    c: Result<&str, ()>,
+    d: Result<usize, ()>,
+    e: btree_map::Entry<'e, (), f32>,
+    e2: btree_map::Entry<'e, (), f32>,
+    f: btree_map::Entry<'f, (), char>,
+    f2: btree_map::Entry<'f, (), char>,
+    g: hash_map::Entry<'g, (), ()>,
+    g2: hash_map::Entry<'g, (), ()>,
+    h: hash_map::Entry<'h, (), u8>,
+    h2: hash_map::Entry<'h, (), u8>,
+) {
+    _ = a.unwrap_or(false);
+    //~^ concealed_obvious_default
+    _ = a.unwrap_or(false);
+
+    _ = b.unwrap_or(None);
+    //~^ concealed_obvious_default
+    _ = b.unwrap_or(None);
+
+    _ = c.unwrap_or("");
+    //~^ concealed_obvious_default
+    _ = c.unwrap_or("");
+
+    _ = d.unwrap_or(0);
+    //~^ concealed_obvious_default
+    _ = d.unwrap_or(0);
+
+    _ = e.or_insert(0.0);
+    //~^ concealed_obvious_default
+    _ = e2.or_insert(0.0);
+
+    _ = f.or_insert('\0');
+    //~^ concealed_obvious_default
+    _ = f2.or_insert('\0');
+
+    _ = g.or_insert(());
+    //~^ concealed_obvious_default
+    _ = g2.or_insert(());
+
+    _ = h.or_insert(0);
+    //~^ concealed_obvious_default
+    _ = h2.or_insert(0);
+
+    //
+    // If the receiver comes from macro expansion,
+    // that's fine and we lint that.
+    //
+
+    _ = proc_macros::external! {
+        Some(false)
+    }
+    .unwrap_or(false);
+    //~^ concealed_obvious_default
+
+    _ = proc_macros::external! {
+        Some(false)
+    }
+    .unwrap_or(false);
+
+    //
+    // However, if the method itself comes from
+    // macro expansion, it should not be linted.
+    //
+
+    _ = proc_macros::external! {
+        Some(false).unwrap_or_default()
+    };
+
+    _ = proc_macros::external! {
+        Some(false).unwrap_or(false)
+    };
+}
+
+fn main() {}