nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-05-14 02:49:40 +00:00
Code Issues Packages Projects Releases Wiki Activity

Rollup merge of #134737 - estebank:deive-lint-default-fields-base, r=compiler-errors

Browse Source 
Implement `default_overrides_default_fields` lint

Detect when a manual `Default` implementation isn't using the existing default field values and suggest using `..` instead:

```
error: `Default` impl doesn't use the declared default field values
  --> $DIR/manual-default-impl-could-be-derived.rs:14:1
   |
LL | / impl Default for A {
LL | |     fn default() -> Self {
LL | |         A {
LL | |             y: 0,
   | |                - this field has a default value
...  |
LL | | }
   | |_^
   |
   = help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
note: the lint level is defined here
  --> $DIR/manual-default-impl-could-be-derived.rs:5:9
   |
LL | #![deny(default_overrides_default_fields)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

r? `@compiler-errors`

This is a simpler version of #134441, detecting the simpler case when a field with a default should have not been specified in the manual `Default::default()`, instead using `..` for it. It doesn't provide any suggestions, nor the checks for "equivalences" nor whether the value used in the imp being used would be suitable as a default field value.
This commit is contained in:
Stuart Cook 2024-12-28 16:50:36 +11:00 committed by GitHub
commit 3e3db73c9b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 526 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

185
compiler/rustc_lint/src/default_could_be_derived.rs Normal file
View File

@ -0,0 +1,185 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::Diag;
use rustc_hir as hir;
use rustc_middle::ty;
use rustc_session::{declare_lint, impl_lint_pass};
use rustc_span::Symbol;
use rustc_span::symbol::sym;
use crate::{LateContext, LateLintPass};
declare_lint! {
/// The `default_overrides_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,
/// y: NonDefault,
/// }
///
/// struct NonDefault;
///
/// #[deny(default_overrides_default_fields)]
/// impl Default for Foo {
/// fn default() -> Foo {
/// Foo { x: 100, y: NonDefault }
/// }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Manually writing a `Default` implementation for a type that has
/// default field values runs the risk of diverging behavior between
/// `Type { .. }` and `<Type as Default>::default()`, which would be a
/// foot-gun for users of that type that would expect these to be
/// equivalent. If `Default` can't be derived due to some fields not
/// having a `Default` implementation, we encourage the use of `..` for
/// the fields that do have a default field value.
pub DEFAULT_OVERRIDES_DEFAULT_FIELDS,
Deny,
"detect `Default` impl that should use the type's default field values",
@feature_gate = default_field_values;
}
#[derive(Default)]
pub(crate) struct DefaultCouldBeDerived;
impl_lint_pass!(DefaultCouldBeDerived => [DEFAULT_OVERRIDES_DEFAULT_FIELDS]);
impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived {
fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) {
// Look for manual implementations of `Default`.
let Some(default_def_id) = cx.tcx.get_diagnostic_item(sym::Default) else { return };
let hir::ImplItemKind::Fn(_sig, body_id) = impl_item.kind else { return };
let assoc = cx.tcx.associated_item(impl_item.owner_id);
let parent = assoc.container_id(cx.tcx);
if cx.tcx.has_attr(parent, sym::automatically_derived) {
// We don't care about what `#[derive(Default)]` produces in this lint.
return;
}
let Some(trait_ref) = cx.tcx.impl_trait_ref(parent) else { return };
let trait_ref = trait_ref.instantiate_identity();
if trait_ref.def_id != default_def_id {
return;
}
let ty = trait_ref.self_ty();
let ty::Adt(def, _) = ty.kind() else { return };
// We now know we have a manually written definition of a `<Type as Default>::default()`.
let hir = cx.tcx.hir();
let type_def_id = def.did();
let body = hir.body(body_id);
// FIXME: evaluate bodies with statements and evaluate bindings to see if they would be
// derivable.
let hir::ExprKind::Block(hir::Block { stmts: _, expr: Some(expr), .. }, None) =
body.value.kind
else {
return;
};
// Keep a mapping of field name to `hir::FieldDef` for every field in the type. We'll use
// these to check for things like checking whether it has a default or using its span for
// suggestions.
let orig_fields = match hir.get_if_local(type_def_id) {
Some(hir::Node::Item(hir::Item {
kind:
hir::ItemKind::Struct(hir::VariantData::Struct { fields, recovered: _ }, _generics),
..
})) => fields.iter().map(|f| (f.ident.name, f)).collect::<FxHashMap<_, _>>(),
_ => return,
};
// We check `fn default()` body is a single ADT literal and get all the fields that are
// being set.
let hir::ExprKind::Struct(_qpath, fields, tail) = expr.kind else { return };
// We have a struct literal
//
// struct Foo {
// field: Type,
// }
//
// impl Default for Foo {
// fn default() -> Foo {
// Foo {
// field: val,
// }
// }
// }
//
// We would suggest `#[derive(Default)]` if `field` has a default value, regardless of what
// it is; we don't want to encourage divergent behavior between `Default::default()` and
// `..`.
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.
// We have nothing meaninful to do with this.
return;
}
// At least one of the fields with a default value have been overriden in
// the `Default` implementation. We suggest removing it and relying on `..`
// instead.
let any_default_field_given =
fields.iter().any(|f| orig_fields.get(&f.ident.name).and_then(|f| f.default).is_some());
if !any_default_field_given {
// None of the default fields were actually provided explicitly, so the manual impl
// doesn't override them (the user used `..`), so there's no risk of divergent behavior.
return;
}
let Some(local) = parent.as_local() else { return };
let hir_id = cx.tcx.local_def_id_to_hir_id(local);
let hir::Node::Item(item) = cx.tcx.hir_node(hir_id) else { return };
cx.tcx.node_span_lint(DEFAULT_OVERRIDES_DEFAULT_FIELDS, hir_id, item.span, |diag| {
mk_lint(diag, orig_fields, fields);
});
}
}
fn mk_lint(
diag: &mut Diag<'_, ()>,
orig_fields: FxHashMap<Symbol, &hir::FieldDef<'_>>,
fields: &[hir::ExprField<'_>],
) {
diag.primary_message("`Default` impl doesn't use the declared default field values");
// For each field in the struct expression
// - if the field in the type has a default value, it should be removed
// - elif the field is an expression that could be a default value, it should be used as the
// field's default value (FIXME: not done).
// - else, we wouldn't touch this field, it would remain in the manual impl
let mut removed_all_fields = true;
for field in fields {
if orig_fields.get(&field.ident.name).and_then(|f| f.default).is_some() {
diag.span_label(field.expr.span, "this field has a default value");
} else {
removed_all_fields = false;
}
}
diag.help(if removed_all_fields {
"to avoid divergence in behavior between `Struct { .. }` and \
`<Struct as Default>::default()`, derive the `Default`"
} else {
"use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them \
diverging over time"
});
}

3
compiler/rustc_lint/src/lib.rs
View File

@ -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::default(),
DerefIntoDynSupertrait: DerefIntoDynSupertrait,
DropForgetUseless: DropForgetUseless,
ImproperCTypesDeclarations: ImproperCTypesDeclarations,

194
tests/ui/structs/manual-default-impl-could-be-derived.rs Normal file
View File

@ -0,0 +1,194 @@
// Warn when we encounter a manual `Default` impl that could be derived.
// Restricted only to types using `default_field_values`.
#![feature(default_field_values)]
#![allow(dead_code)]
#![deny(default_overrides_default_fields)]
struct S(i32);
fn s() -> S { S(1) }
struct A {
x: S,
y: i32 = 1,
}
impl Default for A { //~ ERROR default_overrides_default_fields
fn default() -> Self {
A {
y: 0,
x: s(),
}
}
}
struct B {
x: S = S(3),
y: i32 = 1,
}
impl Default for B { //~ ERROR default_overrides_default_fields
fn default() -> Self {
B {
x: s(),
y: 0,
}
}
}
struct C {
x: S,
y: i32 = 1,
z: i32 = 1,
}
impl Default for C { //~ ERROR default_overrides_default_fields
fn default() -> Self {
C {
x: s(),
y: 0,
..
}
}
}
struct D {
x: S,
y: i32 = 1,
z: i32 = 1,
}
impl Default for D { //~ ERROR default_overrides_default_fields
fn default() -> Self {
D {
y: 0,
x: s(),
..
}
}
}
struct E {
x: S,
y: i32 = 1,
z: i32 = 1,
}
impl Default for E { //~ ERROR default_overrides_default_fields
fn default() -> Self {
E {
y: 0,
z: 0,
x: s(),
}
}
}
// Let's ensure that the span for `x` and the span for `y` don't overlap when suggesting their
// removal in favor of their default field values.
struct E2 {
x: S,
y: i32 = 1,
z: i32 = 1,
}
impl Default for E2 { //~ ERROR default_overrides_default_fields
fn default() -> Self {
E2 {
x: s(),
y: i(),
z: 0,
}
}
}
fn i() -> i32 {
1
}
// Account for a `const fn` being the `Default::default()` of a field's type.
struct F {
x: G,
y: i32 = 1,
}
impl Default for F { //~ ERROR default_overrides_default_fields
fn default() -> Self {
F {
x: g_const(),
y: 0,
}
}
}
struct G;
impl Default for G { // ok
fn default() -> Self {
g_const()
}
}
const fn g_const() -> G {
G
}
// Account for a `const fn` being used in `Default::default()`, even if the type doesn't use it as
// its own `Default`. We suggest setting the default field value in that case.
struct H {
x: I,
y: i32 = 1,
}
impl Default for H { //~ ERROR default_overrides_default_fields
fn default() -> Self {
H {
x: i_const(),
y: 0,
}
}
}
struct I;
const fn i_const() -> I {
I
}
// Account for a `const` and struct literal being the `Default::default()` of a field's type.
struct M {
x: N,
y: i32 = 1,
z: A,
}
impl Default for M { // ok, `y` is not specified
fn default() -> Self {
M {
x: N_CONST,
z: A {
x: S(0),
y: 0,
},
..
}
}
}
struct N;
const N_CONST: N = N;
struct O {
x: Option<i32>,
y: i32 = 1,
}
impl Default for O { //~ ERROR default_overrides_default_fields
fn default() -> Self {
O {
x: None,
y: 1,
}
}
}
fn main() {}

144
tests/ui/structs/manual-default-impl-could-be-derived.stderr Normal file
View File

@ -0,0 +1,144 @@
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:14:1
|
LL | / impl Default for A {
LL | | fn default() -> Self {
LL | | A {
LL | | y: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
note: the lint level is defined here
--> $DIR/manual-default-impl-could-be-derived.rs:5:9
|
LL | #![deny(default_overrides_default_fields)]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:28:1
|
LL | / impl Default for B {
LL | | fn default() -> Self {
LL | | B {
LL | | x: s(),
| | --- this field has a default value
LL | | y: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: to avoid divergence in behavior between `Struct { .. }` and `<Struct as Default>::default()`, derive the `Default`
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:43:1
|
LL | / impl Default for C {
LL | | fn default() -> Self {
LL | | C {
LL | | x: s(),
LL | | y: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:59:1
|
LL | / impl Default for D {
LL | | fn default() -> Self {
LL | | D {
LL | | y: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:75:1
|
LL | / impl Default for E {
LL | | fn default() -> Self {
LL | | E {
LL | | y: 0,
| | - this field has a default value
LL | | z: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:93:1
|
LL | / impl Default for E2 {
LL | | fn default() -> Self {
LL | | E2 {
LL | | x: s(),
LL | | y: i(),
| | --- this field has a default value
LL | | z: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:113:1
|
LL | / impl Default for F {
LL | | fn default() -> Self {
LL | | F {
LL | | x: g_const(),
LL | | y: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:141:1
|
LL | / impl Default for H {
LL | | fn default() -> Self {
LL | | H {
LL | | x: i_const(),
LL | | y: 0,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
error: `Default` impl doesn't use the declared default field values
--> $DIR/manual-default-impl-could-be-derived.rs:185:1
|
LL | / impl Default for O {
LL | | fn default() -> Self {
LL | | O {
LL | | x: None,
LL | | y: 1,
| | - this field has a default value
... |
LL | | }
| |_^
|
= help: use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them diverging over time
error: aborting due to 9 previous errors