Skip to content

Commit 464ebb0

Browse files
committed
Fix unwrap_or_else_default false positive
1 parent be15e60 commit 464ebb0

File tree

7 files changed

+578
-5
lines changed

7 files changed

+578
-5
lines changed

clippy_lints/src/methods/unwrap_or_else_default.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
//! Lint for `some_result_or_option.unwrap_or_else(Default::default)`
22
33
use super::UNWRAP_OR_ELSE_DEFAULT;
4-
use clippy_utils::{
5-
diagnostics::span_lint_and_sugg, is_default_equivalent_call, source::snippet_with_applicability,
6-
ty::is_type_diagnostic_item,
7-
};
4+
use clippy_utils::ty::{expr_type_is_certain, is_type_diagnostic_item};
5+
use clippy_utils::{diagnostics::span_lint_and_sugg, is_default_equivalent_call, source::snippet_with_applicability};
86
use rustc_ast::ast::LitKind;
97
use rustc_errors::Applicability;
108
use rustc_hir as hir;
@@ -17,6 +15,10 @@ pub(super) fn check<'tcx>(
1715
recv: &'tcx hir::Expr<'_>,
1816
u_arg: &'tcx hir::Expr<'_>,
1917
) {
18+
if !expr_type_is_certain(cx, recv) {
19+
return;
20+
}
21+
2022
// something.unwrap_or_else(Default::default)
2123
// ^^^^^^^^^- recv ^^^^^^^^^^^^^^^^- u_arg
2224
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- expr

clippy_utils/src/ty.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ use std::iter;
3030

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

33+
mod type_certainty;
34+
pub use type_certainty::expr_type_is_certain;
35+
3336
/// Checks if the given type implements copy.
3437
pub fn is_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
3538
ty.is_copy_modulo_regions(cx.tcx, cx.param_env)
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
use rustc_hir::def_id::DefId;
2+
use std::fmt::Debug;
3+
4+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
5+
pub enum Certainty {
6+
/// Determining the type requires contextual information.
7+
Uncertain,
8+
9+
/// The type can be determined purely from subexpressions. If the argument is `Some(..)`, the
10+
/// specific `DefId` is known. Such arguments are needed to handle path segments whose `res` is
11+
/// `Res::Err`.
12+
Certain(Option<DefId>),
13+
14+
/// The heuristic believes that more than one `DefId` applies to a type---this is a bug.
15+
Contradiction,
16+
}
17+
18+
pub trait Meet {
19+
fn meet(self, other: Self) -> Self;
20+
}
21+
22+
pub trait TryJoin: Sized {
23+
fn try_join(self, other: Self) -> Option<Self>;
24+
}
25+
26+
impl Meet for Option<DefId> {
27+
fn meet(self, other: Self) -> Self {
28+
match (self, other) {
29+
(None, _) | (_, None) => None,
30+
(Some(lhs), Some(rhs)) => {
31+
if lhs == rhs {
32+
Some(lhs)
33+
} else {
34+
None
35+
}
36+
},
37+
}
38+
}
39+
}
40+
41+
impl TryJoin for Option<DefId> {
42+
fn try_join(self, other: Self) -> Option<Self> {
43+
match (self, other) {
44+
(Some(lhs), Some(rhs)) => {
45+
if lhs == rhs {
46+
Some(Some(lhs))
47+
} else {
48+
None
49+
}
50+
},
51+
(Some(def_id), _) | (_, Some(def_id)) => Some(Some(def_id)),
52+
(None, None) => Some(None),
53+
}
54+
}
55+
}
56+
57+
impl Meet for Certainty {
58+
fn meet(self, other: Self) -> Self {
59+
match (self, other) {
60+
(Certainty::Uncertain, _) | (_, Certainty::Uncertain) => Certainty::Uncertain,
61+
(Certainty::Certain(lhs), Certainty::Certain(rhs)) => Certainty::Certain(lhs.meet(rhs)),
62+
(Certainty::Certain(inner), _) | (_, Certainty::Certain(inner)) => Certainty::Certain(inner),
63+
(Certainty::Contradiction, Certainty::Contradiction) => Certainty::Contradiction,
64+
}
65+
}
66+
}
67+
68+
impl Certainty {
69+
/// Join two `Certainty`s preserving their `DefId`s (if any). Generally speaking, this method
70+
/// should be used only when `self` and `other` refer directly to types. Otherwise,
71+
/// `join_clearing_def_ids` should be used.
72+
pub fn join(self, other: Self) -> Self {
73+
match (self, other) {
74+
(Certainty::Contradiction, _) | (_, Certainty::Contradiction) => Certainty::Contradiction,
75+
76+
(Certainty::Certain(lhs), Certainty::Certain(rhs)) => {
77+
if let Some(inner) = lhs.try_join(rhs) {
78+
Certainty::Certain(inner)
79+
} else {
80+
debug_assert!(false, "Contradiction with {lhs:?} and {rhs:?}");
81+
Certainty::Contradiction
82+
}
83+
},
84+
85+
(Certainty::Certain(inner), _) | (_, Certainty::Certain(inner)) => Certainty::Certain(inner),
86+
87+
(Certainty::Uncertain, Certainty::Uncertain) => Certainty::Uncertain,
88+
}
89+
}
90+
91+
/// Join two `Certainty`s after clearing their `DefId`s. This method should be used when `self`
92+
/// or `other` do not necessarily refer to types, e.g., when they are aggregations of other
93+
/// `Certainty`s.
94+
pub fn join_clearing_def_ids(self, other: Self) -> Self {
95+
self.clear_def_id().join(other.clear_def_id())
96+
}
97+
98+
pub fn clear_def_id(self) -> Certainty {
99+
match self {
100+
Certainty::Uncertain => Certainty::Uncertain,
101+
Certainty::Certain(_) => Certainty::Certain(None),
102+
Certainty::Contradiction => Certainty::Contradiction,
103+
}
104+
}
105+
106+
pub fn with_def_id(self, def_id: DefId) -> Certainty {
107+
match self {
108+
Certainty::Uncertain => Certainty::Uncertain,
109+
Certainty::Certain(_) => Certainty::Certain(Some(def_id)),
110+
Certainty::Contradiction => Certainty::Contradiction,
111+
}
112+
}
113+
114+
pub fn to_def_id(self) -> Option<DefId> {
115+
match self {
116+
Certainty::Certain(inner) => inner,
117+
_ => None,
118+
}
119+
}
120+
121+
pub fn is_certain(self) -> bool {
122+
matches!(self, Self::Certain(_))
123+
}
124+
}
125+
126+
/// Think: `iter.all(/* is certain */)`
127+
pub fn meet(iter: impl Iterator<Item = Certainty>) -> Certainty {
128+
iter.fold(Certainty::Certain(None), Certainty::meet)
129+
}
130+
131+
/// Think: `iter.any(/* is certain */)`
132+
pub fn join(iter: impl Iterator<Item = Certainty>) -> Certainty {
133+
iter.fold(Certainty::Uncertain, Certainty::join)
134+
}

0 commit comments

Comments
 (0)