Skip to content

Commit 55286e7

Browse files
samueltardieusamueltardieu
authored
Fix derivable_impls suggests wrongly on derive_const (#15535)
Closes rust-lang/rust-clippy#15493 Closes rust-lang/rust-clippy#15536 changelog: [`derivable_impls`] fix wrong suggestions on `derive_const`
2 parents 3a3d3e4 + e01cf9b commit 55286e7

8 files changed

+287
-31
lines changed

‎clippy_lints/src/derivable_impls.rs‎

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use rustc_hir::{
1010
};
1111
use rustc_lint::{LateContext, LateLintPass};
1212
use rustc_middle::ty::adjustment::{Adjust, PointerCoercion};
13-
use rustc_middle::ty::{self, AdtDef, GenericArgsRef, Ty, TypeckResults};
13+
use rustc_middle::ty::{self, AdtDef, GenericArgsRef, Ty, TypeckResults, VariantDef};
1414
use rustc_session::impl_lint_pass;
1515
use rustc_span::sym;
1616

@@ -85,6 +85,13 @@ fn contains_trait_object(ty: Ty<'_>) -> bool {
8585
}
8686
}
8787

88+
fn determine_derive_macro(cx: &LateContext<'_>, is_const: bool) -> Option<&'static str> {
89+
(!is_const)
90+
.then_some("derive")
91+
.or_else(|| cx.tcx.features().enabled(sym::derive_const).then_some("derive_const"))
92+
}
93+
94+
#[expect(clippy::too_many_arguments)]
8895
fn check_struct<'tcx>(
8996
cx: &LateContext<'tcx>,
9097
item: &'tcx Item<'_>,
@@ -93,6 +100,7 @@ fn check_struct<'tcx>(
93100
adt_def: AdtDef<'_>,
94101
ty_args: GenericArgsRef<'_>,
95102
typeck_results: &'tcx TypeckResults<'tcx>,
103+
is_const: bool,
96104
) {
97105
if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind
98106
&& let Some(PathSegment { args, .. }) = p.segments.last()
@@ -125,14 +133,18 @@ fn check_struct<'tcx>(
125133
ExprKind::Tup(fields) => fields.iter().all(is_default_without_adjusts),
126134
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(is_default_without_adjusts),
127135
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_without_adjusts(ef.expr)),
128-
_ => false,
136+
_ => return,
129137
};
130138

131-
if should_emit {
139+
if should_emit && let Some(derive_snippet) = determine_derive_macro(cx, is_const) {
132140
let struct_span = cx.tcx.def_span(adt_def.did());
141+
let indent_enum = indent_of(cx, struct_span).unwrap_or(0);
133142
let suggestions = vec![
134143
(item.span, String::new()), // Remove the manual implementation
135-
(struct_span.shrink_to_lo(), "#[derive(Default)]\n".to_string()), // Add the derive attribute
144+
(
145+
struct_span.shrink_to_lo(),
146+
format!("#[{derive_snippet}(Default)]\n{}", " ".repeat(indent_enum)),
147+
), // Add the derive attribute
136148
];
137149

138150
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
@@ -145,11 +157,41 @@ fn check_struct<'tcx>(
145157
}
146158
}
147159

148-
fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Expr<'_>, adt_def: AdtDef<'_>) {
149-
if let ExprKind::Path(QPath::Resolved(None, p)) = &peel_blocks(func_expr).kind
150-
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res
151-
&& let variant_id = cx.tcx.parent(id)
152-
&& let Some(variant_def) = adt_def.variants().iter().find(|v| v.def_id == variant_id)
160+
fn extract_enum_variant<'tcx>(
161+
cx: &LateContext<'tcx>,
162+
func_expr: &'tcx Expr<'tcx>,
163+
adt_def: AdtDef<'tcx>,
164+
) -> Option<&'tcx VariantDef> {
165+
match &peel_blocks(func_expr).kind {
166+
ExprKind::Path(QPath::Resolved(None, p))
167+
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res
168+
&& let variant_id = cx.tcx.parent(id)
169+
&& let Some(variant_def) = adt_def.variants().iter().find(|v| v.def_id == variant_id) =>
170+
{
171+
Some(variant_def)
172+
},
173+
ExprKind::Path(QPath::TypeRelative(ty, segment))
174+
if let TyKind::Path(QPath::Resolved(None, p)) = &ty.kind
175+
&& let Res::SelfTyAlias {
176+
is_trait_impl: true, ..
177+
} = p.res
178+
&& let variant_ident = segment.ident
179+
&& let Some(variant_def) = adt_def.variants().iter().find(|v| v.ident(cx.tcx) == variant_ident) =>
180+
{
181+
Some(variant_def)
182+
},
183+
_ => None,
184+
}
185+
}
186+
187+
fn check_enum<'tcx>(
188+
cx: &LateContext<'tcx>,
189+
item: &'tcx Item<'tcx>,
190+
func_expr: &'tcx Expr<'tcx>,
191+
adt_def: AdtDef<'tcx>,
192+
is_const: bool,
193+
) {
194+
if let Some(variant_def) = extract_enum_variant(cx, func_expr, adt_def)
153195
&& variant_def.fields.is_empty()
154196
&& !variant_def.is_field_list_non_exhaustive()
155197
{
@@ -158,11 +200,15 @@ fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Ex
158200
let variant_span = cx.tcx.def_span(variant_def.def_id);
159201
let indent_variant = indent_of(cx, variant_span).unwrap_or(0);
160202

203+
let Some(derive_snippet) = determine_derive_macro(cx, is_const) else {
204+
return;
205+
};
206+
161207
let suggestions = vec![
162208
(item.span, String::new()), // Remove the manual implementation
163209
(
164210
enum_span.shrink_to_lo(),
165-
format!("#[derive(Default)]\n{}", " ".repeat(indent_enum)),
211+
format!("#[{derive_snippet}(Default)]\n{}", " ".repeat(indent_enum)),
166212
), // Add the derive attribute
167213
(
168214
variant_span.shrink_to_lo(),
@@ -201,10 +247,20 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
201247
&& !attrs.iter().any(|attr| attr.doc_str().is_some())
202248
&& cx.tcx.hir_attrs(impl_item_hir).is_empty()
203249
{
250+
let is_const = of_trait.constness == hir::Constness::Const;
204251
if adt_def.is_struct() {
205-
check_struct(cx, item, self_ty, func_expr, adt_def, args, cx.tcx.typeck_body(*b));
252+
check_struct(
253+
cx,
254+
item,
255+
self_ty,
256+
func_expr,
257+
adt_def,
258+
args,
259+
cx.tcx.typeck_body(*b),
260+
is_const,
261+
);
206262
} else if adt_def.is_enum() && self.msrv.meets(cx, msrvs::DEFAULT_ENUM_ATTRIBUTE) {
207-
check_enum(cx, item, func_expr, adt_def);
263+
check_enum(cx, item, func_expr, adt_def, is_const);
208264
}
209265
}
210266
}

‎clippy_lints/src/matches/significant_drop_in_scrutinee.rs‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,12 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
226226
}
227227
}
228228

229-
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
229+
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Default)]
230230
enum SigDropHolder {
231231
/// No values with significant drop present in this expression.
232232
///
233233
/// Expressions that we've emitted lints do not count.
234+
#[default]
234235
None,
235236
/// Some field in this expression references to values with significant drop.
236237
///
@@ -244,12 +245,6 @@ enum SigDropHolder {
244245
Moved,
245246
}
246247

247-
impl Default for SigDropHolder {
248-
fn default() -> Self {
249-
Self::None
250-
}
251-
}
252-
253248
struct SigDropHelper<'a, 'tcx> {
254249
cx: &'a LateContext<'tcx>,
255250
parent_expr: Option<&'tcx Expr<'tcx>>,

‎tests/ui/derivable_impls.fixed‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
#![allow(dead_code)]
2+
#![feature(const_trait_impl)]
3+
#![feature(const_default)]
24

35
use std::collections::HashMap;
46

@@ -326,4 +328,40 @@ mod issue11368 {
326328
}
327329
}
328330

331+
mod issue15493 {
332+
#[derive(Copy, Clone)]
333+
#[repr(transparent)]
334+
struct Foo(u64);
335+
336+
impl const Default for Foo {
337+
fn default() -> Self {
338+
Self(0)
339+
}
340+
}
341+
342+
#[derive(Copy, Clone)]
343+
enum Bar {
344+
A,
345+
B,
346+
}
347+
348+
impl const Default for Bar {
349+
fn default() -> Self {
350+
Bar::A
351+
}
352+
}
353+
}
354+
355+
mod issue15536 {
356+
#[derive(Copy, Clone)]
357+
#[derive(Default)]
358+
enum Bar {
359+
#[default]
360+
A,
361+
B,
362+
}
363+
364+
365+
}
366+
329367
fn main() {}

‎tests/ui/derivable_impls.rs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
#![allow(dead_code)]
2+
#![feature(const_trait_impl)]
3+
#![feature(const_default)]
24

35
use std::collections::HashMap;
46

@@ -396,4 +398,43 @@ mod issue11368 {
396398
}
397399
}
398400

401+
mod issue15493 {
402+
#[derive(Copy, Clone)]
403+
#[repr(transparent)]
404+
struct Foo(u64);
405+
406+
impl const Default for Foo {
407+
fn default() -> Self {
408+
Self(0)
409+
}
410+
}
411+
412+
#[derive(Copy, Clone)]
413+
enum Bar {
414+
A,
415+
B,
416+
}
417+
418+
impl const Default for Bar {
419+
fn default() -> Self {
420+
Bar::A
421+
}
422+
}
423+
}
424+
425+
mod issue15536 {
426+
#[derive(Copy, Clone)]
427+
enum Bar {
428+
A,
429+
B,
430+
}
431+
432+
impl Default for Bar {
433+
//~^ derivable_impls
434+
fn default() -> Self {
435+
Self::A
436+
}
437+
}
438+
}
439+
399440
fn main() {}

‎tests/ui/derivable_impls.stderr‎

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this `impl` can be derived
2-
--> tests/ui/derivable_impls.rs:20:1
2+
--> tests/ui/derivable_impls.rs:22:1
33
|
44
LL | / impl std::default::Default for FooDefault<'_> {
55
LL | |
@@ -18,7 +18,7 @@ LL ~ struct FooDefault<'a> {
1818
|
1919

2020
error: this `impl` can be derived
21-
--> tests/ui/derivable_impls.rs:42:1
21+
--> tests/ui/derivable_impls.rs:44:1
2222
|
2323
LL | / impl std::default::Default for TupleDefault {
2424
LL | |
@@ -35,7 +35,7 @@ LL ~ struct TupleDefault(bool, i32, u64);
3535
|
3636

3737
error: this `impl` can be derived
38-
--> tests/ui/derivable_impls.rs:95:1
38+
--> tests/ui/derivable_impls.rs:97:1
3939
|
4040
LL | / impl Default for StrDefault<'_> {
4141
LL | |
@@ -52,7 +52,7 @@ LL ~ struct StrDefault<'a>(&'a str);
5252
|
5353

5454
error: this `impl` can be derived
55-
--> tests/ui/derivable_impls.rs:122:1
55+
--> tests/ui/derivable_impls.rs:124:1
5656
|
5757
LL | / impl Default for Y {
5858
LL | |
@@ -69,7 +69,7 @@ LL ~ struct Y(u32);
6969
|
7070

7171
error: this `impl` can be derived
72-
--> tests/ui/derivable_impls.rs:162:1
72+
--> tests/ui/derivable_impls.rs:164:1
7373
|
7474
LL | / impl Default for WithoutSelfCurly {
7575
LL | |
@@ -86,7 +86,7 @@ LL ~ struct WithoutSelfCurly {
8686
|
8787

8888
error: this `impl` can be derived
89-
--> tests/ui/derivable_impls.rs:171:1
89+
--> tests/ui/derivable_impls.rs:173:1
9090
|
9191
LL | / impl Default for WithoutSelfParan {
9292
LL | |
@@ -103,7 +103,7 @@ LL ~ struct WithoutSelfParan(bool);
103103
|
104104

105105
error: this `impl` can be derived
106-
--> tests/ui/derivable_impls.rs:194:1
106+
--> tests/ui/derivable_impls.rs:196:1
107107
|
108108
LL | / impl Default for DirectDefaultDefaultCall {
109109
LL | |
@@ -119,7 +119,7 @@ LL ~ pub struct DirectDefaultDefaultCall {
119119
|
120120

121121
error: this `impl` can be derived
122-
--> tests/ui/derivable_impls.rs:206:1
122+
--> tests/ui/derivable_impls.rs:208:1
123123
|
124124
LL | / impl Default for EquivalentToDefaultDefaultCallVec {
125125
LL | |
@@ -135,7 +135,7 @@ LL ~ pub struct EquivalentToDefaultDefaultCallVec {
135135
|
136136

137137
error: this `impl` can be derived
138-
--> tests/ui/derivable_impls.rs:234:1
138+
--> tests/ui/derivable_impls.rs:236:1
139139
|
140140
LL | / impl Default for EquivalentToDefaultDefaultCallLocal {
141141
LL | |
@@ -151,7 +151,7 @@ LL ~ pub struct EquivalentToDefaultDefaultCallLocal {
151151
|
152152

153153
error: this `impl` can be derived
154-
--> tests/ui/derivable_impls.rs:274:1
154+
--> tests/ui/derivable_impls.rs:276:1
155155
|
156156
LL | / impl Default for RepeatDefault1 {
157157
LL | |
@@ -168,7 +168,7 @@ LL ~ pub struct RepeatDefault1 {
168168
|
169169

170170
error: this `impl` can be derived
171-
--> tests/ui/derivable_impls.rs:309:1
171+
--> tests/ui/derivable_impls.rs:311:1
172172
|
173173
LL | / impl Default for SimpleEnum {
174174
LL | |
@@ -187,5 +187,28 @@ LL ~ #[default]
187187
LL ~ Bar,
188188
|
189189

190-
error: aborting due to 11 previous errors
190+
error: this `impl` can be derived
191+
--> tests/ui/derivable_impls.rs:432:5
192+
|
193+
LL | / impl Default for Bar {
194+
LL | |
195+
LL | | fn default() -> Self {
196+
LL | | Self::A
197+
LL | | }
198+
LL | | }
199+
| |_____^
200+
|
201+
help: replace the manual implementation with a derive attribute and mark the default variant
202+
|
203+
LL ~ #[derive(Default)]
204+
LL ~ enum Bar {
205+
LL ~ #[default]
206+
LL ~ A,
207+
LL | B,
208+
LL | }
209+
LL |
210+
LL ~
211+
|
212+
213+
error: aborting due to 12 previous errors
191214

0 commit comments

Comments
 (0)