Skip to content

Commit 7f627a4

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.
1 parent 2a8d79e commit 7f627a4

9 files changed

+137
-76
lines changed

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

+25-11
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,21 @@
11
use rustc_hir::def_id::DefId;
22
use std::fmt::Debug;
33

4+
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
5+
pub enum TypeKind {
6+
PrimTy,
7+
AdtDef(DefId),
8+
}
9+
410
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
511
pub enum Certainty {
612
/// Determining the type requires contextual information.
713
Uncertain,
814

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

1420
/// The heuristic believes that more than one `DefId` applies to a type---this is a bug.
1521
Contradiction,
@@ -23,7 +29,7 @@ pub trait TryJoin: Sized {
2329
fn try_join(self, other: Self) -> Option<Self>;
2430
}
2531

26-
impl Meet for Option<DefId> {
32+
impl Meet for Option<TypeKind> {
2733
fn meet(self, other: Self) -> Self {
2834
match (self, other) {
2935
(None, _) | (_, None) => None,
@@ -32,11 +38,11 @@ impl Meet for Option<DefId> {
3238
}
3339
}
3440

35-
impl TryJoin for Option<DefId> {
41+
impl TryJoin for Option<TypeKind> {
3642
fn try_join(self, other: Self) -> Option<Self> {
3743
match (self, other) {
3844
(Some(lhs), Some(rhs)) => (lhs == rhs).then_some(Some(lhs)),
39-
(Some(def_id), _) | (_, Some(def_id)) => Some(Some(def_id)),
45+
(Some(ty_kind), _) | (_, Some(ty_kind)) => Some(Some(ty_kind)),
4046
(None, None) => Some(None),
4147
}
4248
}
@@ -79,29 +85,37 @@ impl Certainty {
7985
/// Join two `Certainty`s after clearing their `DefId`s. This method should be used when `self`
8086
/// or `other` do not necessarily refer to types, e.g., when they are aggregations of other
8187
/// `Certainty`s.
82-
pub fn join_clearing_def_ids(self, other: Self) -> Self {
83-
self.clear_def_id().join(other.clear_def_id())
88+
pub fn join_clearing_types(self, other: Self) -> Self {
89+
self.clear_type().join(other.clear_type())
8490
}
8591

86-
pub fn clear_def_id(self) -> Certainty {
92+
pub fn clear_type(self) -> Certainty {
8793
if matches!(self, Certainty::Certain(_)) {
8894
Certainty::Certain(None)
8995
} else {
9096
self
9197
}
9298
}
9399

100+
pub fn with_prim_ty(self) -> Certainty {
101+
if matches!(self, Certainty::Certain(_)) {
102+
Certainty::Certain(Some(TypeKind::PrimTy))
103+
} else {
104+
self
105+
}
106+
}
107+
94108
pub fn with_def_id(self, def_id: DefId) -> Certainty {
95109
if matches!(self, Certainty::Certain(_)) {
96-
Certainty::Certain(Some(def_id))
110+
Certainty::Certain(Some(TypeKind::AdtDef(def_id)))
97111
} else {
98112
self
99113
}
100114
}
101115

102116
pub fn to_def_id(self) -> Option<DefId> {
103117
match self {
104-
Certainty::Certain(inner) => inner,
118+
Certainty::Certain(Some(TypeKind::AdtDef(def_id))) => Some(def_id),
105119
_ => None,
106120
}
107121
}

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

+17-14
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,7 +46,7 @@ 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, _) => {
@@ -117,11 +117,10 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>, in_arg: bool) -> C
117117
_ => Certainty::Uncertain,
118118
};
119119

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()
120+
match cx.typeck_results().expr_ty(expr).kind() {
121+
ty::Adt(adt_def, _) => certainty.with_def_id(adt_def.did()),
122+
ty::Bool | ty::Char | ty::Int(_) | ty::Uint(_) | ty::Float(_) => certainty.with_prim_ty(),
123+
_ => certainty.clear_type(),
125124
}
126125
}
127126

@@ -209,7 +208,11 @@ fn qpath_certainty(cx: &LateContext<'_>, qpath: &QPath<'_>, resolves_to_type: bo
209208
.map_or(Certainty::Uncertain, |def_id| {
210209
let generics = cx.tcx.generics_of(def_id);
211210
if generics.is_empty() {
212-
Certainty::Certain(if resolves_to_type { Some(def_id) } else { None })
211+
Certainty::Certain(if resolves_to_type {
212+
Some(TypeKind::AdtDef(def_id))
213+
} else {
214+
None
215+
})
213216
} else {
214217
Certainty::Uncertain
215218
}
@@ -249,7 +252,7 @@ fn path_segment_certainty(
249252
.args
250253
.map_or(Certainty::Uncertain, |args| generic_args_certainty(cx, args));
251254
// 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);
255+
let certainty = lhs.join_clearing_types(rhs);
253256
if resolves_to_type {
254257
if let DefKind::TyAlias = cx.tcx.def_kind(def_id) {
255258
adt_def_id(cx.tcx.type_of(def_id).instantiate_identity())
@@ -265,9 +268,9 @@ fn path_segment_certainty(
265268
}
266269
},
267270

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

272275
// `get_parent` because `hir_id` refers to a `Pat`, and we're interested in the node containing the `Pat`.
273276
Res::Local(hir_id) => match cx.tcx.parent_hir_node(hir_id) {
@@ -284,13 +287,13 @@ fn path_segment_certainty(
284287
if resolves_to_type {
285288
certainty
286289
} else {
287-
certainty.clear_def_id()
290+
certainty.clear_type()
288291
}
289292
},
290293
_ => Certainty::Uncertain,
291294
},
292295

293-
_ => Certainty::Uncertain,
296+
_ => Certainty::Certain(None),
294297
};
295298
debug_assert!(resolves_to_type || certainty.to_def_id().is_none());
296299
certainty

Diff for: ‎tests/ui/manual_unwrap_or_default_unfixable.stderr

+10-30
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,15 @@
1-
error: if let can be simplified with `.unwrap_or_default()`
2-
--> tests/ui/manual_unwrap_or_default_unfixable.rs:5:13
1+
error[E0284]: type annotations needed
2+
--> tests/ui/manual_unwrap_or_default_unfixable.rs:5:9
33
|
4-
LL | let _ = if let Some(x) = "1".parse().ok() {
5-
| _____________^
6-
LL | |
7-
LL | | x
8-
LL | | } else {
9-
LL | | i32::default()
10-
LL | | };
11-
| |_____^ help: ascribe the type i32 and replace your expression with: `"1".parse().ok().unwrap_or_default()`
4+
LL | let _ = "1".parse().ok().unwrap_or_default();
5+
| ^ ----- type must be known at this point
126
|
13-
= note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings`
14-
= help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]`
15-
16-
error: if let can be simplified with `.unwrap_or_default()`
17-
--> tests/ui/manual_unwrap_or_default_unfixable.rs:11:13
18-
|
19-
LL | let _ = if let Some(x) = None { x } else { i32::default() };
20-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `None::<i32>.unwrap_or_default()`
21-
22-
error: if let can be simplified with `.unwrap_or_default()`
23-
--> tests/ui/manual_unwrap_or_default_unfixable.rs:15:13
24-
|
25-
LL | let _ = if let Some(x) = a { x } else { i32::default() };
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.unwrap_or_default()`
27-
28-
error: if let can be simplified with `.unwrap_or_default()`
29-
--> tests/ui/manual_unwrap_or_default_unfixable.rs:17:13
7+
= note: cannot satisfy `<_ as std::str::FromStr>::Err == _`
8+
help: consider giving this pattern a type
309
|
31-
LL | let _ = if let Some(x) = Some(99) { x } else { i32::default() };
32-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `Some(99).unwrap_or_default()`
10+
LL | let _: /* Type */ = "1".parse().ok().unwrap_or_default();
11+
| ++++++++++++
3312

34-
error: aborting due to 4 previous errors
13+
error: aborting due to 1 previous error
3514

15+
For more information about this error, try `rustc --explain E0284`.

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)