Skip to content

Commit eb00908

Browse files
committed
fix: derivable_impls FN when enum is qualified with Self
1 parent 450e25b commit eb00908

File tree

6 files changed

+95
-25
lines changed

6 files changed

+95
-25
lines changed

clippy_lints/src/derivable_impls.rs

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

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

8888
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")
89+
(!is_const)
90+
.then_some("derive")
91+
.or_else(|| cx.tcx.features().enabled(sym::derive_const).then_some("derive_const"))
9692
}
9793

9894
#[expect(clippy::too_many_arguments)]
@@ -137,18 +133,18 @@ fn check_struct<'tcx>(
137133
ExprKind::Tup(fields) => fields.iter().all(is_default_without_adjusts),
138134
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(is_default_without_adjusts),
139135
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_without_adjusts(ef.expr)),
140-
_ => false,
141-
};
142-
143-
let Some(derive_snippet) = determine_derive_macro(cx, is_const) else {
144-
return;
136+
_ => return,
145137
};
146138

147-
if should_emit {
139+
if should_emit && let Some(derive_snippet) = determine_derive_macro(cx, is_const) {
148140
let struct_span = cx.tcx.def_span(adt_def.did());
141+
let indent_enum = indent_of(cx, struct_span).unwrap_or(0);
149142
let suggestions = vec![
150143
(item.span, String::new()), // Remove the manual implementation
151-
(struct_span.shrink_to_lo(), format!("#[{derive_snippet}(Default)]\n")), // 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
152148
];
153149

154150
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
@@ -161,17 +157,41 @@ fn check_struct<'tcx>(
161157
}
162158
}
163159

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+
164187
fn check_enum<'tcx>(
165188
cx: &LateContext<'tcx>,
166-
item: &'tcx Item<'_>,
167-
func_expr: &Expr<'_>,
168-
adt_def: AdtDef<'_>,
189+
item: &'tcx Item<'tcx>,
190+
func_expr: &'tcx Expr<'tcx>,
191+
adt_def: AdtDef<'tcx>,
169192
is_const: bool,
170193
) {
171-
if let ExprKind::Path(QPath::Resolved(None, p)) = &peel_blocks(func_expr).kind
172-
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res
173-
&& let variant_id = cx.tcx.parent(id)
174-
&& let Some(variant_def) = adt_def.variants().iter().find(|v| v.def_id == variant_id)
194+
if let Some(variant_def) = extract_enum_variant(cx, func_expr, adt_def)
175195
&& variant_def.fields.is_empty()
176196
&& !variant_def.is_field_list_non_exhaustive()
177197
{

tests/ui/derivable_impls.fixed

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,4 +352,16 @@ mod issue15493 {
352352
}
353353
}
354354

355+
mod issue15536 {
356+
#[derive(Copy, Clone)]
357+
#[derive(Default)]
358+
enum Bar {
359+
#[default]
360+
A,
361+
B,
362+
}
363+
364+
365+
}
366+
355367
fn main() {}

tests/ui/derivable_impls.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,4 +422,19 @@ mod issue15493 {
422422
}
423423
}
424424

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+
425440
fn main() {}

tests/ui/derivable_impls.stderr

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -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

tests/ui/derivable_impls_derive_const.fixed

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ mod issue15493 {
77
#[derive(Copy, Clone)]
88
#[repr(transparent)]
99
#[derive_const(Default)]
10-
struct Foo(u64);
10+
struct Foo(u64);
1111

1212

1313

tests/ui/derivable_impls_derive_const.stderr

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ LL | | }
1414
help: replace the manual implementation with a derive attribute
1515
|
1616
LL ~ #[derive_const(Default)]
17-
LL ~ struct Foo(u64);
17+
LL ~ struct Foo(u64);
1818
LL |
1919
LL ~
2020
|

0 commit comments

Comments
 (0)