Skip to content

Commit 8ad3b0f

Browse files
samueltardieusamueltardieu
committed
Properly resolve associated constants of primitive types
Associated constants of primitive types are resolved as `Res::Err` in the HIR. To force a resolution, the primitive type must be recognized as certain. Since they are not ADT, they don't have a `DefId`. By allowing the `Certainty` type to embed a `TypeKind` which is either a `PrimTy` or a `DefId`, we allow primitives types to be resolved, and the associated constants as well. Also, in method calls, if the receiver is not certain, consider that the overall call is not either.
1 parent 2a8d79e commit 8ad3b0f

File tree

9 files changed

+158
-49
lines changed

9 files changed

+158
-49
lines changed

Diff for: ‎clippy_utils/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#![feature(assert_matches)]
1111
#![feature(unwrap_infallible)]
1212
#![feature(array_windows)]
13+
#![feature(try_trait_v2)]
1314
#![recursion_limit = "512"]
1415
#![allow(
1516
clippy::missing_errors_doc,

Diff for: ‎clippy_utils/src/ty/type_certainty/certainty.rs

+51-11
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,22 @@
11
use rustc_hir::def_id::DefId;
22
use std::fmt::Debug;
3+
use std::ops::{ControlFlow, FromResidual, Try};
4+
5+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
6+
pub enum TypeKind {
7+
PrimTy,
8+
AdtDef(DefId),
9+
}
310

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

916
/// 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>),
17+
/// specific primitive type or `DefId` is known. Such arguments are needed to handle path
18+
/// segments whose `res` is `Res::Err`.
19+
Certain(Option<TypeKind>),
1320

1421
/// The heuristic believes that more than one `DefId` applies to a type---this is a bug.
1522
Contradiction,
@@ -23,7 +30,7 @@ pub trait TryJoin: Sized {
2330
fn try_join(self, other: Self) -> Option<Self>;
2431
}
2532

26-
impl Meet for Option<DefId> {
33+
impl Meet for Option<TypeKind> {
2734
fn meet(self, other: Self) -> Self {
2835
match (self, other) {
2936
(None, _) | (_, None) => None,
@@ -32,11 +39,11 @@ impl Meet for Option<DefId> {
3239
}
3340
}
3441

35-
impl TryJoin for Option<DefId> {
42+
impl TryJoin for Option<TypeKind> {
3643
fn try_join(self, other: Self) -> Option<Self> {
3744
match (self, other) {
3845
(Some(lhs), Some(rhs)) => (lhs == rhs).then_some(Some(lhs)),
39-
(Some(def_id), _) | (_, Some(def_id)) => Some(Some(def_id)),
46+
(Some(ty_kind), _) | (_, Some(ty_kind)) => Some(Some(ty_kind)),
4047
(None, None) => Some(None),
4148
}
4249
}
@@ -79,29 +86,37 @@ impl Certainty {
7986
/// Join two `Certainty`s after clearing their `DefId`s. This method should be used when `self`
8087
/// or `other` do not necessarily refer to types, e.g., when they are aggregations of other
8188
/// `Certainty`s.
82-
pub fn join_clearing_def_ids(self, other: Self) -> Self {
83-
self.clear_def_id().join(other.clear_def_id())
89+
pub fn join_clearing_types(self, other: Self) -> Self {
90+
self.clear_type().join(other.clear_type())
8491
}
8592

86-
pub fn clear_def_id(self) -> Certainty {
93+
pub fn clear_type(self) -> Certainty {
8794
if matches!(self, Certainty::Certain(_)) {
8895
Certainty::Certain(None)
8996
} else {
9097
self
9198
}
9299
}
93100

101+
pub fn with_prim_ty(self) -> Certainty {
102+
if matches!(self, Certainty::Certain(_)) {
103+
Certainty::Certain(Some(TypeKind::PrimTy))
104+
} else {
105+
self
106+
}
107+
}
108+
94109
pub fn with_def_id(self, def_id: DefId) -> Certainty {
95110
if matches!(self, Certainty::Certain(_)) {
96-
Certainty::Certain(Some(def_id))
111+
Certainty::Certain(Some(TypeKind::AdtDef(def_id)))
97112
} else {
98113
self
99114
}
100115
}
101116

102117
pub fn to_def_id(self) -> Option<DefId> {
103118
match self {
104-
Certainty::Certain(inner) => inner,
119+
Certainty::Certain(Some(TypeKind::AdtDef(def_id))) => Some(def_id),
105120
_ => None,
106121
}
107122
}
@@ -120,3 +135,28 @@ pub fn meet(iter: impl Iterator<Item = Certainty>) -> Certainty {
120135
pub fn join(iter: impl Iterator<Item = Certainty>) -> Certainty {
121136
iter.fold(Certainty::Uncertain, Certainty::join)
122137
}
138+
139+
pub struct NoCertainty(Certainty);
140+
141+
impl FromResidual<NoCertainty> for Certainty {
142+
fn from_residual(residual: NoCertainty) -> Self {
143+
residual.0
144+
}
145+
}
146+
147+
impl Try for Certainty {
148+
type Output = Certainty;
149+
150+
type Residual = NoCertainty;
151+
152+
fn from_output(output: Self::Output) -> Self {
153+
output
154+
}
155+
156+
fn branch(self) -> ControlFlow<Self::Residual, Self::Output> {
157+
match self {
158+
Certainty::Certain(_) => ControlFlow::Continue(self),
159+
_ => ControlFlow::Break(NoCertainty(self)),
160+
}
161+
}
162+
}

Diff for: ‎clippy_utils/src/ty/type_certainty/mod.rs

+21-17
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use rustc_middle::ty::{self, AdtDef, GenericArgKind, Ty};
2222
use rustc_span::{Span, Symbol};
2323

2424
mod certainty;
25-
use certainty::{Certainty, Meet, join, meet};
25+
use certainty::{Certainty, Meet, TypeKind, join, meet};
2626

2727
pub fn expr_type_is_certain(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
2828
expr_type_certainty(cx, expr, false).is_certain()
@@ -46,11 +46,12 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>, in_arg: bool) -> C
4646
} else {
4747
Certainty::Uncertain
4848
};
49-
lhs.join_clearing_def_ids(rhs)
49+
lhs.join_clearing_types(rhs)
5050
},
5151

5252
ExprKind::MethodCall(method, receiver, args, _) => {
53-
let mut receiver_type_certainty = expr_type_certainty(cx, receiver, false);
53+
let mut receiver_type_certainty = expr_type_certainty(cx, receiver, false)?;
54+
5455
// Even if `receiver_type_certainty` is `Certain(Some(..))`, the `Self` type in the method
5556
// identified by `type_dependent_def_id(..)` can differ. This can happen as a result of a `deref`,
5657
// for example. So update the `DefId` in `receiver_type_certainty` (if any).
@@ -66,7 +67,7 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>, in_arg: bool) -> C
6667
.chain(args.iter().map(|arg| expr_type_certainty(cx, arg, true))),
6768
)
6869
} else {
69-
Certainty::Uncertain
70+
return Certainty::Uncertain;
7071
};
7172
lhs.join(rhs)
7273
},
@@ -117,12 +118,13 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>, in_arg: bool) -> C
117118
_ => Certainty::Uncertain,
118119
};
119120

120-
let expr_ty = cx.typeck_results().expr_ty(expr);
121-
if let Some(def_id) = adt_def_id(expr_ty) {
122-
certainty.with_def_id(def_id)
123-
} else {
124-
certainty.clear_def_id()
125-
}
121+
let result = match cx.typeck_results().expr_ty(expr).kind() {
122+
ty::Adt(adt_def, _) => certainty.with_def_id(adt_def.did()),
123+
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) => certainty.with_prim_ty(),
124+
_ => certainty.clear_type(),
125+
};
126+
127+
result
126128
}
127129

128130
struct CertaintyVisitor<'cx, 'tcx> {
@@ -209,7 +211,11 @@ fn qpath_certainty(cx: &LateContext<'_>, qpath: &QPath<'_>, resolves_to_type: bo
209211
.map_or(Certainty::Uncertain, |def_id| {
210212
let generics = cx.tcx.generics_of(def_id);
211213
if generics.is_empty() {
212-
Certainty::Certain(if resolves_to_type { Some(def_id) } else { None })
214+
Certainty::Certain(if resolves_to_type {
215+
Some(TypeKind::AdtDef(def_id))
216+
} else {
217+
None
218+
})
213219
} else {
214220
Certainty::Uncertain
215221
}
@@ -249,7 +255,7 @@ fn path_segment_certainty(
249255
.args
250256
.map_or(Certainty::Uncertain, |args| generic_args_certainty(cx, args));
251257
// See the comment preceding `qpath_certainty`. `def_id` could refer to a type or a value.
252-
let certainty = lhs.join_clearing_def_ids(rhs);
258+
let certainty = lhs.join_clearing_types(rhs);
253259
if resolves_to_type {
254260
if let DefKind::TyAlias = cx.tcx.def_kind(def_id) {
255261
adt_def_id(cx.tcx.type_of(def_id).instantiate_identity())
@@ -265,9 +271,7 @@ fn path_segment_certainty(
265271
}
266272
},
267273

268-
Res::PrimTy(_) | Res::SelfTyParam { .. } | Res::SelfTyAlias { .. } | Res::SelfCtor(_) => {
269-
Certainty::Certain(None)
270-
},
274+
Res::PrimTy(_) => Certainty::Certain(Some(TypeKind::PrimTy)),
271275

272276
// `get_parent` because `hir_id` refers to a `Pat`, and we're interested in the node containing the `Pat`.
273277
Res::Local(hir_id) => match cx.tcx.parent_hir_node(hir_id) {
@@ -284,13 +288,13 @@ fn path_segment_certainty(
284288
if resolves_to_type {
285289
certainty
286290
} else {
287-
certainty.clear_def_id()
291+
certainty.clear_type()
288292
}
289293
},
290294
_ => Certainty::Uncertain,
291295
},
292296

293-
_ => Certainty::Uncertain,
297+
_ => Certainty::Certain(None),
294298
};
295299
debug_assert!(resolves_to_type || certainty.to_def_id().is_none());
296300
certainty

Diff for: ‎tests/ui/or_fun_call.fixed

+8-4
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,8 @@ mod issue8239 {
212212
acc.push_str(&f);
213213
acc
214214
})
215-
.unwrap_or(String::new());
215+
.unwrap_or_default();
216+
//~^ unwrap_or_default
216217
}
217218

218219
fn more_to_max_suggestion_highest_lines_1() {
@@ -225,7 +226,8 @@ mod issue8239 {
225226
acc.push_str(&f);
226227
acc
227228
})
228-
.unwrap_or(String::new());
229+
.unwrap_or_default();
230+
//~^ unwrap_or_default
229231
}
230232

231233
fn equal_to_max_suggestion_highest_lines() {
@@ -237,7 +239,8 @@ mod issue8239 {
237239
acc.push_str(&f);
238240
acc
239241
})
240-
.unwrap_or(String::new());
242+
.unwrap_or_default();
243+
//~^ unwrap_or_default
241244
}
242245

243246
fn less_than_max_suggestion_highest_lines() {
@@ -248,7 +251,8 @@ mod issue8239 {
248251
acc.push_str(&f);
249252
acc
250253
})
251-
.unwrap_or(String::new());
254+
.unwrap_or_default();
255+
//~^ unwrap_or_default
252256
}
253257
}
254258

Diff for: ‎tests/ui/or_fun_call.rs

+4
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ mod issue8239 {
213213
acc
214214
})
215215
.unwrap_or(String::new());
216+
//~^ unwrap_or_default
216217
}
217218

218219
fn more_to_max_suggestion_highest_lines_1() {
@@ -226,6 +227,7 @@ mod issue8239 {
226227
acc
227228
})
228229
.unwrap_or(String::new());
230+
//~^ unwrap_or_default
229231
}
230232

231233
fn equal_to_max_suggestion_highest_lines() {
@@ -238,6 +240,7 @@ mod issue8239 {
238240
acc
239241
})
240242
.unwrap_or(String::new());
243+
//~^ unwrap_or_default
241244
}
242245

243246
fn less_than_max_suggestion_highest_lines() {
@@ -249,6 +252,7 @@ mod issue8239 {
249252
acc
250253
})
251254
.unwrap_or(String::new());
255+
//~^ unwrap_or_default
252256
}
253257
}
254258

0 commit comments

Comments
 (0)