diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs index ae8f6b79449..c987fdb1a67 100644 --- a/clippy_lints/src/derivable_impls.rs +++ b/clippy_lints/src/derivable_impls.rs @@ -1,11 +1,13 @@ use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::indent_of; use clippy_utils::{is_default_equivalent, peel_blocks}; use rustc_errors::Applicability; use rustc_hir::{ - def::{DefKind, Res}, - Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind, + def::{CtorKind, CtorOf, DefKind, Res}, + Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, Ty, TyKind, }; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{AdtDef, DefIdTree}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -61,6 +63,98 @@ fn is_path_self(e: &Expr<'_>) -> bool { } } +fn check_struct<'tcx>( + cx: &LateContext<'tcx>, + item: &'tcx Item<'_>, + self_ty: &Ty<'_>, + func_expr: &Expr<'_>, + adt_def: AdtDef<'_>, +) { + if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind { + if let Some(PathSegment { args: Some(a), .. }) = p.segments.last() { + for arg in a.args { + if !matches!(arg, GenericArg::Lifetime(_)) { + return; + } + } + } + } + let should_emit = match peel_blocks(func_expr).kind { + ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)), + ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)), + ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)), + _ => false, + }; + + if should_emit { + let struct_span = cx.tcx.def_span(adt_def.did()); + span_lint_and_then(cx, DERIVABLE_IMPLS, item.span, "this `impl` can be derived", |diag| { + diag.span_suggestion_hidden( + item.span, + "remove the manual implementation...", + String::new(), + Applicability::MachineApplicable, + ); + diag.span_suggestion( + struct_span.shrink_to_lo(), + "...and instead derive it", + "#[derive(Default)]\n".to_string(), + Applicability::MachineApplicable, + ); + }); + } +} + +fn check_enum<'tcx>(cx: &LateContext<'tcx>, item: &'tcx Item<'_>, func_expr: &Expr<'_>, adt_def: AdtDef<'_>) { + if_chain! { + if let ExprKind::Path(QPath::Resolved(None, p)) = &peel_blocks(func_expr).kind; + if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), id) = p.res; + if let variant_id = cx.tcx.parent(id); + if let Some(variant_def) = adt_def.variants().iter().find(|v| v.def_id == variant_id); + if variant_def.fields.is_empty(); + if !variant_def.is_field_list_non_exhaustive(); + + then { + let enum_span = cx.tcx.def_span(adt_def.did()); + let indent_enum = indent_of(cx, enum_span).unwrap_or(0); + let variant_span = cx.tcx.def_span(variant_def.def_id); + let indent_variant = indent_of(cx, variant_span).unwrap_or(0); + span_lint_and_then( + cx, + DERIVABLE_IMPLS, + item.span, + "this `impl` can be derived", + |diag| { + diag.span_suggestion_hidden( + item.span, + "remove the manual implementation...", + String::new(), + Applicability::MachineApplicable + ); + diag.span_suggestion( + enum_span.shrink_to_lo(), + "...and instead derive it...", + format!( + "#[derive(Default)]\n{indent}", + indent = " ".repeat(indent_enum), + ), + Applicability::MachineApplicable + ); + diag.span_suggestion( + variant_span.shrink_to_lo(), + "...and mark the default variant", + format!( + "#[default]\n{indent}", + indent = " ".repeat(indent_variant), + ), + Applicability::MachineApplicable + ); + } + ); + } + } +} + impl<'tcx> LateLintPass<'tcx> for DerivableImpls { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) { if_chain! { @@ -83,47 +177,12 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls { if !attrs.iter().any(|attr| attr.doc_str().is_some()); if let child_attrs = cx.tcx.hir().attrs(impl_item_hir); if !child_attrs.iter().any(|attr| attr.doc_str().is_some()); - if adt_def.is_struct(); - then { - if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind { - if let Some(PathSegment { args: Some(a), .. }) = p.segments.last() { - for arg in a.args { - if !matches!(arg, GenericArg::Lifetime(_)) { - return; - } - } - } - } - let should_emit = match peel_blocks(func_expr).kind { - ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)), - ExprKind::Call(callee, args) - if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)), - ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)), - _ => false, - }; - if should_emit { - let struct_span = cx.tcx.def_span(adt_def.did()); - span_lint_and_then( - cx, - DERIVABLE_IMPLS, - item.span, - "this `impl` can be derived", - |diag| { - diag.span_suggestion_hidden( - item.span, - "remove the manual implementation...", - String::new(), - Applicability::MachineApplicable - ); - diag.span_suggestion( - struct_span.shrink_to_lo(), - "...and instead derive it", - "#[derive(Default)]\n".to_string(), - Applicability::MachineApplicable - ); - } - ); + then { + if adt_def.is_struct() { + check_struct(cx, item, self_ty, func_expr, adt_def); + } else if adt_def.is_enum() { + check_enum(cx, item, func_expr, adt_def); } } } diff --git a/tests/ui/derivable_impls.fixed b/tests/ui/derivable_impls.fixed index 7dcdfb0937e..ee8456f5deb 100644 --- a/tests/ui/derivable_impls.fixed +++ b/tests/ui/derivable_impls.fixed @@ -210,4 +210,25 @@ impl Default for IntOrString { } } +#[derive(Default)] +pub enum SimpleEnum { + Foo, + #[default] + Bar, +} + + + +pub enum NonExhaustiveEnum { + Foo, + #[non_exhaustive] + Bar, +} + +impl Default for NonExhaustiveEnum { + fn default() -> Self { + NonExhaustiveEnum::Bar + } +} + fn main() {} diff --git a/tests/ui/derivable_impls.rs b/tests/ui/derivable_impls.rs index 625cbcdde23..14af419bcad 100644 --- a/tests/ui/derivable_impls.rs +++ b/tests/ui/derivable_impls.rs @@ -244,4 +244,27 @@ impl Default for IntOrString { } } +pub enum SimpleEnum { + Foo, + Bar, +} + +impl Default for SimpleEnum { + fn default() -> Self { + SimpleEnum::Bar + } +} + +pub enum NonExhaustiveEnum { + Foo, + #[non_exhaustive] + Bar, +} + +impl Default for NonExhaustiveEnum { + fn default() -> Self { + NonExhaustiveEnum::Bar + } +} + fn main() {} diff --git a/tests/ui/derivable_impls.stderr b/tests/ui/derivable_impls.stderr index c1db5a58b1f..81963c3be5b 100644 --- a/tests/ui/derivable_impls.stderr +++ b/tests/ui/derivable_impls.stderr @@ -113,5 +113,26 @@ help: ...and instead derive it LL | #[derive(Default)] | -error: aborting due to 7 previous errors +error: this `impl` can be derived + --> $DIR/derivable_impls.rs:252:1 + | +LL | / impl Default for SimpleEnum { +LL | | fn default() -> Self { +LL | | SimpleEnum::Bar +LL | | } +LL | | } + | |_^ + | + = help: remove the manual implementation... +help: ...and instead derive it... + | +LL | #[derive(Default)] + | +help: ...and mark the default variant + | +LL ~ #[default] +LL ~ Bar, + | + +error: aborting due to 8 previous errors