New lint: explicit_struct_update

[codeshaunted]

Adds a new lint called explicit_struct_update that checks if struct field instantiation for 2 or more fields can be replaced with struct update syntax. For example:

let d = A {
    a: a.a,
    b: a.b,
    c: a.c,
    d: 5,
};

becomes

let d = A {
    d: 5,
    ..a
};

I've given it the category of complexity, but I believe pedantic could also fit. I'm also not sure if this is the best name.

changelog: [explicit_struct_update]: new lint

[rustbot]

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[samueltardieu]

Thanks, this is a very interesting lint.

Can you please add tests in which part of the expression for the initialization comes from a macro, or the copy happens in a macro? You don't seem to deal with macros at all, how should the lint behave in this case?

[samueltardieu]

You should either use the snippet with applicability variants (so that applicability can be adjusted), or return early if you cannot obtain a snippet (or the indentation of a snippet). This should never happen in practice, but it is not desirable to see a MachineApplicable applied when obtaining snippets may have failed.

[samueltardieu]

The lint name should probably use the plural form, as hinted in the guide.

[samueltardieu]

Can you add a test for this?

I wonder if it would not be more efficient to just check, below, that path is not a QPath::LangItem(…), as it would indicate that the code comes from lowering and can probably not be modified anyway.

[samueltardieu]

I think you could do this more concisely, with something like:

Suggested change

	// collect the fields that are being initialized with the same field from another struct of the same
	// type
	let update_fields = fields.iter().try_fold(
	Vec::new(),
	|mut acc: Vec<(&rustc_hir::Expr<'_>, &rustc_hir::Expr<'_>)>, f| {
	if let ExprKind::Field(base_expr, field_ident) = f.expr.kind {
	if let Some(last) = acc.last() {
	match (last.1.kind, base_expr.kind) {
	(
	ExprKind::Path(hir::QPath::Resolved(_, hir::Path { res: res_a, .. })),
	ExprKind::Path(hir::QPath::Resolved(_, hir::Path { res: res_b, .. })),
	) if res_a != res_b => return None, /* if we detect instantiation from multiple bases, we */
	// don't want to lint
	_ => (),
	}
	}
	if cx.typeck_results().expr_ty(base_expr) == ty && f.ident == field_ident {
	// accumulate the expressions mapping to the actual field expression, and the expression of the
	// base struct, we do this so we can determine if the base struct is the same for all
	acc.push((f.expr, base_expr));
	}
	}
	Some(acc)
	},
	);
	let (update_base, update_fields): (_, Vec<_>) = match update_fields {
	// we only care about the field expressions at this point
	Some(fields) if fields.len() > 1 => (fields[0].1, fields.iter().map(|x| x.0.hir_id).collect()),
	// no lint if there's no fields or multiple bases
	_ => return,
	};
	// the field assignments we are keeping
	let non_update_fields_spans: Vec<_> = fields
	.iter()
	.filter_map(|f| {
	if update_fields.contains(&f.expr.hir_id) {
	None
	} else {
	Some(f.span)
	}
	})
	.collect();
	// collect the fields that are being initialized with the same field from another struct of the same
	// type
	let (mut update_base, mut update_fields, mut non_update_fields_spans) = (None, vec![], vec![]);
	for field in fields {
	if let ExprKind::Field(base_expr, field_ident) = field.expr.kind
	&& let ExprKind::Path(hir::QPath::Resolved(_, hir::Path { res, .. })) = base_expr.kind
	&& field_ident == field.ident
	&& cx.typeck_results().expr_ty(base_expr) == ty
	{
	if matches!(update_base.replace((res, base_expr.span)), Some((base_res, _)) if base_res != res) {
	// Initialization from multiple bases
	return;
	}
	update_fields.push(field.hir_id);
	} else {
	non_update_fields_spans.push(field.span);
	}
	}
	let update_base_span = match update_base {
	Some((_, span)) if update_fields.len() > 1 => span,
	_ => return,
	};