diff options
| author | Philipp Krones <hello@philkrones.com> | 2025-01-09 18:00:37 +0100 |
|---|---|---|
| committer | Philipp Krones <hello@philkrones.com> | 2025-01-09 18:00:37 +0100 |
| commit | b5bf09e57afa50a2ecc9bfc07bad4ef64d9df447 (patch) | |
| tree | cf262aea51c147a0348a7112966198ea63fc0303 /clippy_lints/src/inconsistent_struct_constructor.rs | |
| parent | 11f38ade90a2e1f9cee925260ffbe8bc0e3ad761 (diff) | |
| parent | 894e87cd5160a2198940a35dc105ce6e46d9763e (diff) | |
| download | rust-b5bf09e57afa50a2ecc9bfc07bad4ef64d9df447.tar.gz rust-b5bf09e57afa50a2ecc9bfc07bad4ef64d9df447.zip | |
Merge remote-tracking branch 'upstream/master' into rustup
Diffstat (limited to 'clippy_lints/src/inconsistent_struct_constructor.rs')
| -rw-r--r-- | clippy_lints/src/inconsistent_struct_constructor.rs | 127 |
1 files changed, 90 insertions, 37 deletions
diff --git a/clippy_lints/src/inconsistent_struct_constructor.rs b/clippy_lints/src/inconsistent_struct_constructor.rs index 4fcd2abb769..39ff3c13bcc 100644 --- a/clippy_lints/src/inconsistent_struct_constructor.rs +++ b/clippy_lints/src/inconsistent_struct_constructor.rs @@ -1,19 +1,21 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_config::Conf; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::fulfill_or_allowed; use clippy_utils::source::snippet; use rustc_data_structures::fx::FxHashMap; use rustc_errors::Applicability; -use rustc_hir::{self as hir, ExprKind, StructTailExpr}; +use rustc_hir::{self as hir, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; +use rustc_middle::ty::TyCtxt; +use rustc_session::impl_lint_pass; +use rustc_span::Span; use rustc_span::symbol::Symbol; -use std::fmt::{self, Write as _}; declare_clippy_lint! { /// ### What it does - /// Checks for struct constructors where all fields are shorthand and - /// the order of the field init shorthand in the constructor is inconsistent - /// with the order in the struct definition. + /// Checks for struct constructors where the order of the field + /// init in the constructor is inconsistent with the order in the + /// struct definition. /// /// ### Why is this bad? /// Since the order of fields in a constructor doesn't affect the @@ -59,16 +61,37 @@ declare_clippy_lint! { #[clippy::version = "1.52.0"] pub INCONSISTENT_STRUCT_CONSTRUCTOR, pedantic, - "the order of the field init shorthand is inconsistent with the order in the struct definition" + "the order of the field init is inconsistent with the order in the struct definition" } -declare_lint_pass!(InconsistentStructConstructor => [INCONSISTENT_STRUCT_CONSTRUCTOR]); +pub struct InconsistentStructConstructor { + lint_inconsistent_struct_field_initializers: bool, +} + +impl InconsistentStructConstructor { + pub fn new(conf: &'static Conf) -> Self { + Self { + lint_inconsistent_struct_field_initializers: conf.lint_inconsistent_struct_field_initializers, + } + } +} + +impl_lint_pass!(InconsistentStructConstructor => [INCONSISTENT_STRUCT_CONSTRUCTOR]); impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if let ExprKind::Struct(qpath, fields, base) = expr.kind - && fields.iter().all(|f| f.is_shorthand) - && !expr.span.from_expansion() + let ExprKind::Struct(_, fields, _) = expr.kind else { + return; + }; + let all_fields_are_shorthand = fields.iter().all(|f| f.is_shorthand); + let applicability = if all_fields_are_shorthand { + Applicability::MachineApplicable + } else if self.lint_inconsistent_struct_field_initializers { + Applicability::MaybeIncorrect + } else { + return; + }; + if !expr.span.from_expansion() && let ty = cx.typeck_results().expr_ty(expr) && let Some(adt_def) = ty.ty_adt_def() && adt_def.is_struct() @@ -85,36 +108,24 @@ impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor { return; } - let mut ordered_fields: Vec<_> = fields.iter().map(|f| f.ident.name).collect(); - ordered_fields.sort_unstable_by_key(|id| def_order_map[id]); - - let mut fields_snippet = String::new(); - let (last_ident, idents) = ordered_fields.split_last().unwrap(); - for ident in idents { - let _: fmt::Result = write!(fields_snippet, "{ident}, "); - } - fields_snippet.push_str(&last_ident.to_string()); - - let base_snippet = if let StructTailExpr::Base(base) = base { - format!(", ..{}", snippet(cx, base.span, "..")) - } else { - String::new() - }; - - let sugg = format!( - "{} {{ {fields_snippet}{base_snippet} }}", - snippet(cx, qpath.span(), ".."), - ); + let span = field_with_attrs_span(cx.tcx, fields.first().unwrap()) + .with_hi(field_with_attrs_span(cx.tcx, fields.last().unwrap()).hi()); if !fulfill_or_allowed(cx, INCONSISTENT_STRUCT_CONSTRUCTOR, Some(ty_hir_id)) { - span_lint_and_sugg( + span_lint_and_then( cx, INCONSISTENT_STRUCT_CONSTRUCTOR, - expr.span, + span, "struct constructor field order is inconsistent with struct definition field order", - "try", - sugg, - Applicability::MachineApplicable, + |diag| { + let msg = if all_fields_are_shorthand { + "try" + } else { + "if the field evaluation order doesn't matter, try" + }; + let sugg = suggestion(cx, fields, &def_order_map); + diag.span_suggestion(span, msg, sugg, applicability); + }, ); } } @@ -135,3 +146,45 @@ fn is_consistent_order<'tcx>(fields: &'tcx [hir::ExprField<'tcx>], def_order_map true } + +fn suggestion<'tcx>( + cx: &LateContext<'_>, + fields: &'tcx [hir::ExprField<'tcx>], + def_order_map: &FxHashMap<Symbol, usize>, +) -> String { + let ws = fields + .windows(2) + .map(|w| { + let w0_span = field_with_attrs_span(cx.tcx, &w[0]); + let w1_span = field_with_attrs_span(cx.tcx, &w[1]); + let span = w0_span.between(w1_span); + snippet(cx, span, " ") + }) + .collect::<Vec<_>>(); + + let mut fields = fields.to_vec(); + fields.sort_unstable_by_key(|field| def_order_map[&field.ident.name]); + let field_snippets = fields + .iter() + .map(|field| snippet(cx, field_with_attrs_span(cx.tcx, field), "..")) + .collect::<Vec<_>>(); + + assert_eq!(field_snippets.len(), ws.len() + 1); + + let mut sugg = String::new(); + for i in 0..field_snippets.len() { + sugg += &field_snippets[i]; + if i < ws.len() { + sugg += &ws[i]; + } + } + sugg +} + +fn field_with_attrs_span(tcx: TyCtxt<'_>, field: &hir::ExprField<'_>) -> Span { + if let Some(attr) = tcx.hir().attrs(field.hir_id).first() { + field.span.with_lo(attr.span.lo()) + } else { + field.span + } +} |
