Skip to content

Commit 450e25b

Browse files
profetiaprofetia
committed
fix: derivable_impls suggests wrongly on derive_const 
1 parent 228f064 commit 450e25b

File tree

7 files changed

+207
-16
lines changed

7 files changed

+207
-16
lines changed

‎clippy_lints/src/derivable_impls.rs‎

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,17 @@ fn contains_trait_object(ty: Ty<'_>) -> bool {
8585
}
8686
}
8787

88+
fn determine_derive_macro(cx: &LateContext<'_>, is_const: bool) -> Option<&'static str> {
89+
if is_const {
90+
if !cx.tcx.features().enabled(sym::derive_const) {
91+
return None;
92+
}
93+
return Some("derive_const");
94+
}
95+
Some("derive")
96+
}
97+
98+
#[expect(clippy::too_many_arguments)]
8899
fn check_struct<'tcx>(
89100
cx: &LateContext<'tcx>,
90101
item: &'tcx Item<'_>,
@@ -93,6 +104,7 @@ fn check_struct<'tcx>(
93104
adt_def: AdtDef<'_>,
94105
ty_args: GenericArgsRef<'_>,
95106
typeck_results: &'tcx TypeckResults<'tcx>,
107+
is_const: bool,
96108
) {
97109
if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind
98110
&& let Some(PathSegment { args, .. }) = p.segments.last()
@@ -128,11 +140,15 @@ fn check_struct<'tcx>(
128140
_ => false,
129141
};
130142

143+
let Some(derive_snippet) = determine_derive_macro(cx, is_const) else {
144+
return;
145+
};
146+
131147
if should_emit {
132148
let struct_span = cx.tcx.def_span(adt_def.did());
133149
let suggestions = vec![
134150
(item.span, String::new()), // Remove the manual implementation
135-
(struct_span.shrink_to_lo(), "#[derive(Default)]\n".to_string()), // Add the derive attribute
151+
(struct_span.shrink_to_lo(), format!("#[{derive_snippet}(Default)]\n")), // Add the derive attribute
136152
];
137153

138154
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
@@ -145,7 +161,13 @@ fn check_struct<'tcx>(
145161
}
146162
}
147163

148-
fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Expr<'_>, adt_def: AdtDef<'_>) {
164+
fn check_enum<'tcx>(
165+
cx: &LateContext<'tcx>,
166+
item: &'tcx Item<'_>,
167+
func_expr: &Expr<'_>,
168+
adt_def: AdtDef<'_>,
169+
is_const: bool,
170+
) {
149171
if let ExprKind::Path(QPath::Resolved(None, p)) = &peel_blocks(func_expr).kind
150172
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res
151173
&& let variant_id = cx.tcx.parent(id)
@@ -158,11 +180,15 @@ fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Ex
158180
let variant_span = cx.tcx.def_span(variant_def.def_id);
159181
let indent_variant = indent_of(cx, variant_span).unwrap_or(0);
160182

183+
let Some(derive_snippet) = determine_derive_macro(cx, is_const) else {
184+
return;
185+
};
186+
161187
let suggestions = vec![
162188
(item.span, String::new()), // Remove the manual implementation
163189
(
164190
enum_span.shrink_to_lo(),
165-
format!("#[derive(Default)]\n{}", " ".repeat(indent_enum)),
191+
format!("#[{derive_snippet}(Default)]\n{}", " ".repeat(indent_enum)),
166192
), // Add the derive attribute
167193
(
168194
variant_span.shrink_to_lo(),
@@ -201,10 +227,20 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
201227
&& !attrs.iter().any(|attr| attr.doc_str().is_some())
202228
&& cx.tcx.hir_attrs(impl_item_hir).is_empty()
203229
{
230+
let is_const = of_trait.constness == hir::Constness::Const;
204231
if adt_def.is_struct() {
205-
check_struct(cx, item, self_ty, func_expr, adt_def, args, cx.tcx.typeck_body(*b));
232+
check_struct(
233+
cx,
234+
item,
235+
self_ty,
236+
func_expr,
237+
adt_def,
238+
args,
239+
cx.tcx.typeck_body(*b),
240+
is_const,
241+
);
206242
} else if adt_def.is_enum() && self.msrv.meets(cx, msrvs::DEFAULT_ENUM_ATTRIBUTE) {
207-
check_enum(cx, item, func_expr, adt_def);
243+
check_enum(cx, item, func_expr, adt_def, is_const);
208244
}
209245
}
210246
}

‎tests/ui/derivable_impls.fixed‎

Lines changed: 26 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,28 @@ 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+
329355
fn main() {}

‎tests/ui/derivable_impls.rs‎

Lines changed: 26 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,28 @@ 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+
399425
fn main() {}

‎tests/ui/derivable_impls.stderr‎

Lines changed: 11 additions & 11 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 | |

‎tests/ui/derivable_impls_derive_const.fixed‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![allow(dead_code)]
2+
#![feature(const_trait_impl)]
3+
#![feature(const_default)]
4+
#![feature(derive_const)]
5+
6+
mod issue15493 {
7+
#[derive(Copy, Clone)]
8+
#[repr(transparent)]
9+
#[derive_const(Default)]
10+
struct Foo(u64);
11+
12+
13+
14+
#[derive(Copy, Clone)]
15+
#[derive_const(Default)]
16+
enum Bar {
17+
#[default]
18+
A,
19+
B,
20+
}
21+
22+
23+
}
24+
25+
fn main() {}

‎tests/ui/derivable_impls_derive_const.rs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#![allow(dead_code)]
2+
#![feature(const_trait_impl)]
3+
#![feature(const_default)]
4+
#![feature(derive_const)]
5+
6+
mod issue15493 {
7+
#[derive(Copy, Clone)]
8+
#[repr(transparent)]
9+
struct Foo(u64);
10+
11+
impl const Default for Foo {
12+
//~^ derivable_impls
13+
fn default() -> Self {
14+
Self(0)
15+
}
16+
}
17+
18+
#[derive(Copy, Clone)]
19+
enum Bar {
20+
A,
21+
B,
22+
}
23+
24+
impl const Default for Bar {
25+
//~^ derivable_impls
26+
fn default() -> Self {
27+
Bar::A
28+
}
29+
}
30+
}
31+
32+
fn main() {}

‎tests/ui/derivable_impls_derive_const.stderr‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
error: this `impl` can be derived
2+
--> tests/ui/derivable_impls_derive_const.rs:11:5
3+
|
4+
LL | / impl const Default for Foo {
5+
LL | |
6+
LL | | fn default() -> Self {
7+
LL | | Self(0)
8+
LL | | }
9+
LL | | }
10+
| |_____^
11+
|
12+
= note: `-D clippy::derivable-impls` implied by `-D warnings`
13+
= help: to override `-D warnings` add `#[allow(clippy::derivable_impls)]`
14+
help: replace the manual implementation with a derive attribute
15+
|
16+
LL ~ #[derive_const(Default)]
17+
LL ~ struct Foo(u64);
18+
LL |
19+
LL ~
20+
|
21+
22+
error: this `impl` can be derived
23+
--> tests/ui/derivable_impls_derive_const.rs:24:5
24+
|
25+
LL | / impl const Default for Bar {
26+
LL | |
27+
LL | | fn default() -> Self {
28+
LL | | Bar::A
29+
LL | | }
30+
LL | | }
31+
| |_____^
32+
|
33+
help: replace the manual implementation with a derive attribute and mark the default variant
34+
|
35+
LL ~ #[derive_const(Default)]
36+
LL ~ enum Bar {
37+
LL ~ #[default]
38+
LL ~ A,
39+
LL | B,
40+
LL | }
41+
LL |
42+
LL ~
43+
|
44+
45+
error: aborting due to 2 previous errors
46+

0 commit comments

Comments
 (0)