Skip to content

Detect when a field default is not using that field's type's default values #135859

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 128 additions & 3 deletions compiler/rustc_lint/src/default_could_be_derived.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::{Applicability, Diag};
use hir::def::{CtorOf, DefKind, Res};
use rustc_ast::Recovered;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::{Applicability, Diag, MultiSpan};
use rustc_hir as hir;
use rustc_middle::ty;
use rustc_middle::ty::TyCtxt;
Expand Down Expand Up @@ -50,7 +52,6 @@ declare_lint! {
@feature_gate = default_field_values;
}

#[derive(Default)]
pub(crate) struct DefaultCouldBeDerived;

impl_lint_pass!(DefaultCouldBeDerived => [DEFAULT_OVERRIDES_DEFAULT_FIELDS]);
Expand Down Expand Up @@ -201,3 +202,127 @@ fn mk_lint(
diag.help(msg);
}
}

declare_lint! {
/// The `default_field_overrides_default_field` lint checks for struct literals in field default
/// values with fields that have in turn default values. These should instead be skipped and
/// rely on `..` for them.
///
/// ### Example
///
/// ```rust,compile_fail
/// #![feature(default_field_values)]
/// #![deny(default_field_overrides_default_field)]
///
/// struct Foo {
/// x: Bar = Bar { x: 0 }, // `Foo { .. }.x.x` != `Bar { .. }.x`
/// }
///
/// struct Bar {
/// x: i32 = 101,
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// Defaulting a field to a value different to that field's type already defined default can
/// easily lead to confusion due to diverging behavior. Acknowleding that there can be reasons
/// for one to write an API that does this, this is not outright rejected by the compiler,
/// merely linted against.
pub DEFAULT_FIELD_OVERRIDES_DEFAULT_FIELD,
Deny,
"detect default field value that should use the type's default field values",
@feature_gate = default_field_values;
}

pub(crate) struct DefaultFieldOverride;

impl_lint_pass!(DefaultFieldOverride => [DEFAULT_FIELD_OVERRIDES_DEFAULT_FIELD]);

impl DefaultFieldOverride {
fn lint_variant(&mut self, cx: &LateContext<'_>, data: &hir::VariantData<'_>) {
if !cx.tcx.features().default_field_values() {
return;
}
let hir::VariantData::Struct { fields, recovered: Recovered::No } = data else {
return;
};

for default in fields.iter().filter_map(|f| f.default) {
let body = cx.tcx.hir().body(default.body);
let hir::ExprKind::Struct(hir::QPath::Resolved(_, path), fields, _) = body.value.kind
else {
continue;
};
let Res::Def(
DefKind::Variant
| DefKind::Struct
| DefKind::Ctor(CtorOf::Variant | CtorOf::Struct, ..),
def_id,
) = path.res
else {
continue;
};
let fields_set: FxHashSet<_> = fields.iter().map(|f| f.ident.name).collect();
let variant = cx.tcx.expect_variant_res(path.res);
let mut to_lint = vec![];
let mut defs = vec![];

for field in &variant.fields {
if fields_set.contains(&field.name) {
for f in fields {
if f.ident.name == field.name
&& let Some(default) = field.value
{
to_lint.push((f.expr.span, f.ident.name));
defs.push(cx.tcx.def_span(default));
}
}
}
}

if to_lint.is_empty() {
continue;
}
cx.tcx.node_span_lint(
DEFAULT_FIELD_OVERRIDES_DEFAULT_FIELD,
body.value.hir_id,
to_lint.iter().map(|&(span, _)| span).collect::<Vec<_>>(),
|diag| {
diag.primary_message("default field overrides that field's type's default");
diag.span_label(path.span, "when constructing this value");
let type_name = cx.tcx.item_name(def_id);
for (span, name) in to_lint {
diag.span_label(
span,
format!(
"this overrides the default of field `{name}` in `{type_name}`"
),
);
}
let mut overriden_spans: MultiSpan = defs.clone().into();
overriden_spans.push_span_label(cx.tcx.def_span(def_id), "");
diag.span_note(
overriden_spans,
format!(
"{this} field's default value in `{type_name}` is overriden",
this = if defs.len() == 1 { "this" } else { "these" }
),
);
},
);
}
}
}

impl<'tcx> LateLintPass<'tcx> for DefaultFieldOverride {
fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) {
let hir::ItemKind::Struct(data, _) = item.kind else { return };
self.lint_variant(cx, &data);
}
fn check_variant(&mut self, cx: &LateContext<'_>, variant: &hir::Variant<'_>) {
self.lint_variant(cx, &variant.data);
}
}
5 changes: 3 additions & 2 deletions compiler/rustc_lint/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ use async_closures::AsyncClosureUsage;
use async_fn_in_trait::AsyncFnInTrait;
use builtin::*;
use dangling::*;
use default_could_be_derived::DefaultCouldBeDerived;
use default_could_be_derived::{DefaultCouldBeDerived, DefaultFieldOverride};
use deref_into_dyn_supertrait::*;
use drop_forget_useless::*;
use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
Expand Down Expand Up @@ -191,7 +191,8 @@ late_lint_methods!(
BuiltinCombinedModuleLateLintPass,
[
ForLoopsOverFallibles: ForLoopsOverFallibles,
DefaultCouldBeDerived: DefaultCouldBeDerived::default(),
DefaultCouldBeDerived: DefaultCouldBeDerived,
DefaultFieldOverride: DefaultFieldOverride,
DerefIntoDynSupertrait: DerefIntoDynSupertrait,
DropForgetUseless: DropForgetUseless,
ImproperCTypesDeclarations: ImproperCTypesDeclarations,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
#![feature(default_field_values)]
#![allow(dead_code)]

mod a {
pub struct Foo {
pub something: i32 = 2,
}

pub struct Bar {
pub foo: Foo = Foo { something: 10 }, //~ ERROR default field overrides that field's type's default
}
}

mod b {
pub enum Foo {
X {
something: i32 = 2,
}
}

pub enum Bar {
Y {
foo: Foo = Foo::X { something: 10 }, //~ ERROR default field overrides that field's type's default
}
}
}

fn main() {
let x = a::Bar { .. };
let y = a::Foo { .. };
assert_eq!(x.foo.something, y.something);
let x = b::Bar::Y { .. };
let y = b::Foo::X { .. };
match (x, y) {
(b::Bar::Y { foo: b::Foo::X { something: a} }, b::Foo::X { something:b }) if a == b=> {}
_ => panic!(),
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
error: default field overrides that field's type's default
--> $DIR/diverging-default-field-values-in-field.rs:10:41
|
LL | pub foo: Foo = Foo { something: 10 },
| --- ^^ this overrides the default of field `something` in `Foo`
| |
| when constructing this value
|
note: this field's default value in `Foo` is overriden
--> $DIR/diverging-default-field-values-in-field.rs:6:30
|
LL | pub struct Foo {
| --------------
LL | pub something: i32 = 2,
| ^
= note: `#[deny(default_field_overrides_default_field)]` on by default

error: default field overrides that field's type's default
--> $DIR/diverging-default-field-values-in-field.rs:23:44
|
LL | foo: Foo = Foo::X { something: 10 },
| ------ ^^ this overrides the default of field `something` in `X`
| |
| when constructing this value
|
note: this field's default value in `X` is overriden
--> $DIR/diverging-default-field-values-in-field.rs:17:30
|
LL | X {
| -
LL | something: i32 = 2,
| ^

error: aborting due to 2 previous errors

Loading