Skip to content

Commit ef11e11

Browse files
Henri Lunnikivihegza
Henri Lunnikivi
authored andcommitted
Retain order of statements in suggested code
Use Vec instead of HashMap to prevent re-orders. Also flip one assignment in test to account for this case.
1 parent 82771e0 commit ef11e11

File tree

3 files changed

+15
-9
lines changed

3 files changed

+15
-9
lines changed

clippy_lints/src/field_reassign_with_default.rs

+12-6
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use crate::utils::{contains_name, match_def_path, paths, qpath_res, snippet, span_lint_and_note};
22
use if_chain::if_chain;
3-
use rustc_data_structures::fx::FxHashMap;
43
use rustc_hir::def::Res;
54
use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind};
65
use rustc_middle::ty::{self, Adt, Ty};
@@ -57,7 +56,7 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
5756
// find all "later statement"'s where the fields of the binding set as
5857
// Default::default() get reassigned, unless the reassignment refers to the original binding
5958
let mut first_assign = None;
60-
let mut assigned_fields = FxHashMap::default();
59+
let mut assigned_fields = Vec::new();
6160
let mut cancel_lint = false;
6261
for consecutive_statement in &block.stmts[stmt_idx + 1..] {
6362
// interrupt if the statement is a let binding (`Local`) that shadows the original
@@ -75,8 +74,15 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
7574
break;
7675
}
7776

78-
// always re-insert set value, this way the latest value is stored for output snippet
79-
assigned_fields.insert(field_ident.name, assign_rhs);
77+
// if the field was previously assigned, replace the assignment, otherwise insert the assignment
78+
if let Some(prev) = assigned_fields
79+
.iter_mut()
80+
.find(|(field_name, _)| field_name == &field_ident.name)
81+
{
82+
*prev = (field_ident.name, assign_rhs);
83+
} else {
84+
assigned_fields.push((field_ident.name, assign_rhs));
85+
}
8086

8187
// also set first instance of error for help message
8288
if first_assign.is_none() {
@@ -100,12 +106,12 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
100106
let assigned_fields = assigned_fields
101107
.into_iter()
102108
.filter(|(_, rhs)| !is_expr_default(rhs, cx))
103-
.collect::<FxHashMap<Symbol, &Expr<'_>>>();
109+
.collect::<Vec<(Symbol, &Expr<'_>)>>();
104110

105111
// if all fields of the struct are not assigned, add `.. Default::default()` to the suggestion.
106112
let ext_with_default = !fields_of_type(binding_type)
107113
.iter()
108-
.all(|field| assigned_fields.contains_key(&field.name));
114+
.all(|field| assigned_fields.iter().any(|(a, _)| a == &field.name));
109115

110116
let field_list = assigned_fields
111117
.into_iter()

tests/ui/field_reassign_with_default.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ fn main() {
5454

5555
// wrong, produces second error in stderr
5656
let mut a: A = Default::default();
57-
a.i = 42;
5857
a.j = 43;
58+
a.i = 42;
5959

6060
// wrong, produces third error in stderr
6161
let mut a: A = Default::default();

tests/ui/field_reassign_with_default.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ LL | let mut a: A = Default::default();
1414
error: field assignment outside of initializer for an instance created with Default::default()
1515
--> $DIR/field_reassign_with_default.rs:57:5
1616
|
17-
LL | a.i = 42;
17+
LL | a.j = 43;
1818
| ^^^^^^^^^
1919
|
20-
note: consider initializing the variable with `A { i: 42, j: 43 }`
20+
note: consider initializing the variable with `A { j: 43, i: 42 }`
2121
--> $DIR/field_reassign_with_default.rs:56:5
2222
|
2323
LL | let mut a: A = Default::default();

0 commit comments

Comments
 (0)