Skip to content

Commit 95968ac

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 95968ac

9 files changed

+128
-47
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

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ LL | | x
88
LL | | } else {
99
LL | | i32::default()
1010
LL | | };
11-
| |_____^ help: ascribe the type i32 and replace your expression with: `"1".parse().ok().unwrap_or_default()`
11+
| |_____^ help: replace it with: `"1".parse().ok().unwrap_or_default()`
1212
|
1313
= note: `-D clippy::manual-unwrap-or-default` implied by `-D warnings`
1414
= help: to override `-D warnings` add `#[allow(clippy::manual_unwrap_or_default)]`

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

Diff for: ‎tests/ui/or_fun_call.stderr

+40-16
Original file line numberDiff line numberDiff line change
@@ -142,74 +142,98 @@ error: function call inside of `unwrap_or`
142142
LL | None.unwrap_or( unsafe { ptr_to_ref(s) } );
143143
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| unsafe { ptr_to_ref(s) })`
144144

145+
error: use of `unwrap_or` to construct default value
146+
--> tests/ui/or_fun_call.rs:215:14
147+
|
148+
LL | .unwrap_or(String::new());
149+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
150+
151+
error: use of `unwrap_or` to construct default value
152+
--> tests/ui/or_fun_call.rs:229:14
153+
|
154+
LL | .unwrap_or(String::new());
155+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
156+
157+
error: use of `unwrap_or` to construct default value
158+
--> tests/ui/or_fun_call.rs:242:14
159+
|
160+
LL | .unwrap_or(String::new());
161+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
162+
163+
error: use of `unwrap_or` to construct default value
164+
--> tests/ui/or_fun_call.rs:254:10
165+
|
166+
LL | .unwrap_or(String::new());
167+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
168+
145169
error: function call inside of `map_or`
146-
--> tests/ui/or_fun_call.rs:276:25
170+
--> tests/ui/or_fun_call.rs:280:25
147171
|
148172
LL | let _ = Some(4).map_or(g(), |v| v);
149173
| ^^^^^^^^^^^^^^^^^^ help: try: `map_or_else(g, |v| v)`
150174

151175
error: function call inside of `map_or`
152-
--> tests/ui/or_fun_call.rs:278:25
176+
--> tests/ui/or_fun_call.rs:282:25
153177
|
154178
LL | let _ = Some(4).map_or(g(), f);
155179
| ^^^^^^^^^^^^^^ help: try: `map_or_else(g, f)`
156180

157181
error: use of `unwrap_or_else` to construct default value
158-
--> tests/ui/or_fun_call.rs:310:18
182+
--> tests/ui/or_fun_call.rs:314:18
159183
|
160184
LL | with_new.unwrap_or_else(Vec::new);
161185
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
162186

163187
error: use of `unwrap_or_else` to construct default value
164-
--> tests/ui/or_fun_call.rs:314:28
188+
--> tests/ui/or_fun_call.rs:318:28
165189
|
166190
LL | with_default_trait.unwrap_or_else(Default::default);
167191
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
168192

169193
error: use of `unwrap_or_else` to construct default value
170-
--> tests/ui/or_fun_call.rs:318:27
194+
--> tests/ui/or_fun_call.rs:322:27
171195
|
172196
LL | with_default_type.unwrap_or_else(u64::default);
173197
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
174198

175199
error: use of `unwrap_or_else` to construct default value
176-
--> tests/ui/or_fun_call.rs:322:22
200+
--> tests/ui/or_fun_call.rs:326:22
177201
|
178202
LL | real_default.unwrap_or_else(<FakeDefault as Default>::default);
179203
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
180204

181205
error: use of `or_insert_with` to construct default value
182-
--> tests/ui/or_fun_call.rs:326:23
206+
--> tests/ui/or_fun_call.rs:330:23
183207
|
184208
LL | map.entry(42).or_insert_with(String::new);
185209
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()`
186210

187211
error: use of `or_insert_with` to construct default value
188-
--> tests/ui/or_fun_call.rs:330:25
212+
--> tests/ui/or_fun_call.rs:334:25
189213
|
190214
LL | btree.entry(42).or_insert_with(String::new);
191215
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()`
192216

193217
error: use of `unwrap_or_else` to construct default value
194-
--> tests/ui/or_fun_call.rs:334:25
218+
--> tests/ui/or_fun_call.rs:338:25
195219
|
196220
LL | let _ = stringy.unwrap_or_else(String::new);
197221
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
198222

199223
error: function call inside of `unwrap_or`
200-
--> tests/ui/or_fun_call.rs:376:17
224+
--> tests/ui/or_fun_call.rs:380:17
201225
|
202226
LL | let _ = opt.unwrap_or({ f() }); // suggest `.unwrap_or_else(f)`
203227
| ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(f)`
204228

205229
error: function call inside of `unwrap_or`
206-
--> tests/ui/or_fun_call.rs:381:17
230+
--> tests/ui/or_fun_call.rs:385:17
207231
|
208232
LL | let _ = opt.unwrap_or(f() + 1); // suggest `.unwrap_or_else(|| f() + 1)`
209233
| ^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| f() + 1)`
210234

211235
error: function call inside of `unwrap_or`
212-
--> tests/ui/or_fun_call.rs:386:17
236+
--> tests/ui/or_fun_call.rs:390:17
213237
|
214238
LL | let _ = opt.unwrap_or({
215239
| _________________^
@@ -229,22 +253,22 @@ LL ~ });
229253
|
230254

231255
error: function call inside of `map_or`
232-
--> tests/ui/or_fun_call.rs:392:17
256+
--> tests/ui/or_fun_call.rs:396:17
233257
|
234258
LL | let _ = opt.map_or(f() + 1, |v| v); // suggest `.map_or_else(|| f() + 1, |v| v)`
235259
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `map_or_else(|| f() + 1, |v| v)`
236260

237261
error: use of `unwrap_or` to construct default value
238-
--> tests/ui/or_fun_call.rs:397:17
262+
--> tests/ui/or_fun_call.rs:401:17
239263
|
240264
LL | let _ = opt.unwrap_or({ i32::default() });
241265
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_default()`
242266

243267
error: function call inside of `unwrap_or`
244-
--> tests/ui/or_fun_call.rs:404:21
268+
--> tests/ui/or_fun_call.rs:408:21
245269
|
246270
LL | let _ = opt_foo.unwrap_or(Foo { val: String::default() });
247271
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Foo { val: String::default() })`
248272

249-
error: aborting due to 38 previous errors
273+
error: aborting due to 42 previous errors
250274

0 commit comments

Comments
 (0)