Skip to content

Fix unwrap_or_else_default false positive #11135

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
Jul 19, 2023
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
6 changes: 5 additions & 1 deletion clippy_lints/src/methods/unwrap_or_else_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use super::UNWRAP_OR_ELSE_DEFAULT;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::is_default_equivalent_call;
use clippy_utils::source::snippet_with_applicability;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::ty::{expr_type_is_certain, is_type_diagnostic_item};
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir as hir;
Expand All @@ -17,6 +17,10 @@ pub(super) fn check<'tcx>(
recv: &'tcx hir::Expr<'_>,
u_arg: &'tcx hir::Expr<'_>,
) {
if !expr_type_is_certain(cx, recv) {
return;
}

// something.unwrap_or_else(Default::default)
// ^^^^^^^^^- recv ^^^^^^^^^^^^^^^^- u_arg
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr
Expand Down
3 changes: 3 additions & 0 deletions clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ use std::iter;

use crate::{match_def_path, path_res, paths};

mod type_certainty;
pub use type_certainty::expr_type_is_certain;

/// Checks if the given type implements copy.
pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
ty.is_copy_modulo_regions(cx.tcx, cx.param_env)
Expand Down
122 changes: 122 additions & 0 deletions clippy_utils/src/ty/type_certainty/certainty.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
use rustc_hir::def_id::DefId;
use std::fmt::Debug;

#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum Certainty {
/// Determining the type requires contextual information.
Uncertain,

/// The type can be determined purely from subexpressions. If the argument is `Some(..)`, the
/// specific `DefId` is known. Such arguments are needed to handle path segments whose `res` is
/// `Res::Err`.
Certain(Option<DefId>),

/// The heuristic believes that more than one `DefId` applies to a type---this is a bug.
Contradiction,
}

pub trait Meet {
fn meet(self, other: Self) -> Self;
}

pub trait TryJoin: Sized {
fn try_join(self, other: Self) -> Option<Self>;
}

impl Meet for Option<DefId> {
fn meet(self, other: Self) -> Self {
match (self, other) {
(None, _) | (_, None) => None,
(Some(lhs), Some(rhs)) => (lhs == rhs).then_some(lhs),
}
}
}

impl TryJoin for Option<DefId> {
fn try_join(self, other: Self) -> Option<Self> {
match (self, other) {
(Some(lhs), Some(rhs)) => (lhs == rhs).then_some(Some(lhs)),
(Some(def_id), _) | (_, Some(def_id)) => Some(Some(def_id)),
(None, None) => Some(None),
}
}
}

impl Meet for Certainty {
fn meet(self, other: Self) -> Self {
match (self, other) {
(Certainty::Uncertain, _) | (_, Certainty::Uncertain) => Certainty::Uncertain,
(Certainty::Certain(lhs), Certainty::Certain(rhs)) => Certainty::Certain(lhs.meet(rhs)),
(Certainty::Certain(inner), _) | (_, Certainty::Certain(inner)) => Certainty::Certain(inner),
(Certainty::Contradiction, Certainty::Contradiction) => Certainty::Contradiction,
}
}
}

impl Certainty {
/// Join two `Certainty`s preserving their `DefId`s (if any). Generally speaking, this method
/// should be used only when `self` and `other` refer directly to types. Otherwise,
/// `join_clearing_def_ids` should be used.
pub fn join(self, other: Self) -> Self {
match (self, other) {
(Certainty::Contradiction, _) | (_, Certainty::Contradiction) => Certainty::Contradiction,

(Certainty::Certain(lhs), Certainty::Certain(rhs)) => {
if let Some(inner) = lhs.try_join(rhs) {
Certainty::Certain(inner)
} else {
debug_assert!(false, "Contradiction with {lhs:?} and {rhs:?}");
Certainty::Contradiction
}
},

(Certainty::Certain(inner), _) | (_, Certainty::Certain(inner)) => Certainty::Certain(inner),

(Certainty::Uncertain, Certainty::Uncertain) => Certainty::Uncertain,
}
}

/// Join two `Certainty`s after clearing their `DefId`s. This method should be used when `self`
/// or `other` do not necessarily refer to types, e.g., when they are aggregations of other
/// `Certainty`s.
pub fn join_clearing_def_ids(self, other: Self) -> Self {
self.clear_def_id().join(other.clear_def_id())
}

pub fn clear_def_id(self) -> Certainty {
if matches!(self, Certainty::Certain(_)) {
Certainty::Certain(None)
} else {
self
}
}

pub fn with_def_id(self, def_id: DefId) -> Certainty {
if matches!(self, Certainty::Certain(_)) {
Certainty::Certain(Some(def_id))
} else {
self
}
}

pub fn to_def_id(self) -> Option<DefId> {
match self {
Certainty::Certain(inner) => inner,
_ => None,
}
}

pub fn is_certain(self) -> bool {
matches!(self, Self::Certain(_))
}
}

/// Think: `iter.all(/* is certain */)`
pub fn meet(iter: impl Iterator<Item = Certainty>) -> Certainty {
iter.fold(Certainty::Certain(None), Certainty::meet)
}

/// Think: `iter.any(/* is certain */)`
pub fn join(iter: impl Iterator<Item = Certainty>) -> Certainty {
iter.fold(Certainty::Uncertain, Certainty::join)
}
Loading