Skip to content

Commit 8f1555f

Browse files
committed
Auto merge of #7794 - ThibsG:FieldReassignDefault6312, r=llogiq
Fix false positive when `Drop` and `Copy` involved in `field_reassign_with_default` lint Fix FP in `field_reassign_with_default` lint when type implements `Drop` but not all fields are `Copy`. fixes: #6312 changelog: [`field_reassign_with_default`] Fix FP when `Drop` and `Copy` are involved
2 parents 22144c0 + 7aaed02 commit 8f1555f

File tree

3 files changed

+97
-1
lines changed

3 files changed

+97
-1
lines changed

clippy_lints/src/default.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg};
22
use clippy_utils::source::snippet_with_macro_callsite;
3+
use clippy_utils::ty::{has_drop, is_copy};
34
use clippy_utils::{any_parent_is_automatically_derived, contains_name, in_macro, match_def_path, paths};
45
use if_chain::if_chain;
56
use rustc_data_structures::fx::FxHashSet;
@@ -139,6 +140,13 @@ impl LateLintPass<'_> for Default {
139140
.fields
140141
.iter()
141142
.all(|field| field.vis.is_accessible_from(module_did, cx.tcx));
143+
let all_fields_are_copy = variant
144+
.fields
145+
.iter()
146+
.all(|field| {
147+
is_copy(cx, cx.tcx.type_of(field.did))
148+
});
149+
if !has_drop(cx, binding_type) || all_fields_are_copy;
142150
then {
143151
(local, variant, ident.name, binding_type, expr.span)
144152
} else {

tests/ui/field_reassign_with_default.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,3 +183,67 @@ struct WrapperMulti<T, U> {
183183
i: T,
184184
j: U,
185185
}
186+
187+
mod issue6312 {
188+
use std::sync::atomic::AtomicBool;
189+
use std::sync::Arc;
190+
191+
// do not lint: type implements `Drop` but not all fields are `Copy`
192+
#[derive(Clone, Default)]
193+
pub struct ImplDropNotAllCopy {
194+
name: String,
195+
delay_data_sync: Arc<AtomicBool>,
196+
}
197+
198+
impl Drop for ImplDropNotAllCopy {
199+
fn drop(&mut self) {
200+
self.close()
201+
}
202+
}
203+
204+
impl ImplDropNotAllCopy {
205+
fn new(name: &str) -> Self {
206+
let mut f = ImplDropNotAllCopy::default();
207+
f.name = name.to_owned();
208+
f
209+
}
210+
fn close(&self) {}
211+
}
212+
213+
// lint: type implements `Drop` and all fields are `Copy`
214+
#[derive(Clone, Default)]
215+
pub struct ImplDropAllCopy {
216+
name: usize,
217+
delay_data_sync: bool,
218+
}
219+
220+
impl Drop for ImplDropAllCopy {
221+
fn drop(&mut self) {
222+
self.close()
223+
}
224+
}
225+
226+
impl ImplDropAllCopy {
227+
fn new(name: &str) -> Self {
228+
let mut f = ImplDropAllCopy::default();
229+
f.name = name.len();
230+
f
231+
}
232+
fn close(&self) {}
233+
}
234+
235+
// lint: type does not implement `Drop` though all fields are `Copy`
236+
#[derive(Clone, Default)]
237+
pub struct NoDropAllCopy {
238+
name: usize,
239+
delay_data_sync: bool,
240+
}
241+
242+
impl NoDropAllCopy {
243+
fn new(name: &str) -> Self {
244+
let mut f = NoDropAllCopy::default();
245+
f.name = name.len();
246+
f
247+
}
248+
}
249+
}

tests/ui/field_reassign_with_default.stderr

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,5 +107,29 @@ note: consider initializing the variable with `WrapperMulti::<i32, i64> { i: 42,
107107
LL | let mut a: WrapperMulti<i32, i64> = Default::default();
108108
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
109109

110-
error: aborting due to 9 previous errors
110+
error: field assignment outside of initializer for an instance created with Default::default()
111+
--> $DIR/field_reassign_with_default.rs:229:13
112+
|
113+
LL | f.name = name.len();
114+
| ^^^^^^^^^^^^^^^^^^^^
115+
|
116+
note: consider initializing the variable with `issue6312::ImplDropAllCopy { name: name.len(), ..Default::default() }` and removing relevant reassignments
117+
--> $DIR/field_reassign_with_default.rs:228:13
118+
|
119+
LL | let mut f = ImplDropAllCopy::default();
120+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
121+
122+
error: field assignment outside of initializer for an instance created with Default::default()
123+
--> $DIR/field_reassign_with_default.rs:245:13
124+
|
125+
LL | f.name = name.len();
126+
| ^^^^^^^^^^^^^^^^^^^^
127+
|
128+
note: consider initializing the variable with `issue6312::NoDropAllCopy { name: name.len(), ..Default::default() }` and removing relevant reassignments
129+
--> $DIR/field_reassign_with_default.rs:244:13
130+
|
131+
LL | let mut f = NoDropAllCopy::default();
132+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
133+
134+
error: aborting due to 11 previous errors
111135

0 commit comments

Comments
 (0)