|
| 1 | +use rustc_data_structures::fx::FxHashMap; |
| 2 | +use rustc_errors::Diag; |
| 3 | +use rustc_hir as hir; |
| 4 | +use rustc_middle::ty; |
| 5 | +use rustc_session::{declare_lint, impl_lint_pass}; |
| 6 | +use rustc_span::Symbol; |
| 7 | +use rustc_span::symbol::sym; |
| 8 | + |
| 9 | +use crate::{LateContext, LateLintPass}; |
| 10 | + |
| 11 | +declare_lint! { |
| 12 | + /// The `default_overrides_default_fields` lint checks for manual `impl` blocks of the |
| 13 | + /// `Default` trait of types with default field values. |
| 14 | + /// |
| 15 | + /// ### Example |
| 16 | + /// |
| 17 | + /// ```rust,compile_fail |
| 18 | + /// #![feature(default_field_values)] |
| 19 | + /// struct Foo { |
| 20 | + /// x: i32 = 101, |
| 21 | + /// y: NonDefault, |
| 22 | + /// } |
| 23 | + /// |
| 24 | + /// struct NonDefault; |
| 25 | + /// |
| 26 | + /// #[deny(default_overrides_default_fields)] |
| 27 | + /// impl Default for Foo { |
| 28 | + /// fn default() -> Foo { |
| 29 | + /// Foo { x: 100, y: NonDefault } |
| 30 | + /// } |
| 31 | + /// } |
| 32 | + /// ``` |
| 33 | + /// |
| 34 | + /// {{produces}} |
| 35 | + /// |
| 36 | + /// ### Explanation |
| 37 | + /// |
| 38 | + /// Manually writing a `Default` implementation for a type that has |
| 39 | + /// default field values runs the risk of diverging behavior between |
| 40 | + /// `Type { .. }` and `<Type as Default>::default()`, which would be a |
| 41 | + /// foot-gun for users of that type that would expect these to be |
| 42 | + /// equivalent. If `Default` can't be derived due to some fields not |
| 43 | + /// having a `Default` implementation, we encourage the use of `..` for |
| 44 | + /// the fields that do have a default field value. |
| 45 | + pub DEFAULT_OVERRIDES_DEFAULT_FIELDS, |
| 46 | + Deny, |
| 47 | + "detect `Default` impl that should use the type's default field values", |
| 48 | + @feature_gate = default_field_values; |
| 49 | +} |
| 50 | + |
| 51 | +#[derive(Default)] |
| 52 | +pub(crate) struct DefaultCouldBeDerived; |
| 53 | + |
| 54 | +impl_lint_pass!(DefaultCouldBeDerived => [DEFAULT_OVERRIDES_DEFAULT_FIELDS]); |
| 55 | + |
| 56 | +impl<'tcx> LateLintPass<'tcx> for DefaultCouldBeDerived { |
| 57 | + fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) { |
| 58 | + // Look for manual implementations of `Default`. |
| 59 | + let Some(default_def_id) = cx.tcx.get_diagnostic_item(sym::Default) else { return }; |
| 60 | + let hir::ImplItemKind::Fn(_sig, body_id) = impl_item.kind else { return }; |
| 61 | + let assoc = cx.tcx.associated_item(impl_item.owner_id); |
| 62 | + let parent = assoc.container_id(cx.tcx); |
| 63 | + if cx.tcx.has_attr(parent, sym::automatically_derived) { |
| 64 | + // We don't care about what `#[derive(Default)]` produces in this lint. |
| 65 | + return; |
| 66 | + } |
| 67 | + let Some(trait_ref) = cx.tcx.impl_trait_ref(parent) else { return }; |
| 68 | + let trait_ref = trait_ref.instantiate_identity(); |
| 69 | + if trait_ref.def_id != default_def_id { |
| 70 | + return; |
| 71 | + } |
| 72 | + let ty = trait_ref.self_ty(); |
| 73 | + let ty::Adt(def, _) = ty.kind() else { return }; |
| 74 | + |
| 75 | + // We now know we have a manually written definition of a `<Type as Default>::default()`. |
| 76 | + |
| 77 | + let hir = cx.tcx.hir(); |
| 78 | + |
| 79 | + let type_def_id = def.did(); |
| 80 | + let body = hir.body(body_id); |
| 81 | + |
| 82 | + // FIXME: evaluate bodies with statements and evaluate bindings to see if they would be |
| 83 | + // derivable. |
| 84 | + let hir::ExprKind::Block(hir::Block { stmts: _, expr: Some(expr), .. }, None) = |
| 85 | + body.value.kind |
| 86 | + else { |
| 87 | + return; |
| 88 | + }; |
| 89 | + |
| 90 | + // Keep a mapping of field name to `hir::FieldDef` for every field in the type. We'll use |
| 91 | + // these to check for things like checking whether it has a default or using its span for |
| 92 | + // suggestions. |
| 93 | + let orig_fields = match hir.get_if_local(type_def_id) { |
| 94 | + Some(hir::Node::Item(hir::Item { |
| 95 | + kind: |
| 96 | + hir::ItemKind::Struct(hir::VariantData::Struct { fields, recovered: _ }, _generics), |
| 97 | + .. |
| 98 | + })) => fields.iter().map(|f| (f.ident.name, f)).collect::<FxHashMap<_, _>>(), |
| 99 | + _ => return, |
| 100 | + }; |
| 101 | + |
| 102 | + // We check `fn default()` body is a single ADT literal and get all the fields that are |
| 103 | + // being set. |
| 104 | + let hir::ExprKind::Struct(_qpath, fields, tail) = expr.kind else { return }; |
| 105 | + |
| 106 | + // We have a struct literal |
| 107 | + // |
| 108 | + // struct Foo { |
| 109 | + // field: Type, |
| 110 | + // } |
| 111 | + // |
| 112 | + // impl Default for Foo { |
| 113 | + // fn default() -> Foo { |
| 114 | + // Foo { |
| 115 | + // field: val, |
| 116 | + // } |
| 117 | + // } |
| 118 | + // } |
| 119 | + // |
| 120 | + // We would suggest `#[derive(Default)]` if `field` has a default value, regardless of what |
| 121 | + // it is; we don't want to encourage divergent behavior between `Default::default()` and |
| 122 | + // `..`. |
| 123 | + |
| 124 | + if let hir::StructTailExpr::Base(_) = tail { |
| 125 | + // This is *very* niche. We'd only get here if someone wrote |
| 126 | + // impl Default for Ty { |
| 127 | + // fn default() -> Ty { |
| 128 | + // Ty { ..something() } |
| 129 | + // } |
| 130 | + // } |
| 131 | + // where `something()` would have to be a call or path. |
| 132 | + // We have nothing meaninful to do with this. |
| 133 | + return; |
| 134 | + } |
| 135 | + |
| 136 | + // At least one of the fields with a default value have been overriden in |
| 137 | + // the `Default` implementation. We suggest removing it and relying on `..` |
| 138 | + // instead. |
| 139 | + let any_default_field_given = |
| 140 | + fields.iter().any(|f| orig_fields.get(&f.ident.name).and_then(|f| f.default).is_some()); |
| 141 | + |
| 142 | + if !any_default_field_given { |
| 143 | + // None of the default fields were actually provided explicitly, so the manual impl |
| 144 | + // doesn't override them (the user used `..`), so there's no risk of divergent behavior. |
| 145 | + return; |
| 146 | + } |
| 147 | + |
| 148 | + let Some(local) = parent.as_local() else { return }; |
| 149 | + let hir_id = cx.tcx.local_def_id_to_hir_id(local); |
| 150 | + let hir::Node::Item(item) = cx.tcx.hir_node(hir_id) else { return }; |
| 151 | + cx.tcx.node_span_lint(DEFAULT_OVERRIDES_DEFAULT_FIELDS, hir_id, item.span, |diag| { |
| 152 | + mk_lint(diag, orig_fields, fields); |
| 153 | + }); |
| 154 | + } |
| 155 | +} |
| 156 | + |
| 157 | +fn mk_lint( |
| 158 | + diag: &mut Diag<'_, ()>, |
| 159 | + orig_fields: FxHashMap<Symbol, &hir::FieldDef<'_>>, |
| 160 | + fields: &[hir::ExprField<'_>], |
| 161 | +) { |
| 162 | + diag.primary_message("`Default` impl doesn't use the declared default field values"); |
| 163 | + |
| 164 | + // For each field in the struct expression |
| 165 | + // - if the field in the type has a default value, it should be removed |
| 166 | + // - elif the field is an expression that could be a default value, it should be used as the |
| 167 | + // field's default value (FIXME: not done). |
| 168 | + // - else, we wouldn't touch this field, it would remain in the manual impl |
| 169 | + let mut removed_all_fields = true; |
| 170 | + for field in fields { |
| 171 | + if orig_fields.get(&field.ident.name).and_then(|f| f.default).is_some() { |
| 172 | + diag.span_label(field.expr.span, "this field has a default value"); |
| 173 | + } else { |
| 174 | + removed_all_fields = false; |
| 175 | + } |
| 176 | + } |
| 177 | + |
| 178 | + diag.help(if removed_all_fields { |
| 179 | + "to avoid divergence in behavior between `Struct { .. }` and \ |
| 180 | + `<Struct as Default>::default()`, derive the `Default`" |
| 181 | + } else { |
| 182 | + "use the default values in the `impl` with `Struct { mandatory_field, .. }` to avoid them \ |
| 183 | + diverging over time" |
| 184 | + }); |
| 185 | +} |
0 commit comments