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
This commit is contained in:
bors 2021-10-09 13:44:36 +00:00
commit 8f1555f123
3 changed files with 97 additions and 1 deletions

View File

@ -1,5 +1,6 @@
use clippy_utils::diagnostics::{span_lint_and_note, span_lint_and_sugg};
use clippy_utils::source::snippet_with_macro_callsite;
use clippy_utils::ty::{has_drop, is_copy};
use clippy_utils::{any_parent_is_automatically_derived, contains_name, in_macro, match_def_path, paths};
use if_chain::if_chain;
use rustc_data_structures::fx::FxHashSet;
@ -139,6 +140,13 @@ impl LateLintPass<'_> for Default {
.fields
.iter()
.all(|field| field.vis.is_accessible_from(module_did, cx.tcx));
let all_fields_are_copy = variant
.fields
.iter()
.all(|field| {
is_copy(cx, cx.tcx.type_of(field.did))
});
if !has_drop(cx, binding_type) || all_fields_are_copy;
then {
(local, variant, ident.name, binding_type, expr.span)
} else {

View File

@ -183,3 +183,67 @@ struct WrapperMulti<T, U> {
i: T,
j: U,
}
mod issue6312 {
use std::sync::atomic::AtomicBool;
use std::sync::Arc;
// do not lint: type implements `Drop` but not all fields are `Copy`
#[derive(Clone, Default)]
pub struct ImplDropNotAllCopy {
name: String,
delay_data_sync: Arc<AtomicBool>,
}
impl Drop for ImplDropNotAllCopy {
fn drop(&mut self) {
self.close()
}
}
impl ImplDropNotAllCopy {
fn new(name: &str) -> Self {
let mut f = ImplDropNotAllCopy::default();
f.name = name.to_owned();
f
}
fn close(&self) {}
}
// lint: type implements `Drop` and all fields are `Copy`
#[derive(Clone, Default)]
pub struct ImplDropAllCopy {
name: usize,
delay_data_sync: bool,
}
impl Drop for ImplDropAllCopy {
fn drop(&mut self) {
self.close()
}
}
impl ImplDropAllCopy {
fn new(name: &str) -> Self {
let mut f = ImplDropAllCopy::default();
f.name = name.len();
f
}
fn close(&self) {}
}
// lint: type does not implement `Drop` though all fields are `Copy`
#[derive(Clone, Default)]
pub struct NoDropAllCopy {
name: usize,
delay_data_sync: bool,
}
impl NoDropAllCopy {
fn new(name: &str) -> Self {
let mut f = NoDropAllCopy::default();
f.name = name.len();
f
}
}
}

View File

@ -107,5 +107,29 @@ note: consider initializing the variable with `WrapperMulti::<i32, i64> { i: 42,
LL | let mut a: WrapperMulti<i32, i64> = Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 9 previous errors
error: field assignment outside of initializer for an instance created with Default::default()
--> $DIR/field_reassign_with_default.rs:229:13
|
LL | f.name = name.len();
| ^^^^^^^^^^^^^^^^^^^^
|
note: consider initializing the variable with `issue6312::ImplDropAllCopy { name: name.len(), ..Default::default() }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:228:13
|
LL | let mut f = ImplDropAllCopy::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: field assignment outside of initializer for an instance created with Default::default()
--> $DIR/field_reassign_with_default.rs:245:13
|
LL | f.name = name.len();
| ^^^^^^^^^^^^^^^^^^^^
|
note: consider initializing the variable with `issue6312::NoDropAllCopy { name: name.len(), ..Default::default() }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:244:13
|
LL | let mut f = NoDropAllCopy::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 11 previous errors