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`
This commit is contained in:
commit
55286e796f
8 changed files with 287 additions and 31 deletions
|
|
@ -10,7 +10,7 @@ use rustc_hir::{
|
|||
};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::adjustment::{Adjust, PointerCoercion};
|
||||
use rustc_middle::ty::{self, AdtDef, GenericArgsRef, Ty, TypeckResults};
|
||||
use rustc_middle::ty::{self, AdtDef, GenericArgsRef, Ty, TypeckResults, VariantDef};
|
||||
use rustc_session::impl_lint_pass;
|
||||
use rustc_span::sym;
|
||||
|
||||
|
|
@ -85,6 +85,13 @@ fn contains_trait_object(ty: Ty<'_>) -> bool {
|
|||
}
|
||||
}
|
||||
|
||||
fn determine_derive_macro(cx: &LateContext<'_>, is_const: bool) -> Option<&'static str> {
|
||||
(!is_const)
|
||||
.then_some("derive")
|
||||
.or_else(|| cx.tcx.features().enabled(sym::derive_const).then_some("derive_const"))
|
||||
}
|
||||
|
||||
#[expect(clippy::too_many_arguments)]
|
||||
fn check_struct<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
item: &'tcx Item<'_>,
|
||||
|
|
@ -93,6 +100,7 @@ fn check_struct<'tcx>(
|
|||
adt_def: AdtDef<'_>,
|
||||
ty_args: GenericArgsRef<'_>,
|
||||
typeck_results: &'tcx TypeckResults<'tcx>,
|
||||
is_const: bool,
|
||||
) {
|
||||
if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind
|
||||
&& let Some(PathSegment { args, .. }) = p.segments.last()
|
||||
|
|
@ -125,14 +133,18 @@ fn check_struct<'tcx>(
|
|||
ExprKind::Tup(fields) => fields.iter().all(is_default_without_adjusts),
|
||||
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(is_default_without_adjusts),
|
||||
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_without_adjusts(ef.expr)),
|
||||
_ => false,
|
||||
_ => return,
|
||||
};
|
||||
|
||||
if should_emit {
|
||||
if should_emit && let Some(derive_snippet) = determine_derive_macro(cx, is_const) {
|
||||
let struct_span = cx.tcx.def_span(adt_def.did());
|
||||
let indent_enum = indent_of(cx, struct_span).unwrap_or(0);
|
||||
let suggestions = vec![
|
||||
(item.span, String::new()), // Remove the manual implementation
|
||||
(struct_span.shrink_to_lo(), "#[derive(Default)]\n".to_string()), // Add the derive attribute
|
||||
(
|
||||
struct_span.shrink_to_lo(),
|
||||
format!("#[{derive_snippet}(Default)]\n{}", " ".repeat(indent_enum)),
|
||||
), // Add the derive attribute
|
||||
];
|
||||
|
||||
span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| {
|
||||
|
|
@ -145,11 +157,41 @@ fn check_struct<'tcx>(
|
|||
}
|
||||
}
|
||||
|
||||
fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Expr<'_>, adt_def: AdtDef<'_>) {
|
||||
if let ExprKind::Path(QPath::Resolved(None, p)) = &peel_blocks(func_expr).kind
|
||||
&& let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res
|
||||
&& let variant_id = cx.tcx.parent(id)
|
||||
&& let Some(variant_def) = adt_def.variants().iter().find(|v| v.def_id == variant_id)
|
||||
fn extract_enum_variant<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
func_expr: &'tcx Expr<'tcx>,
|
||||
adt_def: AdtDef<'tcx>,
|
||||
) -> Option<&'tcx VariantDef> {
|
||||
match &peel_blocks(func_expr).kind {
|
||||
ExprKind::Path(QPath::Resolved(None, p))
|
||||
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res
|
||||
&& let variant_id = cx.tcx.parent(id)
|
||||
&& let Some(variant_def) = adt_def.variants().iter().find(|v| v.def_id == variant_id) =>
|
||||
{
|
||||
Some(variant_def)
|
||||
},
|
||||
ExprKind::Path(QPath::TypeRelative(ty, segment))
|
||||
if let TyKind::Path(QPath::Resolved(None, p)) = &ty.kind
|
||||
&& let Res::SelfTyAlias {
|
||||
is_trait_impl: true, ..
|
||||
} = p.res
|
||||
&& let variant_ident = segment.ident
|
||||
&& let Some(variant_def) = adt_def.variants().iter().find(|v| v.ident(cx.tcx) == variant_ident) =>
|
||||
{
|
||||
Some(variant_def)
|
||||
},
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
fn check_enum<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
item: &'tcx Item<'tcx>,
|
||||
func_expr: &'tcx Expr<'tcx>,
|
||||
adt_def: AdtDef<'tcx>,
|
||||
is_const: bool,
|
||||
) {
|
||||
if let Some(variant_def) = extract_enum_variant(cx, func_expr, adt_def)
|
||||
&& variant_def.fields.is_empty()
|
||||
&& !variant_def.is_field_list_non_exhaustive()
|
||||
{
|
||||
|
|
@ -158,11 +200,15 @@ fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Ex
|
|||
let variant_span = cx.tcx.def_span(variant_def.def_id);
|
||||
let indent_variant = indent_of(cx, variant_span).unwrap_or(0);
|
||||
|
||||
let Some(derive_snippet) = determine_derive_macro(cx, is_const) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let suggestions = vec![
|
||||
(item.span, String::new()), // Remove the manual implementation
|
||||
(
|
||||
enum_span.shrink_to_lo(),
|
||||
format!("#[derive(Default)]\n{}", " ".repeat(indent_enum)),
|
||||
format!("#[{derive_snippet}(Default)]\n{}", " ".repeat(indent_enum)),
|
||||
), // Add the derive attribute
|
||||
(
|
||||
variant_span.shrink_to_lo(),
|
||||
|
|
@ -201,10 +247,20 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
|
|||
&& !attrs.iter().any(|attr| attr.doc_str().is_some())
|
||||
&& cx.tcx.hir_attrs(impl_item_hir).is_empty()
|
||||
{
|
||||
let is_const = of_trait.constness == hir::Constness::Const;
|
||||
if adt_def.is_struct() {
|
||||
check_struct(cx, item, self_ty, func_expr, adt_def, args, cx.tcx.typeck_body(*b));
|
||||
check_struct(
|
||||
cx,
|
||||
item,
|
||||
self_ty,
|
||||
func_expr,
|
||||
adt_def,
|
||||
args,
|
||||
cx.tcx.typeck_body(*b),
|
||||
is_const,
|
||||
);
|
||||
} else if adt_def.is_enum() && self.msrv.meets(cx, msrvs::DEFAULT_ENUM_ATTRIBUTE) {
|
||||
check_enum(cx, item, func_expr, adt_def);
|
||||
check_enum(cx, item, func_expr, adt_def, is_const);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -226,11 +226,12 @@ impl<'a, 'tcx> SigDropChecker<'a, 'tcx> {
|
|||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug)]
|
||||
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Debug, Default)]
|
||||
enum SigDropHolder {
|
||||
/// No values with significant drop present in this expression.
|
||||
///
|
||||
/// Expressions that we've emitted lints do not count.
|
||||
#[default]
|
||||
None,
|
||||
/// Some field in this expression references to values with significant drop.
|
||||
///
|
||||
|
|
@ -244,12 +245,6 @@ enum SigDropHolder {
|
|||
Moved,
|
||||
}
|
||||
|
||||
impl Default for SigDropHolder {
|
||||
fn default() -> Self {
|
||||
Self::None
|
||||
}
|
||||
}
|
||||
|
||||
struct SigDropHelper<'a, 'tcx> {
|
||||
cx: &'a LateContext<'tcx>,
|
||||
parent_expr: Option<&'tcx Expr<'tcx>>,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
#![allow(dead_code)]
|
||||
#![feature(const_trait_impl)]
|
||||
#![feature(const_default)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
|
|
@ -326,4 +328,40 @@ mod issue11368 {
|
|||
}
|
||||
}
|
||||
|
||||
mod issue15493 {
|
||||
#[derive(Copy, Clone)]
|
||||
#[repr(transparent)]
|
||||
struct Foo(u64);
|
||||
|
||||
impl const Default for Foo {
|
||||
fn default() -> Self {
|
||||
Self(0)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
enum Bar {
|
||||
A,
|
||||
B,
|
||||
}
|
||||
|
||||
impl const Default for Bar {
|
||||
fn default() -> Self {
|
||||
Bar::A
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mod issue15536 {
|
||||
#[derive(Copy, Clone)]
|
||||
#[derive(Default)]
|
||||
enum Bar {
|
||||
#[default]
|
||||
A,
|
||||
B,
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
#![allow(dead_code)]
|
||||
#![feature(const_trait_impl)]
|
||||
#![feature(const_default)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
|
||||
|
|
@ -396,4 +398,43 @@ mod issue11368 {
|
|||
}
|
||||
}
|
||||
|
||||
mod issue15493 {
|
||||
#[derive(Copy, Clone)]
|
||||
#[repr(transparent)]
|
||||
struct Foo(u64);
|
||||
|
||||
impl const Default for Foo {
|
||||
fn default() -> Self {
|
||||
Self(0)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
enum Bar {
|
||||
A,
|
||||
B,
|
||||
}
|
||||
|
||||
impl const Default for Bar {
|
||||
fn default() -> Self {
|
||||
Bar::A
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
mod issue15536 {
|
||||
#[derive(Copy, Clone)]
|
||||
enum Bar {
|
||||
A,
|
||||
B,
|
||||
}
|
||||
|
||||
impl Default for Bar {
|
||||
//~^ derivable_impls
|
||||
fn default() -> Self {
|
||||
Self::A
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:20:1
|
||||
--> tests/ui/derivable_impls.rs:22:1
|
||||
|
|
||||
LL | / impl std::default::Default for FooDefault<'_> {
|
||||
LL | |
|
||||
|
|
@ -18,7 +18,7 @@ LL ~ struct FooDefault<'a> {
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:42:1
|
||||
--> tests/ui/derivable_impls.rs:44:1
|
||||
|
|
||||
LL | / impl std::default::Default for TupleDefault {
|
||||
LL | |
|
||||
|
|
@ -35,7 +35,7 @@ LL ~ struct TupleDefault(bool, i32, u64);
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:95:1
|
||||
--> tests/ui/derivable_impls.rs:97:1
|
||||
|
|
||||
LL | / impl Default for StrDefault<'_> {
|
||||
LL | |
|
||||
|
|
@ -52,7 +52,7 @@ LL ~ struct StrDefault<'a>(&'a str);
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:122:1
|
||||
--> tests/ui/derivable_impls.rs:124:1
|
||||
|
|
||||
LL | / impl Default for Y {
|
||||
LL | |
|
||||
|
|
@ -69,7 +69,7 @@ LL ~ struct Y(u32);
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:162:1
|
||||
--> tests/ui/derivable_impls.rs:164:1
|
||||
|
|
||||
LL | / impl Default for WithoutSelfCurly {
|
||||
LL | |
|
||||
|
|
@ -86,7 +86,7 @@ LL ~ struct WithoutSelfCurly {
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:171:1
|
||||
--> tests/ui/derivable_impls.rs:173:1
|
||||
|
|
||||
LL | / impl Default for WithoutSelfParan {
|
||||
LL | |
|
||||
|
|
@ -103,7 +103,7 @@ LL ~ struct WithoutSelfParan(bool);
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:194:1
|
||||
--> tests/ui/derivable_impls.rs:196:1
|
||||
|
|
||||
LL | / impl Default for DirectDefaultDefaultCall {
|
||||
LL | |
|
||||
|
|
@ -119,7 +119,7 @@ LL ~ pub struct DirectDefaultDefaultCall {
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:206:1
|
||||
--> tests/ui/derivable_impls.rs:208:1
|
||||
|
|
||||
LL | / impl Default for EquivalentToDefaultDefaultCallVec {
|
||||
LL | |
|
||||
|
|
@ -135,7 +135,7 @@ LL ~ pub struct EquivalentToDefaultDefaultCallVec {
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:234:1
|
||||
--> tests/ui/derivable_impls.rs:236:1
|
||||
|
|
||||
LL | / impl Default for EquivalentToDefaultDefaultCallLocal {
|
||||
LL | |
|
||||
|
|
@ -151,7 +151,7 @@ LL ~ pub struct EquivalentToDefaultDefaultCallLocal {
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:274:1
|
||||
--> tests/ui/derivable_impls.rs:276:1
|
||||
|
|
||||
LL | / impl Default for RepeatDefault1 {
|
||||
LL | |
|
||||
|
|
@ -168,7 +168,7 @@ LL ~ pub struct RepeatDefault1 {
|
|||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:309:1
|
||||
--> tests/ui/derivable_impls.rs:311:1
|
||||
|
|
||||
LL | / impl Default for SimpleEnum {
|
||||
LL | |
|
||||
|
|
@ -187,5 +187,28 @@ LL ~ #[default]
|
|||
LL ~ Bar,
|
||||
|
|
||||
|
||||
error: aborting due to 11 previous errors
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls.rs:432:5
|
||||
|
|
||||
LL | / impl Default for Bar {
|
||||
LL | |
|
||||
LL | | fn default() -> Self {
|
||||
LL | | Self::A
|
||||
LL | | }
|
||||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
help: replace the manual implementation with a derive attribute and mark the default variant
|
||||
|
|
||||
LL ~ #[derive(Default)]
|
||||
LL ~ enum Bar {
|
||||
LL ~ #[default]
|
||||
LL ~ A,
|
||||
LL | B,
|
||||
LL | }
|
||||
LL |
|
||||
LL ~
|
||||
|
|
||||
|
||||
error: aborting due to 12 previous errors
|
||||
|
||||
|
|
|
|||
25
tests/ui/derivable_impls_derive_const.fixed
Normal file
25
tests/ui/derivable_impls_derive_const.fixed
Normal file
|
|
@ -0,0 +1,25 @@
|
|||
#![allow(dead_code)]
|
||||
#![feature(const_trait_impl)]
|
||||
#![feature(const_default)]
|
||||
#![feature(derive_const)]
|
||||
|
||||
mod issue15493 {
|
||||
#[derive(Copy, Clone)]
|
||||
#[repr(transparent)]
|
||||
#[derive_const(Default)]
|
||||
struct Foo(u64);
|
||||
|
||||
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
#[derive_const(Default)]
|
||||
enum Bar {
|
||||
#[default]
|
||||
A,
|
||||
B,
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
32
tests/ui/derivable_impls_derive_const.rs
Normal file
32
tests/ui/derivable_impls_derive_const.rs
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
#![allow(dead_code)]
|
||||
#![feature(const_trait_impl)]
|
||||
#![feature(const_default)]
|
||||
#![feature(derive_const)]
|
||||
|
||||
mod issue15493 {
|
||||
#[derive(Copy, Clone)]
|
||||
#[repr(transparent)]
|
||||
struct Foo(u64);
|
||||
|
||||
impl const Default for Foo {
|
||||
//~^ derivable_impls
|
||||
fn default() -> Self {
|
||||
Self(0)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Copy, Clone)]
|
||||
enum Bar {
|
||||
A,
|
||||
B,
|
||||
}
|
||||
|
||||
impl const Default for Bar {
|
||||
//~^ derivable_impls
|
||||
fn default() -> Self {
|
||||
Bar::A
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
46
tests/ui/derivable_impls_derive_const.stderr
Normal file
46
tests/ui/derivable_impls_derive_const.stderr
Normal file
|
|
@ -0,0 +1,46 @@
|
|||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls_derive_const.rs:11:5
|
||||
|
|
||||
LL | / impl const Default for Foo {
|
||||
LL | |
|
||||
LL | | fn default() -> Self {
|
||||
LL | | Self(0)
|
||||
LL | | }
|
||||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
= note: `-D clippy::derivable-impls` implied by `-D warnings`
|
||||
= help: to override `-D warnings` add `#[allow(clippy::derivable_impls)]`
|
||||
help: replace the manual implementation with a derive attribute
|
||||
|
|
||||
LL ~ #[derive_const(Default)]
|
||||
LL ~ struct Foo(u64);
|
||||
LL |
|
||||
LL ~
|
||||
|
|
||||
|
||||
error: this `impl` can be derived
|
||||
--> tests/ui/derivable_impls_derive_const.rs:24:5
|
||||
|
|
||||
LL | / impl const Default for Bar {
|
||||
LL | |
|
||||
LL | | fn default() -> Self {
|
||||
LL | | Bar::A
|
||||
LL | | }
|
||||
LL | | }
|
||||
| |_____^
|
||||
|
|
||||
help: replace the manual implementation with a derive attribute and mark the default variant
|
||||
|
|
||||
LL ~ #[derive_const(Default)]
|
||||
LL ~ enum Bar {
|
||||
LL ~ #[default]
|
||||
LL ~ A,
|
||||
LL | B,
|
||||
LL | }
|
||||
LL |
|
||||
LL ~
|
||||
|
|
||||
|
||||
error: aborting due to 2 previous errors
|
||||
|
||||
Loading…
Add table
Add a link
Reference in a new issue