diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 262e418ecbf74..b2283cf32ecea 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -32,7 +32,7 @@ use rustc_macros::{Decodable, Encodable, HashStable_Generic}; pub use rustc_span::AttrId; use rustc_span::source_map::{Spanned, respan}; use rustc_span::symbol::{Ident, Symbol, kw, sym}; -use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span}; +use rustc_span::{ErrorGuaranteed, Span}; use thin_vec::{ThinVec, thin_vec}; pub use crate::format::*; @@ -388,22 +388,15 @@ impl GenericParam { /// Represents lifetime, type and const parameters attached to a declaration of /// a function, enum, trait, etc. -#[derive(Clone, Encodable, Decodable, Debug)] +#[derive(Clone, Encodable, Decodable, Debug, Default)] pub struct Generics { pub params: ThinVec, pub where_clause: WhereClause, pub span: Span, } -impl Default for Generics { - /// Creates an instance of `Generics`. - fn default() -> Generics { - Generics { params: ThinVec::new(), where_clause: Default::default(), span: DUMMY_SP } - } -} - /// A where-clause in a definition. -#[derive(Clone, Encodable, Decodable, Debug)] +#[derive(Clone, Encodable, Decodable, Debug, Default)] pub struct WhereClause { /// `true` if we ate a `where` token. /// @@ -420,12 +413,6 @@ impl WhereClause { } } -impl Default for WhereClause { - fn default() -> WhereClause { - WhereClause { has_where_token: false, predicates: ThinVec::new(), span: DUMMY_SP } - } -} - /// A single predicate in a where-clause. #[derive(Clone, Encodable, Decodable, Debug)] pub struct WherePredicate { diff --git a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs b/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs index 6f5382ce61d3b..10dacf5fb9139 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state/fixup.rs @@ -1,7 +1,9 @@ use rustc_ast::Expr; use rustc_ast::util::{classify, parser}; -#[derive(Copy, Clone, Debug)] +// The default amount of fixing is minimal fixing. Fixups should be turned on +// in a targeted fashion where needed. +#[derive(Copy, Clone, Debug, Default)] pub(crate) struct FixupContext { /// Print expression such that it can be parsed back as a statement /// consisting of the original expression. @@ -93,20 +95,6 @@ pub(crate) struct FixupContext { parenthesize_exterior_struct_lit: bool, } -/// The default amount of fixing is minimal fixing. Fixups should be turned on -/// in a targeted fashion where needed. -impl Default for FixupContext { - fn default() -> Self { - FixupContext { - stmt: false, - leftmost_subexpression_in_stmt: false, - match_arm: false, - leftmost_subexpression_in_match_arm: false, - parenthesize_exterior_struct_lit: false, - } - } -} - impl FixupContext { /// Create the initial fixup for printing an expression in statement /// position. diff --git a/compiler/rustc_error_codes/src/error_codes/E0665.md b/compiler/rustc_error_codes/src/error_codes/E0665.md index ae54d6d15798d..d5fd2130840de 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0665.md +++ b/compiler/rustc_error_codes/src/error_codes/E0665.md @@ -16,18 +16,30 @@ The `Default` cannot be derived on an enum for the simple reason that the compiler doesn't know which value to pick by default whereas it can for a struct as long as all its fields implement the `Default` trait as well. -If you still want to implement `Default` on your enum, you'll have to do it "by -hand": +For the case where the desired default variant has no data, you can annotate +it with `#[default]` to derive it: ``` +#[derive(Default)] enum Food { + #[default] Sweet, Salty, } +``` + +In the case where the default variant does have data, you will have to +implement `Default` on your enum "by hand": + +``` +enum Food { + Sweet(i32), + Salty, +} impl Default for Food { fn default() -> Food { - Food::Sweet + Food::Sweet(1) } } ``` diff --git a/compiler/rustc_lint/src/default_could_be_derived.rs b/compiler/rustc_lint/src/default_could_be_derived.rs new file mode 100644 index 0000000000000..c6c97cfdf788f --- /dev/null +++ b/compiler/rustc_lint/src/default_could_be_derived.rs @@ -0,0 +1,520 @@ +use rustc_ast::LitKind; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; +use rustc_middle::ty::Ty; +use rustc_session::{declare_lint, declare_lint_pass}; +use rustc_span::symbol::{kw, sym}; + +use crate::{LateContext, LateLintPass}; + +declare_lint! { + /// The `default_could_be_derived` lint checks for manual `impl` blocks + /// of the `Default` trait that could have been derived. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// struct A { + /// b: Option, + /// } + /// + /// #[deny(default_could_be_derived)] + /// impl Default for A { + /// fn default() -> A { + /// A { + /// b: None, + /// } + /// } + /// } + /// + /// enum Foo { + /// Bar, + /// } + /// + /// #[deny(default_could_be_derived)] + /// impl Default for Foo { + /// fn default() -> Foo { + /// Foo::Bar + /// } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// `#[derive(Default)]` uses the `Default` impl for every field of your + /// type. If your manual `Default` impl either invokes `Default::default()` + /// or uses the same value that that associated function produces, then it + /// is better to use the derive to avoid the different `Default` impls from + /// diverging over time. + /// + /// This lint also triggers on cases where there the type has no fields, + /// so the derive for `Default` for a struct is trivial, and for an enum + /// variant with no fields, which can be annotated with `#[default]`. + pub DEFAULT_COULD_BE_DERIVED, + Deny, + "detect `Default` impl that could be derived" +} + +declare_lint! { + /// The `manual_default_for_type_with_default_fields` lint checks for + /// manual `impl` blocks of the `Default` trait of types with default + /// field values. + /// + /// ### Example + /// + /// ```rust,compile_fail + /// #![feature(default_field_values)] + /// struct Foo { + /// x: i32 = 101, + /// } + /// + /// #[deny(manual_default_for_type_with_default_fields)] + /// impl Default for Foo { + /// fn default() -> Foo { + /// Foo { x: 100 } + /// } + /// } + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Manually writing a `Default` implementation for a type that has + /// default field values runs the risk of diverging behavior between + /// `Type { .. }` and `::default()`, which would be a + /// foot-gun for users of that type that would expect these to be + /// equivalent. + pub MANUAL_DEFAULT_FOR_TYPE_WITH_DEFAULT_FIELDS, + Warn, + "detect `Default` impl on type with default field values that should be derived" +} + +declare_lint_pass!(DefaultCouldBeDerived => [DEFAULT_COULD_BE_DERIVED, MANUAL_DEFAULT_FOR_TYPE_WITH_DEFAULT_FIELDS]); + +impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx hir::Item<'tcx>) { + let hir = cx.tcx.hir(); + let hir::ItemKind::Impl(data) = item.kind else { return }; + let Some(trait_ref) = data.of_trait else { return }; + let Res::Def(DefKind::Trait, def_id) = trait_ref.path.res else { return }; + let Some(default_def_id) = cx.tcx.get_diagnostic_item(sym::Default) else { return }; + if Some(def_id) != Some(default_def_id) { + return; + } + if cx.tcx.has_attr(def_id, sym::automatically_derived) { + return; + } + let hir_self_ty = data.self_ty; + let hir::TyKind::Path(hir::QPath::Resolved(_, path)) = hir_self_ty.kind else { return }; + let Res::Def(def_kind, type_def_id) = path.res else { return }; + match hir.get_if_local(type_def_id) { + Some(hir::Node::Item(hir::Item { + kind: + hir::ItemKind::Struct(hir::VariantData::Struct { fields, recovered: _ }, _generics), + .. + })) if cx.tcx.features().default_field_values() => { + let fields_with_default_value: Vec<_> = + fields.iter().filter_map(|f| f.default).collect(); + let fields_with_default_impl: Vec<_> = fields + .iter() + .filter_map(|f| match (f.ty.kind, f.default) { + (hir::TyKind::Path(hir::QPath::Resolved(_, path)), None) + if let Some(def_id) = path.res.opt_def_id() + && let DefKind::Struct | DefKind::Enum = + cx.tcx.def_kind(def_id) => + { + let ty = cx.tcx.type_of(def_id).instantiate_identity(); + let mut count = 0; + cx.tcx.for_each_relevant_impl(default_def_id, ty, |_| count += 1); + if count > 0 { Some(f.ty.span) } else { None } + } + _ => None, + }) + .collect(); + // FIXME: look at the `Default::default()` implementation and call check_expr and + // check_const_expr on every field. If all are either of those, then we suggest + // adding a default field value for the const-able ones and deriving if the feature + // is enabled. + if !fields_with_default_value.is_empty() + && fields.len() + == fields_with_default_value.len() + fields_with_default_impl.len() + { + cx.tcx.node_span_lint( + MANUAL_DEFAULT_FOR_TYPE_WITH_DEFAULT_FIELDS, + item.hir_id(), + item.span, + |diag| { + diag.primary_message( + "manual `Default` impl for type with default field values", + ); + let msg = match ( + !fields_with_default_value.is_empty(), + !fields_with_default_impl.is_empty(), + ) { + (true, true) => "default values or a type that impls `Default`", + (true, false) => "default values", + (false, true) => "a type that impls `Default`", + (false, false) => unreachable!(), + }; + diag.span_label( + cx.tcx.def_span(type_def_id), + format!("all the fields in this struct have {msg}"), + ); + for anon in fields_with_default_value { + diag.span_label(anon.span, "default value"); + } + for field in fields_with_default_impl { + diag.span_label(field, "implements `Default`"); + } + diag.multipart_suggestion_verbose( + "to avoid divergence in behavior between `Struct { .. }` and \ + `::default()`, derive the `Default`", + vec![ + ( + cx.tcx.def_span(type_def_id).shrink_to_lo(), + "#[derive(Default)] ".to_string(), + ), + (item.span, String::new()), + ], + Applicability::MachineApplicable, + ); + }, + ); + return; + } + } + _ => {} + } + let generics = cx.tcx.generics_of(type_def_id); + if !generics.own_params.is_empty() && def_kind != DefKind::Enum { + // For enums, `#[derive(Default)]` forces you to select a unit variant to avoid + // "imperfect derives", unnecessary bounds on type parameters, so even if the enum has + // type parameters we can still lint on the manual impl if the return is a unit + // variant. + return; + } + // We have a manual `impl Default for Ty {}` item, where `Ty` has no type parameters. + + for assoc in data.items { + // We look for the user's `fn default() -> Self` associated fn of the `Default` impl. + + let hir::AssocItemKind::Fn { has_self: false } = assoc.kind else { continue }; + if assoc.ident.name != kw::Default { + continue; + } + let assoc = hir.impl_item(assoc.id); + let hir::ImplItemKind::Fn(_ty, body) = assoc.kind else { continue }; + let body = hir.body(body); + let hir::ExprKind::Block(hir::Block { stmts: [], expr: Some(expr), .. }, None) = + body.value.kind + else { + continue; + }; + + // We check `fn default()` body is a single ADT literal and all the fields are being + // set to something equivalent to the corresponding types' `Default::default()`. + match expr.kind { + hir::ExprKind::Path(hir::QPath::Resolved(_, path)) + if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Const), def_id) = + path.res => + { + // We have a unit variant as the default of an enum in a manual impl. + // + // enum Foo { + // Bar, + // } + // + // impl Default for Foo { + // fn default() -> Foo { + // Foo::Bar + // } + // } + // + // We suggest + // + // #[derive(Default)] enum Foo { + // #[default] Bar, + // } + cx.tcx.node_span_lint( + DEFAULT_COULD_BE_DERIVED, + item.hir_id(), + item.span, + |diag| { + diag.primary_message("`impl Default` that could be derived"); + diag.span_label( + expr.span, + "this enum variant has no fields, so it's trivially derivable", + ); + diag.multipart_suggestion_verbose( + "you don't need to manually `impl Default`, you can derive it", + vec![ + ( + cx.tcx.def_span(type_def_id).shrink_to_lo(), + "#[derive(Default)] ".to_string(), + ), + ( + cx.tcx.def_span(def_id).shrink_to_lo(), + "#[default] ".to_string(), + ), + (item.span, String::new()), + ], + Applicability::MachineApplicable, + ); + }, + ); + } + hir::ExprKind::Struct(_qpath, fields, tail) => { + // We have a struct literal + // + // struct Foo { + // field: Type, + // } + // + // impl Default for Foo { + // fn default() -> Foo { + // Foo { + // field: val, + // } + // } + // } + // + // We suggest #[derive(Default)] if + // - `val` is `Default::default()` + // - `val` matches the `Default::default()` body for that type + // - `val` is `0` + // - `val` is `false` + if let hir::StructTailExpr::Base(_) = tail { + // This is *very* niche. We'd only get here if someone wrote + // impl Default for Ty { + // fn default() -> Ty { + // Ty { ..something() } + // } + // } + // where `something()` would have to be a call or path. + return; + } + if fields.iter().all(|f| check_expr(cx, f.expr)) { + cx.tcx.node_span_lint( + DEFAULT_COULD_BE_DERIVED, + item.hir_id(), + item.span, + |diag| { + diag.primary_message("`impl Default` that could be derived"); + for (i, field) in fields.iter().enumerate() { + let msg = if i == fields.len() - 1 { + if fields.len() == 1 { + "this is the same value the expansion of \ + `#[derive(Default)]` would use" + } else { + "these are the same values the expansion of \ + `#[derive(Default)]` would use" + } + } else { + "" + }; + diag.span_label(field.expr.span, msg); + } + if let hir::StructTailExpr::DefaultFields(span) = tail { + diag.span_label( + span, + "all remaining fields will use the same default field \ + values that the `#[derive(Default)]` would use", + ); + } + diag.multipart_suggestion_verbose( + "you don't need to manually `impl Default`, you can derive it", + vec![ + ( + cx.tcx.def_span(type_def_id).shrink_to_lo(), + "#[derive(Default)] ".to_string(), + ), + (item.span, String::new()), + ], + Applicability::MachineApplicable, + ); + }, + ); + } + } + hir::ExprKind::Call(expr, args) => { + if let hir::ExprKind::Path(hir::QPath::Resolved(_, path)) = expr.kind + && let Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Fn), ctor_def_id) = + path.res + { + let type_def_id = cx.tcx.parent(ctor_def_id); // From Ctor to struct + if args.iter().all(|expr| check_expr(cx, expr)) { + // We have a struct literal + // + // struct Foo(Type); + // + // impl Default for Foo { + // fn default() -> Foo { + // Foo(val) + // } + // } + // + // We suggest #[derive(Default)] if + // - `val` is `Default::default()` + // - `val` matches the `Default::default()` body for that type + // - `val` is `0` + // - `val` is `false` + cx.tcx.node_span_lint( + DEFAULT_COULD_BE_DERIVED, + item.hir_id(), + item.span, + |diag| { + diag.primary_message("`impl Default` that could be derived"); + for (i, field) in args.iter().enumerate() { + let msg = if i == args.len() - 1 { + if args.len() == 1 { + "this is the same value the expansion of \ + `#[derive(Default)]` would use" + } else { + "these are the same values the expansion of \ + `#[derive(Default)]` would use" + } + } else { + "" + }; + diag.span_label(field.span, msg); + } + diag.multipart_suggestion_verbose( + "you don't need to manually `impl Default`, you can derive it", + vec![ + ( + cx.tcx.def_span(type_def_id).shrink_to_lo(), + "#[derive(Default)] ".to_string(), + ), + (item.span, String::new()), + ], + Applicability::MachineApplicable, + ); + }, + ); + } + } + } + hir::ExprKind::Path(hir::QPath::Resolved(_, path)) + if let Res::Def(DefKind::Ctor(CtorOf::Struct, CtorKind::Const), _) = + path.res => + { + // We have a struct literal + // + // struct Foo; + // + // impl Default for Foo { + // fn default() -> Foo { + // Foo + // } + // } + // + // We always suggest `#[derive(Default)]`. + cx.tcx.node_span_lint( + DEFAULT_COULD_BE_DERIVED, + item.hir_id(), + item.span, + |diag| { + diag.primary_message("`impl Default` that could be derived"); + diag.span_label( + expr.span, + "this type has no fields, so it's trivially derivable", + ); + diag.multipart_suggestion_verbose( + "you don't need to manually `impl Default`, you can derive it", + vec![ + ( + cx.tcx.def_span(type_def_id).shrink_to_lo(), + "#[derive(Default)] ".to_string(), + ), + (item.span, String::new()), + ], + Applicability::MachineApplicable, + ); + }, + ); + } + _ => {} + } + } + } +} + +fn check_path<'tcx>( + cx: &LateContext<'tcx>, + path: &hir::QPath<'_>, + hir_id: hir::HirId, + ty: Ty<'tcx>, +) -> bool { + let Some(default_def_id) = cx.tcx.get_diagnostic_item(sym::Default) else { + return false; + }; + let res = cx.qpath_res(&path, hir_id); + let Some(def_id) = res.opt_def_id() else { return false }; + if cx.tcx.is_diagnostic_item(sym::default_fn, def_id) { + // We have `field: Default::default(),`. This is what the derive would do already. + return true; + } + // For every `Default` impl for this type (there should be a single one), we see if it + // has a "canonical" `DefId` for a fn call with no arguments, or a path. If it does, we + // check that `DefId` with the `DefId` of this field's value if it is also a call/path. + // If there's a match, it means that the contents of that type's `Default` impl are the + // same to what the user wrote on *their* `Default` impl for this field. + let mut equivalents = vec![]; + cx.tcx.for_each_relevant_impl(default_def_id, ty, |impl_def_id| { + let equivalent = match impl_def_id.as_local() { + None => cx.tcx.get_default_impl_equivalent(impl_def_id), + Some(local) => { + let def_kind = cx.tcx.def_kind(impl_def_id); + cx.tcx.get_default_equivalent(def_kind, local) + } + }; + if let Some(did) = equivalent { + equivalents.push(did); + } + }); + for did in equivalents { + if did == def_id { + return true; + } + } + false +} + +fn check_expr(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> bool { + match expr.kind { + hir::ExprKind::Lit(spanned_lit) => match spanned_lit.node { + LitKind::Int(val, _) if val == 0 => true, // field: 0, + LitKind::Bool(false) => true, // field: false, + _ => false, + }, + hir::ExprKind::Call(hir::Expr { kind: hir::ExprKind::Path(path), hir_id, .. }, []) => { + // `field: foo(),` or `field: Ty::assoc(),` + let Some(ty) = cx + .tcx + .has_typeck_results(expr.hir_id.owner.def_id) + .then(|| cx.tcx.typeck(expr.hir_id.owner.def_id)) + .and_then(|typeck| typeck.expr_ty_adjusted_opt(expr)) + else { + return false; + }; + check_path(cx, &path, *hir_id, ty) + } + hir::ExprKind::Path(path) => { + // `field: qualified::Path,` or `field: ::Assoc,` + let Some(ty) = cx + .tcx + .has_typeck_results(expr.hir_id.owner.def_id) + .then(|| cx.tcx.typeck(expr.hir_id.owner.def_id)) + .and_then(|typeck| typeck.expr_ty_adjusted_opt(expr)) + else { + return false; + }; + check_path(cx, &path, expr.hir_id, ty) + } + _ => false, + } +} diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index a99c94592b302..b5e1db0283c9c 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -41,6 +41,7 @@ mod async_fn_in_trait; pub mod builtin; mod context; mod dangling; +mod default_could_be_derived; mod deref_into_dyn_supertrait; mod drop_forget_useless; mod early; @@ -85,6 +86,7 @@ use async_closures::AsyncClosureUsage; use async_fn_in_trait::AsyncFnInTrait; use builtin::*; use dangling::*; +use default_could_be_derived::DefaultCouldBeDerived; use deref_into_dyn_supertrait::*; use drop_forget_useless::*; use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums; @@ -189,6 +191,7 @@ late_lint_methods!( BuiltinCombinedModuleLateLintPass, [ ForLoopsOverFallibles: ForLoopsOverFallibles, + DefaultCouldBeDerived: DefaultCouldBeDerived, DerefIntoDynSupertrait: DerefIntoDynSupertrait, DropForgetUseless: DropForgetUseless, ImproperCTypesDeclarations: ImproperCTypesDeclarations, diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 9cad5d98562b6..47c1c2d5fe986 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -1024,6 +1024,7 @@ declare_lint! { "`if`, `match`, `while` and `return` do not need parentheses" } +#[derive(Default)] pub(crate) struct UnusedParens { with_self_ty_parens: bool, /// `1 as (i32) < 2` parses to ExprKind::Lt @@ -1031,12 +1032,6 @@ pub(crate) struct UnusedParens { parens_in_cast_in_lt: Vec, } -impl Default for UnusedParens { - fn default() -> Self { - Self { with_self_ty_parens: false, parens_in_cast_in_lt: Vec::new() } - } -} - impl_lint_pass!(UnusedParens => [UNUSED_PARENS]); impl UnusedDelimLint for UnusedParens { diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index 6eae4f9a8d6ea..7b8662b0adf11 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -1174,6 +1174,12 @@ impl<'a> CrateMetadataRef<'a> { self.root.tables.default_fields.get(self, id).map(|d| d.decode(self)) } + /// For a given `impl Default for Ty`, return the "equivalence" for it, namely the `DefId` of + /// a path or function with no arguments that get's called from `::default()`. + fn get_default_impl_equivalent(self, id: DefIndex) -> Option { + self.root.tables.default_impl_equivalent.get(self, id).map(|d| d.decode(self)) + } + fn get_trait_item_def_id(self, id: DefIndex) -> Option { self.root.tables.trait_item_def_id.get(self, id).map(|d| d.decode_from_cdata(self)) } diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 3077312ccf97e..c1d1e48c9c14b 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -423,6 +423,8 @@ provide! { tcx, def_id, other, cdata, syms } + get_default_impl_equivalent => { cdata.get_default_impl_equivalent(def_id.index) } + crate_extern_paths => { cdata.source().paths().cloned().collect() } expn_that_defined => { cdata.get_expn_that_defined(def_id.index, tcx.sess) } is_doc_hidden => { cdata.get_attr_flags(def_id.index).contains(AttrFlags::IS_DOC_HIDDEN) } diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 92c0e8c3a501d..b548c43671150 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -1568,6 +1568,10 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { let table = tcx.associated_types_for_impl_traits_in_associated_fn(def_id); record_defaulted_array!(self.tables.associated_types_for_impl_traits_in_associated_fn[def_id] <- table); } + + if let Some(path_def_id) = tcx.get_default_equivalent(def_kind, local_id) { + record!(self.tables.default_impl_equivalent[def_id] <- path_def_id); + } } for (def_id, impls) in &tcx.crate_inherent_impls(()).0.inherent_impls { diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index fa843a10adf12..9c70a5053f427 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -451,6 +451,7 @@ define_tables! { trait_item_def_id: Table, expn_that_defined: Table>, default_fields: Table>, + default_impl_equivalent: Table>, params_in_repr: Table>>, repr_options: Table>, // `def_keys` and `def_path_hashes` represent a lazy version of a diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 906a47713f4eb..91188d79e3e60 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1806,6 +1806,12 @@ rustc_queries! { desc { "looking up lifetime defaults for generic parameter `{}`", tcx.def_path_str(def_id) } separate_provide_extern } + + query get_default_impl_equivalent(def_id: DefId) -> Option { + desc { |tcx| "looking up the unit variant or fn call with no arguments equivalence for the `Default::default()` of `{}`", tcx.def_path_str(def_id) } + separate_provide_extern + } + query late_bound_vars_map(owner_id: hir::OwnerId) -> &'tcx SortedMap> { desc { |tcx| "looking up late bound vars inside `{}`", tcx.def_path_str(owner_id) } diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index f32656decd212..a83eccaf20fdc 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -29,7 +29,7 @@ use rustc_data_structures::unord::UnordSet; use rustc_errors::{ Applicability, Diag, DiagCtxtHandle, ErrorGuaranteed, LintDiagnostic, MultiSpan, }; -use rustc_hir::def::{CtorKind, DefKind}; +use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::def_id::{CrateNum, DefId, LOCAL_CRATE, LocalDefId}; use rustc_hir::definitions::Definitions; use rustc_hir::intravisit::Visitor; @@ -3205,6 +3205,58 @@ impl<'tcx> TyCtxt<'tcx> { } } + fn get_expr_equivalent(self, expr: &hir::Expr<'_>) -> Option { + match &expr.kind { + hir::ExprKind::Path(qpath) => { + if self.has_typeck_results(expr.hir_id.owner.def_id) { + let res = self.typeck(expr.hir_id.owner.def_id).qpath_res(&qpath, expr.hir_id); + return res.opt_def_id(); + } + } + hir::ExprKind::Call(hir::Expr { kind: hir::ExprKind::Path(qpath), hir_id, .. }, []) => { + if self.has_typeck_results(expr.hir_id.owner.def_id) { + let res = self.typeck(expr.hir_id.owner.def_id).qpath_res(&qpath, *hir_id); + return res.opt_def_id(); + } + } + _ => {} + } + None + } + + /// Given an `impl Default for Ty` item, return the `DefId` of the single path or fn call within + /// the ` Option { + if (DefKind::Impl { of_trait: true }) == def_kind + && let hir::Node::Item(item) = self.hir_node_by_def_id(local_id) + && let hir::ItemKind::Impl(data) = item.kind + && let Some(trait_ref) = data.of_trait + && let Res::Def(DefKind::Trait, trait_def_id) = trait_ref.path.res + && let Some(default_def_id) = self.get_diagnostic_item(sym::Default) + && Some(trait_def_id) == Some(default_def_id) + { + let hir = self.hir(); + for assoc in data.items { + let hir::AssocItemKind::Fn { has_self: false } = assoc.kind else { continue }; + if assoc.ident.name != kw::Default { + continue; + } + let assoc = hir.impl_item(assoc.id); + let hir::ImplItemKind::Fn(_ty, body) = assoc.kind else { continue }; + let body = hir.body(body); + let hir::ExprKind::Block(hir::Block { stmts: [], expr: Some(expr), .. }, None) = + body.value.kind + else { + continue; + }; + return self.get_expr_equivalent(&expr); + } + } + None + } + /// Whether this is a trait implementation that has `#[diagnostic::do_not_recommend]` pub fn do_not_recommend_impl(self, def_id: DefId) -> bool { self.get_diagnostic_attr(def_id, sym::do_not_recommend).is_some() diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 936c2ca87d69b..03de6811fe130 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -162,9 +162,10 @@ pub struct CoverageOptions { } /// Controls whether branch coverage or MC/DC coverage is enabled. -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, Default)] pub enum CoverageLevel { /// Instrument for coverage at the MIR block level. + #[default] Block, /// Also instrument branch points (includes block coverage). Branch, @@ -189,12 +190,6 @@ pub enum CoverageLevel { Mcdc, } -impl Default for CoverageLevel { - fn default() -> Self { - Self::Block - } -} - /// Settings for `-Z instrument-xray` flag. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Hash)] pub struct InstrumentXRay { diff --git a/library/core/src/ascii/ascii_char.rs b/library/core/src/ascii/ascii_char.rs index 48de4f17b1b3a..fcb4584f5dc7c 100644 --- a/library/core/src/ascii/ascii_char.rs +++ b/library/core/src/ascii/ascii_char.rs @@ -54,12 +54,13 @@ use crate::{assert_unsafe_precondition, fmt}; /// [chart]: https://www.unicode.org/charts/PDF/U0000.pdf /// [NIST FIPS 1-2]: https://nvlpubs.nist.gov/nistpubs/Legacy/FIPS/fipspub1-2-1977.pdf /// [NamesList]: https://www.unicode.org/Public/15.0.0/ucd/NamesList.txt -#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[derive(Copy, Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Default)] #[unstable(feature = "ascii_char", issue = "110998")] #[repr(u8)] pub enum AsciiChar { /// U+0000 (The default variant) #[unstable(feature = "ascii_char_variants", issue = "110998")] + #[default] Null = 0, /// U+0001 #[unstable(feature = "ascii_char_variants", issue = "110998")] diff --git a/library/core/src/default.rs b/library/core/src/default.rs index 4c30290ff263b..cdfbca9f897c1 100644 --- a/library/core/src/default.rs +++ b/library/core/src/default.rs @@ -2,8 +2,6 @@ #![stable(feature = "rust1", since = "1.0.0")] -use crate::ascii::Char as AsciiChar; - /// A trait for giving a type a useful default value. /// /// Sometimes, you want to fall back to some kind of default value, and @@ -79,6 +77,7 @@ use crate::ascii::Char as AsciiChar; /// your type that should be the default: /// /// ``` +/// # #![cfg_attr(not(bootstrap), allow(default_could_be_derived))] /// # #![allow(dead_code)] /// enum Kind { /// A, @@ -123,6 +122,7 @@ pub trait Default: Sized { /// Making your own: /// /// ``` + /// # #![cfg_attr(not(bootstrap), allow(default_could_be_derived))] /// # #[allow(dead_code)] /// enum Kind { /// A, @@ -163,7 +163,6 @@ macro_rules! default_impl { default_impl! { (), (), "Returns the default value of `()`" } default_impl! { bool, false, "Returns the default value of `false`" } default_impl! { char, '\x00', "Returns the default value of `\\x00`" } -default_impl! { AsciiChar, AsciiChar::Null, "Returns the default value of `Null`" } default_impl! { usize, 0, "Returns the default value of `0`" } default_impl! { u8, 0, "Returns the default value of `0`" } diff --git a/library/core/src/option.rs b/library/core/src/option.rs index f4ac7af63961b..6f06d327734d9 100644 --- a/library/core/src/option.rs +++ b/library/core/src/option.rs @@ -564,7 +564,7 @@ use crate::{cmp, convert, hint, mem, slice}; /// The `Option` type. See [the module level documentation](self) for more. #[doc(search_unbox)] -#[derive(Copy, Eq, Debug, Hash)] +#[derive(Copy, Eq, Debug, Hash, Default)] #[rustc_diagnostic_item = "Option"] #[lang = "Option"] #[stable(feature = "rust1", since = "1.0.0")] @@ -573,6 +573,7 @@ pub enum Option { /// No value. #[lang = "None"] #[stable(feature = "rust1", since = "1.0.0")] + #[default] None, /// Some value of type `T`. #[lang = "Some"] @@ -2044,22 +2045,6 @@ where } } -#[stable(feature = "rust1", since = "1.0.0")] -impl Default for Option { - /// Returns [`None`][Option::None]. - /// - /// # Examples - /// - /// ``` - /// let opt: Option = Option::default(); - /// assert!(opt.is_none()); - /// ``` - #[inline] - fn default() -> Option { - None - } -} - #[stable(feature = "rust1", since = "1.0.0")] impl IntoIterator for Option { type Item = T; diff --git a/library/core/tests/hash/mod.rs b/library/core/tests/hash/mod.rs index bf91e9e5df0e2..9f14995f73fe2 100644 --- a/library/core/tests/hash/mod.rs +++ b/library/core/tests/hash/mod.rs @@ -4,16 +4,11 @@ use std::hash::{BuildHasher, Hash, Hasher}; use std::ptr; use std::rc::Rc; +#[derive(Default)] struct MyHasher { hash: u64, } -impl Default for MyHasher { - fn default() -> MyHasher { - MyHasher { hash: 0 } - } -} - impl Hasher for MyHasher { fn write(&mut self, buf: &[u8]) { for byte in buf { @@ -107,6 +102,8 @@ fn test_writer_hasher() { struct Custom { hash: u64, } + +#[derive(Default)] struct CustomHasher { output: u64, } @@ -123,12 +120,6 @@ impl Hasher for CustomHasher { } } -impl Default for CustomHasher { - fn default() -> CustomHasher { - CustomHasher { output: 0 } - } -} - impl Hash for Custom { fn hash(&self, state: &mut H) { state.write_u64(self.hash); diff --git a/library/proc_macro/src/bridge/fxhash.rs b/library/proc_macro/src/bridge/fxhash.rs index 74a41451825ff..3345e099a3724 100644 --- a/library/proc_macro/src/bridge/fxhash.rs +++ b/library/proc_macro/src/bridge/fxhash.rs @@ -22,6 +22,7 @@ pub type FxHashMap = HashMap>; /// out-performs an FNV-based hash within rustc itself -- the collision rate is /// similar or slightly worse than FNV, but the speed of the hash function /// itself is much higher because it works on up to 8 bytes at a time. +#[derive(Default)] pub struct FxHasher { hash: usize, } @@ -31,13 +32,6 @@ const K: usize = 0x9e3779b9; #[cfg(target_pointer_width = "64")] const K: usize = 0x517cc1b727220a95; -impl Default for FxHasher { - #[inline] - fn default() -> FxHasher { - FxHasher { hash: 0 } - } -} - impl FxHasher { #[inline] fn add_to_hash(&mut self, i: usize) { diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index dca5ccca0c404..e7ce5bc61401d 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -81,7 +81,9 @@ extern "C" fn __rust_foreign_exception() -> ! { rtabort!("Rust cannot catch foreign exceptions"); } +#[derive(Default)] enum Hook { + #[default] Default, Custom(Box) + 'static + Sync + Send>), } @@ -96,13 +98,6 @@ impl Hook { } } -impl Default for Hook { - #[inline] - fn default() -> Hook { - Hook::Default - } -} - static HOOK: RwLock = RwLock::new(Hook::Default); /// Registers a custom panic hook, replacing the previously registered hook. diff --git a/library/std/src/sys_common/process.rs b/library/std/src/sys_common/process.rs index 5333ee146f7d6..9f61d69d85875 100644 --- a/library/std/src/sys_common/process.rs +++ b/library/std/src/sys_common/process.rs @@ -8,19 +8,13 @@ use crate::sys::process::{EnvKey, ExitStatus, Process, StdioPipes}; use crate::{env, fmt, io}; // Stores a set of changes to an environment -#[derive(Clone)] +#[derive(Clone, Default)] pub struct CommandEnv { clear: bool, saw_path: bool, vars: BTreeMap>, } -impl Default for CommandEnv { - fn default() -> Self { - CommandEnv { clear: false, saw_path: false, vars: Default::default() } - } -} - impl fmt::Debug for CommandEnv { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let mut debug_command_env = f.debug_struct("CommandEnv"); diff --git a/src/tools/clippy/tests/ui/derivable_impls.fixed b/src/tools/clippy/tests/ui/derivable_impls.fixed index c85f384fd6eb9..af8540038a007 100644 --- a/src/tools/clippy/tests/ui/derivable_impls.fixed +++ b/src/tools/clippy/tests/ui/derivable_impls.fixed @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, default_could_be_derived)] use std::collections::HashMap; diff --git a/src/tools/clippy/tests/ui/derivable_impls.rs b/src/tools/clippy/tests/ui/derivable_impls.rs index 21d73ba8b7778..81d54ab0357be 100644 --- a/src/tools/clippy/tests/ui/derivable_impls.rs +++ b/src/tools/clippy/tests/ui/derivable_impls.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] +#![allow(dead_code, default_could_be_derived)] use std::collections::HashMap; diff --git a/src/tools/clippy/tests/ui/new_without_default.fixed b/src/tools/clippy/tests/ui/new_without_default.fixed index 5a6a92394a7fd..a47e20858dcfb 100644 --- a/src/tools/clippy/tests/ui/new_without_default.fixed +++ b/src/tools/clippy/tests/ui/new_without_default.fixed @@ -38,7 +38,7 @@ impl Bar { } } -pub struct Ok; +#[derive(Default)] pub struct Ok; impl Ok { pub fn new() -> Self { @@ -46,11 +46,6 @@ impl Ok { } } -impl Default for Ok { - fn default() -> Self { - Ok - } -} pub struct Params; diff --git a/src/tools/clippy/tests/ui/new_without_default.stderr b/src/tools/clippy/tests/ui/new_without_default.stderr index 57bf4bd847cce..0c4a5f4f5df71 100644 --- a/src/tools/clippy/tests/ui/new_without_default.stderr +++ b/src/tools/clippy/tests/ui/new_without_default.stderr @@ -166,5 +166,22 @@ LL + } LL + } | -error: aborting due to 9 previous errors +error: `impl Default` that could be derived + --> tests/ui/new_without_default.rs:37:1 + | +LL | / impl Default for Ok { +LL | | fn default() -> Self { +LL | | Ok + | | -- this type has no fields, so it's trivially derivable +LL | | } +LL | | } + | |_^ + | + = note: `#[deny(default_could_be_derived)]` on by default +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] pub struct Ok; + | + +error: aborting due to 10 previous errors diff --git a/src/tools/clippy/tests/ui/or_fun_call.fixed b/src/tools/clippy/tests/ui/or_fun_call.fixed index 625d654dd3964..b1ac63b594efc 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.fixed +++ b/src/tools/clippy/tests/ui/or_fun_call.fixed @@ -21,18 +21,14 @@ fn or_fun_call() { } } - struct FakeDefault; + #[derive(Default)] struct FakeDefault; impl FakeDefault { fn default() -> Self { FakeDefault } } - impl Default for FakeDefault { - fn default() -> Self { - FakeDefault - } - } + enum Enum { A(i32), @@ -268,18 +264,14 @@ mod lazy { } } - struct FakeDefault; + #[derive(Default)] struct FakeDefault; impl FakeDefault { fn default() -> Self { FakeDefault } } - impl Default for FakeDefault { - fn default() -> Self { - FakeDefault - } - } + let with_new = Some(vec![1]); with_new.unwrap_or_default(); diff --git a/src/tools/clippy/tests/ui/or_fun_call.stderr b/src/tools/clippy/tests/ui/or_fun_call.stderr index 9f90a830a2114..44f96a1592316 100644 --- a/src/tools/clippy/tests/ui/or_fun_call.stderr +++ b/src/tools/clippy/tests/ui/or_fun_call.stderr @@ -244,5 +244,46 @@ error: function call inside of `unwrap_or` LL | let _ = opt_foo.unwrap_or(Foo { val: String::default() }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `unwrap_or_else(|| Foo { val: String::default() })` -error: aborting due to 38 previous errors +error: `impl Default` that could be derived + --> tests/ui/or_fun_call.rs:31:5 + | +LL | / impl Default for FakeDefault { +LL | | fn default() -> Self { +LL | | FakeDefault + | | ----------- this type has no fields, so it's trivially derivable +LL | | } +LL | | } + | |_____^ + | + = note: `#[deny(default_could_be_derived)]` on by default +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct FakeDefault; +LL | impl FakeDefault { +... +LL | +LL ~ + | + +error: `impl Default` that could be derived + --> tests/ui/or_fun_call.rs:278:9 + | +LL | / impl Default for FakeDefault { +LL | | fn default() -> Self { +LL | | FakeDefault + | | ----------- this type has no fields, so it's trivially derivable +LL | | } +LL | | } + | |_________^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct FakeDefault; +LL | impl FakeDefault { +... +LL | +LL ~ + | + +error: aborting due to 40 previous errors diff --git a/src/tools/clippy/tests/ui/unwrap_or_else_default.fixed b/src/tools/clippy/tests/ui/unwrap_or_else_default.fixed index 8d5d34175c525..fb88bd4e7952c 100644 --- a/src/tools/clippy/tests/ui/unwrap_or_else_default.fixed +++ b/src/tools/clippy/tests/ui/unwrap_or_else_default.fixed @@ -17,7 +17,7 @@ fn unwrap_or_else_default() { } } - struct HasDefaultAndDuplicate; + #[derive(Default)] struct HasDefaultAndDuplicate; impl HasDefaultAndDuplicate { fn default() -> Self { @@ -25,11 +25,7 @@ fn unwrap_or_else_default() { } } - impl Default for HasDefaultAndDuplicate { - fn default() -> Self { - HasDefaultAndDuplicate - } - } + enum Enum { A(), diff --git a/src/tools/clippy/tests/ui/unwrap_or_else_default.stderr b/src/tools/clippy/tests/ui/unwrap_or_else_default.stderr index e4b4a0a1f6aa7..6e86aac95bbbd 100644 --- a/src/tools/clippy/tests/ui/unwrap_or_else_default.stderr +++ b/src/tools/clippy/tests/ui/unwrap_or_else_default.stderr @@ -97,5 +97,26 @@ error: use of `or_insert_with` to construct default value LL | let _ = inner_map.entry(0).or_insert_with(Default::default); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `or_default()` -error: aborting due to 16 previous errors +error: `impl Default` that could be derived + --> tests/ui/unwrap_or_else_default.rs:28:5 + | +LL | / impl Default for HasDefaultAndDuplicate { +LL | | fn default() -> Self { +LL | | HasDefaultAndDuplicate + | | ---------------------- this type has no fields, so it's trivially derivable +LL | | } +LL | | } + | |_____^ + | + = note: `#[deny(default_could_be_derived)]` on by default +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct HasDefaultAndDuplicate; +LL | +... +LL | +LL ~ + | + +error: aborting due to 17 previous errors diff --git a/src/tools/rustfmt/src/config/options.rs b/src/tools/rustfmt/src/config/options.rs index c526b5c678fd7..7ca0f62ff8ea2 100644 --- a/src/tools/rustfmt/src/config/options.rs +++ b/src/tools/rustfmt/src/config/options.rs @@ -155,9 +155,11 @@ pub enum ReportTactic { /// What Rustfmt should emit. Mostly corresponds to the `--emit` command line /// option. +#[derive(Default)] #[config_type] pub enum EmitMode { /// Emits to files. + #[default] Files, /// Writes the output to stdout. Stdout, @@ -310,12 +312,6 @@ impl ::std::str::FromStr for WidthHeuristics { } } -impl Default for EmitMode { - fn default() -> EmitMode { - EmitMode::Files - } -} - /// A set of directories, files and modules that rustfmt should ignore. #[derive(Default, Clone, Debug, PartialEq)] pub struct IgnoreList { @@ -425,10 +421,12 @@ pub trait CliOptions { } /// The edition of the syntax and semantics of code (RFC 2052). +#[derive(Default)] #[config_type] pub enum Edition { #[value = "2015"] #[doc_hint = "2015"] + #[default] /// Edition 2015. Edition2015, #[value = "2018"] @@ -445,12 +443,6 @@ pub enum Edition { Edition2024, } -impl Default for Edition { - fn default() -> Edition { - Edition::Edition2015 - } -} - impl From for rustc_span::edition::Edition { fn from(edition: Edition) -> Self { match edition { diff --git a/tests/ui/structs/manual-default-impl-could-be-derived.fixed b/tests/ui/structs/manual-default-impl-could-be-derived.fixed new file mode 100644 index 0000000000000..acfd8e704a827 --- /dev/null +++ b/tests/ui/structs/manual-default-impl-could-be-derived.fixed @@ -0,0 +1,118 @@ +// Warn when we encounter a manual `Default` impl that could be derived. +//@ run-rustfix +#![allow(dead_code)] +#![deny(default_could_be_derived, manual_default_for_type_with_default_fields)] +#![feature(default_field_values)] + +#[derive(Debug)] +#[derive(Default)] struct A; + + +#[derive(Debug)] +#[derive(Default)] struct B(Option); + + +#[derive(Debug)] +#[derive(Default)] struct C(Option); + + + +// Explicit check against numeric literals and `Default::default()` calls. +#[derive(Default)] struct D { + x: Option, + y: i32, +} + + +// Explicit check against `None` literal, in the same way that we check against numeric literals. +#[derive(Debug)] +#[derive(Default)] struct E { + x: Option, +} + + +// Detection of unit variant ctors that could have been marked `#[default]`. +#[derive(Default)] enum F { + #[default] Unit, + Tuple(T), +} + + +// Comparison of `impl` *fields* with their `Default::default()` bodies. +#[derive(Default)] struct G { + f: F, +} + + +// Always lint against manual `Default` impl if all fields are defaulted. +#[derive(PartialEq, Debug)] +#[derive(Default)] struct H { + x: i32 = 101, +} + + +// Always lint against manual `Default` impl if all fields are defaulted. +#[derive(PartialEq, Debug)] +#[derive(Default)] struct I { + x: i32 = 101, + y: Option, +} + + +// Account for fn calls that are not assoc fns, still check that they match between what the user +// wrote and the Default impl. +#[derive(Default)] struct J { + x: K, +} + + +struct K; + +impl Default for K { // *could* be derived, but it isn't lintable because of the `foo()` call + fn default() -> Self { + foo() + } +} + +fn foo() -> K { + K +} + +// Verify that cross-crate tracking of "equivalences" keeps working. +#[derive(Default)] struct L { + x: Vec, +} + + +// Account for `const`s +#[derive(Default)] struct M { + x: N, +} + + +struct N; + +impl Default for N { // ok + fn default() -> Self { + N_CONST + } +} + +const N_CONST: N = N; + +fn main() { + let _ = A::default(); + let _ = B::default(); + let _ = C::default(); + let _ = D::default(); + let _ = E::default(); + let _ = F::::default(); + let _ = G::default(); + assert_eq!(H::default(), H { .. }); + let _ = I::default(); + let _ = J::default(); + let _ = K::default(); + let _ = L::default(); + let _ = M::default(); + let _ = N::default(); +} diff --git a/tests/ui/structs/manual-default-impl-could-be-derived.rs b/tests/ui/structs/manual-default-impl-could-be-derived.rs new file mode 100644 index 0000000000000..2019b972c3282 --- /dev/null +++ b/tests/ui/structs/manual-default-impl-could-be-derived.rs @@ -0,0 +1,190 @@ +// Warn when we encounter a manual `Default` impl that could be derived. +//@ run-rustfix +#![allow(dead_code)] +#![deny(default_could_be_derived, manual_default_for_type_with_default_fields)] +#![feature(default_field_values)] + +#[derive(Debug)] +struct A; + +impl Default for A { //~ ERROR default_could_be_derived + fn default() -> Self { A } +} + +#[derive(Debug)] +struct B(Option); + +impl Default for B { //~ ERROR default_could_be_derived + fn default() -> Self { B(Default::default()) } +} + +#[derive(Debug)] +struct C(Option); + +impl Default for C { //~ ERROR default_could_be_derived + fn default() -> Self { C(None) } +} + + +// Explicit check against numeric literals and `Default::default()` calls. +struct D { + x: Option, + y: i32, +} + +impl Default for D { //~ ERROR default_could_be_derived + fn default() -> Self { + D { + x: Default::default(), + y: 0, + } + } +} + +// Explicit check against `None` literal, in the same way that we check against numeric literals. +#[derive(Debug)] +struct E { + x: Option, +} + +impl Default for E { //~ ERROR default_could_be_derived + fn default() -> Self { + E { + x: None, + } + } +} + +// Detection of unit variant ctors that could have been marked `#[default]`. +enum F { + Unit, + Tuple(T), +} + +impl Default for F { //~ ERROR default_could_be_derived + fn default() -> Self { + F::Unit + } +} + +// Comparison of `impl` *fields* with their `Default::default()` bodies. +struct G { + f: F, +} + +impl Default for G { //~ ERROR default_could_be_derived + fn default() -> Self { + G { + f: F::Unit, + } + } +} + +// Always lint against manual `Default` impl if all fields are defaulted. +#[derive(PartialEq, Debug)] +struct H { + x: i32 = 101, +} + +impl Default for H { //~ ERROR [manual_default_for_type_with_default_fields] + fn default() -> Self { + H { + x: 1, + } + } +} + +// Always lint against manual `Default` impl if all fields are defaulted. +#[derive(PartialEq, Debug)] +struct I { + x: i32 = 101, + y: Option, +} + +impl Default for I { //~ ERROR [manual_default_for_type_with_default_fields] + fn default() -> Self { + I { + x: 1, + y: None, + } + } +} + +// Account for fn calls that are not assoc fns, still check that they match between what the user +// wrote and the Default impl. +struct J { + x: K, +} + +impl Default for J { //~ ERROR default_could_be_derived + fn default() -> Self { + J { + x: foo(), // fn call that isn't an assoc fn + } + } +} + +struct K; + +impl Default for K { // *could* be derived, but it isn't lintable because of the `foo()` call + fn default() -> Self { + foo() + } +} + +fn foo() -> K { + K +} + +// Verify that cross-crate tracking of "equivalences" keeps working. +struct L { + x: Vec, +} + +impl Default for L { //~ ERROR default_could_be_derived + fn default() -> Self { + L { + x: Vec::new(), // `::default()` just calls `Vec::new()` + } + } +} + +// Account for `const`s +struct M { + x: N, +} + +impl Default for M { //~ ERROR default_could_be_derived + fn default() -> Self { + M { + x: N_CONST, + } + } +} + +struct N; + +impl Default for N { // ok + fn default() -> Self { + N_CONST + } +} + +const N_CONST: N = N; + +fn main() { + let _ = A::default(); + let _ = B::default(); + let _ = C::default(); + let _ = D::default(); + let _ = E::default(); + let _ = F::::default(); + let _ = G::default(); + assert_eq!(H::default(), H { .. }); + let _ = I::default(); + let _ = J::default(); + let _ = K::default(); + let _ = L::default(); + let _ = M::default(); + let _ = N::default(); +} diff --git a/tests/ui/structs/manual-default-impl-could-be-derived.stderr b/tests/ui/structs/manual-default-impl-could-be-derived.stderr new file mode 100644 index 0000000000000..e5c92baf031e9 --- /dev/null +++ b/tests/ui/structs/manual-default-impl-could-be-derived.stderr @@ -0,0 +1,222 @@ +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:10:1 + | +LL | / impl Default for A { +LL | | fn default() -> Self { A } + | | - this type has no fields, so it's trivially derivable +LL | | } + | |_^ + | +note: the lint level is defined here + --> $DIR/manual-default-impl-could-be-derived.rs:4:9 + | +LL | #![deny(default_could_be_derived, manual_default_for_type_with_default_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ +help: you don't need to manually `impl Default`, you can derive it + | +LL - struct A; +LL + #[derive(Default)] struct A; + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:17:1 + | +LL | / impl Default for B { +LL | | fn default() -> Self { B(Default::default()) } + | | ------------------ this is the same value the expansion of `#[derive(Default)]` would use +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL - struct B(Option); +LL + #[derive(Default)] struct B(Option); + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:24:1 + | +LL | / impl Default for C { +LL | | fn default() -> Self { C(None) } + | | ---- this is the same value the expansion of `#[derive(Default)]` would use +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL - struct C(Option); +LL + #[derive(Default)] struct C(Option); + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:35:1 + | +LL | / impl Default for D { +LL | | fn default() -> Self { +LL | | D { +LL | | x: Default::default(), + | | ------------------ +LL | | y: 0, + | | - these are the same values the expansion of `#[derive(Default)]` would use +... | +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct D { + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:50:1 + | +LL | / impl Default for E { +LL | | fn default() -> Self { +LL | | E { +LL | | x: None, + | | ---- this is the same value the expansion of `#[derive(Default)]` would use +... | +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct E { + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:64:1 + | +LL | / impl Default for F { +LL | | fn default() -> Self { +LL | | F::Unit + | | ------- this enum variant has no fields, so it's trivially derivable +LL | | } +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] enum F { +LL ~ #[default] Unit, + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:75:1 + | +LL | / impl Default for G { +LL | | fn default() -> Self { +LL | | G { +LL | | f: F::Unit, + | | ------- this is the same value the expansion of `#[derive(Default)]` would use +... | +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct G { + | + +error: manual `Default` impl for type with default field values + --> $DIR/manual-default-impl-could-be-derived.rs:89:1 + | +LL | struct H { + | -------- all the fields in this struct have default values +LL | x: i32 = 101, + | --- default value +... +LL | / impl Default for H { +LL | | fn default() -> Self { +LL | | H { +LL | | x: 1, +... | +LL | | } + | |_^ + | +note: the lint level is defined here + --> $DIR/manual-default-impl-could-be-derived.rs:4:35 + | +LL | #![deny(default_could_be_derived, manual_default_for_type_with_default_fields)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: to avoid divergence in behavior between `Struct { .. }` and `::default()`, derive the `Default` + | +LL ~ #[derive(Default)] struct H { + | + +error: manual `Default` impl for type with default field values + --> $DIR/manual-default-impl-could-be-derived.rs:104:1 + | +LL | struct I { + | -------- all the fields in this struct have default values or a type that impls `Default` +LL | x: i32 = 101, + | --- default value +LL | y: Option, + | ----------- implements `Default` +... +LL | / impl Default for I { +LL | | fn default() -> Self { +LL | | I { +LL | | x: 1, +... | +LL | | } + | |_^ + | +help: to avoid divergence in behavior between `Struct { .. }` and `::default()`, derive the `Default` + | +LL ~ #[derive(Default)] struct I { + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:119:1 + | +LL | / impl Default for J { +LL | | fn default() -> Self { +LL | | J { +LL | | x: foo(), // fn call that isn't an assoc fn + | | ----- this is the same value the expansion of `#[derive(Default)]` would use +... | +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct J { + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:144:1 + | +LL | / impl Default for L { +LL | | fn default() -> Self { +LL | | L { +LL | | x: Vec::new(), // `::default()` just calls `Vec::new()` + | | ---------- this is the same value the expansion of `#[derive(Default)]` would use +... | +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct L { + | + +error: `impl Default` that could be derived + --> $DIR/manual-default-impl-could-be-derived.rs:157:1 + | +LL | / impl Default for M { +LL | | fn default() -> Self { +LL | | M { +LL | | x: N_CONST, + | | ------- this is the same value the expansion of `#[derive(Default)]` would use +... | +LL | | } + | |_^ + | +help: you don't need to manually `impl Default`, you can derive it + | +LL ~ #[derive(Default)] struct M { + | + +error: aborting due to 12 previous errors +