]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 | 1 | use clippy_utils::diagnostics::span_lint_and_sugg; |
cdc7bbd5 XL |
2 | use clippy_utils::source::snippet; |
3 | use if_chain::if_chain; | |
f20569fa XL |
4 | use rustc_data_structures::fx::FxHashMap; |
5 | use rustc_errors::Applicability; | |
6 | use rustc_hir::{self as hir, ExprKind}; | |
7 | use rustc_lint::{LateContext, LateLintPass}; | |
8 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
9 | use rustc_span::symbol::Symbol; | |
04454e1e | 10 | use std::fmt::Write as _; |
f20569fa | 11 | |
f20569fa | 12 | declare_clippy_lint! { |
94222f64 XL |
13 | /// ### What it does |
14 | /// Checks for struct constructors where all fields are shorthand and | |
cdc7bbd5 XL |
15 | /// the order of the field init shorthand in the constructor is inconsistent |
16 | /// with the order in the struct definition. | |
f20569fa | 17 | /// |
94222f64 XL |
18 | /// ### Why is this bad? |
19 | /// Since the order of fields in a constructor doesn't affect the | |
f20569fa XL |
20 | /// resulted instance as the below example indicates, |
21 | /// | |
22 | /// ```rust | |
23 | /// #[derive(Debug, PartialEq, Eq)] | |
24 | /// struct Foo { | |
25 | /// x: i32, | |
26 | /// y: i32, | |
27 | /// } | |
28 | /// let x = 1; | |
29 | /// let y = 2; | |
30 | /// | |
cdc7bbd5 | 31 | /// // This assertion never fails: |
f20569fa XL |
32 | /// assert_eq!(Foo { x, y }, Foo { y, x }); |
33 | /// ``` | |
34 | /// | |
cdc7bbd5 | 35 | /// inconsistent order can be confusing and decreases readability and consistency. |
f20569fa | 36 | /// |
94222f64 | 37 | /// ### Example |
f20569fa XL |
38 | /// ```rust |
39 | /// struct Foo { | |
40 | /// x: i32, | |
41 | /// y: i32, | |
42 | /// } | |
43 | /// let x = 1; | |
44 | /// let y = 2; | |
cdc7bbd5 | 45 | /// |
f20569fa XL |
46 | /// Foo { y, x }; |
47 | /// ``` | |
48 | /// | |
49 | /// Use instead: | |
50 | /// ```rust | |
51 | /// # struct Foo { | |
52 | /// # x: i32, | |
53 | /// # y: i32, | |
54 | /// # } | |
55 | /// # let x = 1; | |
56 | /// # let y = 2; | |
57 | /// Foo { x, y }; | |
58 | /// ``` | |
a2a8927a | 59 | #[clippy::version = "1.52.0"] |
f20569fa | 60 | pub INCONSISTENT_STRUCT_CONSTRUCTOR, |
17df50a5 | 61 | pedantic, |
f20569fa XL |
62 | "the order of the field init shorthand is inconsistent with the order in the struct definition" |
63 | } | |
64 | ||
65 | declare_lint_pass!(InconsistentStructConstructor => [INCONSISTENT_STRUCT_CONSTRUCTOR]); | |
66 | ||
5099ac24 | 67 | impl<'tcx> LateLintPass<'tcx> for InconsistentStructConstructor { |
f20569fa XL |
68 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { |
69 | if_chain! { | |
a2a8927a | 70 | if !expr.span.from_expansion(); |
f20569fa XL |
71 | if let ExprKind::Struct(qpath, fields, base) = expr.kind; |
72 | let ty = cx.typeck_results().expr_ty(expr); | |
73 | if let Some(adt_def) = ty.ty_adt_def(); | |
74 | if adt_def.is_struct(); | |
5e7ed085 | 75 | if let Some(variant) = adt_def.variants().iter().next(); |
f20569fa XL |
76 | if fields.iter().all(|f| f.is_shorthand); |
77 | then { | |
78 | let mut def_order_map = FxHashMap::default(); | |
79 | for (idx, field) in variant.fields.iter().enumerate() { | |
5099ac24 | 80 | def_order_map.insert(field.name, idx); |
f20569fa XL |
81 | } |
82 | ||
83 | if is_consistent_order(fields, &def_order_map) { | |
84 | return; | |
85 | } | |
86 | ||
87 | let mut ordered_fields: Vec<_> = fields.iter().map(|f| f.ident.name).collect(); | |
88 | ordered_fields.sort_unstable_by_key(|id| def_order_map[id]); | |
89 | ||
90 | let mut fields_snippet = String::new(); | |
91 | let (last_ident, idents) = ordered_fields.split_last().unwrap(); | |
92 | for ident in idents { | |
04454e1e | 93 | let _ = write!(fields_snippet, "{}, ", ident); |
f20569fa XL |
94 | } |
95 | fields_snippet.push_str(&last_ident.to_string()); | |
96 | ||
97 | let base_snippet = if let Some(base) = base { | |
98 | format!(", ..{}", snippet(cx, base.span, "..")) | |
99 | } else { | |
100 | String::new() | |
101 | }; | |
102 | ||
103 | let sugg = format!("{} {{ {}{} }}", | |
104 | snippet(cx, qpath.span(), ".."), | |
105 | fields_snippet, | |
106 | base_snippet, | |
107 | ); | |
108 | ||
109 | span_lint_and_sugg( | |
110 | cx, | |
111 | INCONSISTENT_STRUCT_CONSTRUCTOR, | |
112 | expr.span, | |
cdc7bbd5 | 113 | "struct constructor field order is inconsistent with struct definition field order", |
f20569fa XL |
114 | "try", |
115 | sugg, | |
116 | Applicability::MachineApplicable, | |
117 | ) | |
118 | } | |
119 | } | |
120 | } | |
121 | } | |
122 | ||
123 | // Check whether the order of the fields in the constructor is consistent with the order in the | |
124 | // definition. | |
125 | fn is_consistent_order<'tcx>(fields: &'tcx [hir::ExprField<'tcx>], def_order_map: &FxHashMap<Symbol, usize>) -> bool { | |
126 | let mut cur_idx = usize::MIN; | |
127 | for f in fields { | |
128 | let next_idx = def_order_map[&f.ident.name]; | |
129 | if cur_idx > next_idx { | |
130 | return false; | |
131 | } | |
132 | cur_idx = next_idx; | |
133 | } | |
134 | ||
135 | true | |
136 | } |