Skip to content

Commit

Permalink
Fix unwrap_or_else_default false positive
Browse files Browse the repository at this point in the history
  • Loading branch information
smoelius committed Jul 19, 2023
1 parent 1d33469 commit f583fd1
Show file tree
Hide file tree
Showing 7 changed files with 568 additions and 2 deletions.
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

0 comments on commit f583fd1

Please sign in to comment.