mirror of
https://github.com/rust-lang/rust.git
synced 2024-11-27 17:24:06 +00:00
Implement field_reassign_with_default
- Implement `field_reassign_with_default` as a `LateLintPass` - Avoid triggering `default_trait_access` on a span already linted by `field_reassigned_with_default` - Merge `default_trait_access` and `field_reassign_with_default` into `Default` - Co-authored-by: Eduardo Broto <ebroto@tutanota.com> - Fixes #568
This commit is contained in:
Henri Lunnikivi
Henri Lunnikivi
parent
e298c830af
commit
7b203f3da6
@ -1714,6 +1714,7 @@ Released 2018-09-13
|
||||
[`extend_from_slice`]: https://rust-lang.github.io/rust-clippy/master/index.html#extend_from_slice
|
||||
[`extra_unused_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#extra_unused_lifetimes
|
||||
[`fallible_impl_from`]: https://rust-lang.github.io/rust-clippy/master/index.html#fallible_impl_from
|
||||
[`field_reassign_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
|
||||
[`filetype_is_file`]: https://rust-lang.github.io/rust-clippy/master/index.html#filetype_is_file
|
||||
[`filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map
|
||||
[`filter_map_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#filter_map_next
|
||||
|
||||
304
clippy_lints/src/default.rs
Normal file
304
clippy_lints/src/default.rs
Normal file
@ -0,0 +1,304 @@
|
||||
use crate::utils::{any_parent_is_automatically_derived, contains_name, match_def_path, paths, qpath_res, snippet};
|
||||
use crate::utils::{span_lint_and_note, span_lint_and_sugg};
|
||||
use if_chain::if_chain;
|
||||
use rustc_data_structures::fx::FxHashSet;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::def::Res;
|
||||
use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::{self, Adt, Ty};
|
||||
use rustc_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::symbol::{Ident, Symbol};
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for literal calls to `Default::default()`.
|
||||
///
|
||||
/// **Why is this bad?** It's more clear to the reader to use the name of the type whose default is
|
||||
/// being gotten than the generic `Default`.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// // Bad
|
||||
/// let s: String = Default::default();
|
||||
///
|
||||
/// // Good
|
||||
/// let s = String::default();
|
||||
/// ```
|
||||
pub DEFAULT_TRAIT_ACCESS,
|
||||
pedantic,
|
||||
"checks for literal calls to `Default::default()`"
|
||||
}
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for immediate reassignment of fields initialized
|
||||
/// with Default::default().
|
||||
///
|
||||
/// **Why is this bad?**It's more idiomatic to use the [functional update syntax](https://doc.rust-lang.org/reference/expressions/struct-expr.html#functional-update-syntax).
|
||||
///
|
||||
/// **Known problems:** Assignments to patterns that are of tuple type are not linted.
|
||||
///
|
||||
/// **Example:**
|
||||
/// Bad:
|
||||
/// ```
|
||||
/// # #[derive(Default)]
|
||||
/// # struct A { i: i32 }
|
||||
/// let mut a: A = Default::default();
|
||||
/// a.i = 42;
|
||||
/// ```
|
||||
/// Use instead:
|
||||
/// ```
|
||||
/// # #[derive(Default)]
|
||||
/// # struct A { i: i32 }
|
||||
/// let a = A {
|
||||
/// i: 42,
|
||||
/// .. Default::default()
|
||||
/// };
|
||||
/// ```
|
||||
pub FIELD_REASSIGN_WITH_DEFAULT,
|
||||
style,
|
||||
"binding initialized with Default should have its fields set in the initializer"
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
pub struct Default {
|
||||
// Spans linted by `field_reassign_with_default`.
|
||||
reassigned_linted: FxHashSet<Span>,
|
||||
}
|
||||
|
||||
impl_lint_pass!(Default => [DEFAULT_TRAIT_ACCESS, FIELD_REASSIGN_WITH_DEFAULT]);
|
||||
|
||||
impl LateLintPass<'_> for Default {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
if_chain! {
|
||||
// Avoid cases already linted by `field_reassign_with_default`
|
||||
if !self.reassigned_linted.contains(&expr.span);
|
||||
if let ExprKind::Call(ref path, ..) = expr.kind;
|
||||
if !any_parent_is_automatically_derived(cx.tcx, expr.hir_id);
|
||||
if let ExprKind::Path(ref qpath) = path.kind;
|
||||
if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id();
|
||||
if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
|
||||
// Detect and ignore <Foo as Default>::default() because these calls do explicitly name the type.
|
||||
if let QPath::Resolved(None, _path) = qpath;
|
||||
then {
|
||||
let expr_ty = cx.typeck_results().expr_ty(expr);
|
||||
if let ty::Adt(def, ..) = expr_ty.kind() {
|
||||
// TODO: Work out a way to put "whatever the imported way of referencing
|
||||
// this type in this file" rather than a fully-qualified type.
|
||||
let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did));
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
DEFAULT_TRAIT_ACCESS,
|
||||
expr.span,
|
||||
&format!("calling `{}` is more clear than this expression", replacement),
|
||||
"try",
|
||||
replacement,
|
||||
Applicability::Unspecified, // First resolve the TODO above
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_block<'tcx>(&mut self, cx: &LateContext<'tcx>, block: &Block<'tcx>) {
|
||||
// find all binding statements like `let mut _ = T::default()` where `T::default()` is the
|
||||
// `default` method of the `Default` trait, and store statement index in current block being
|
||||
// checked and the name of the bound variable
|
||||
let binding_statements_using_default = enumerate_bindings_using_default(cx, block);
|
||||
|
||||
// start from the `let mut _ = _::default();` and look at all the following
|
||||
// statements, see if they re-assign the fields of the binding
|
||||
for (stmt_idx, binding_name, binding_type, span) in binding_statements_using_default {
|
||||
// the last statement of a block cannot trigger the lint
|
||||
if stmt_idx == block.stmts.len() - 1 {
|
||||
break;
|
||||
}
|
||||
|
||||
// find all "later statement"'s where the fields of the binding set as
|
||||
// Default::default() get reassigned, unless the reassignment refers to the original binding
|
||||
let mut first_assign = None;
|
||||
let mut assigned_fields = Vec::new();
|
||||
let mut cancel_lint = false;
|
||||
for consecutive_statement in &block.stmts[stmt_idx + 1..] {
|
||||
// interrupt if the statement is a let binding (`Local`) that shadows the original
|
||||
// binding
|
||||
if stmt_shadows_binding(consecutive_statement, binding_name) {
|
||||
break;
|
||||
}
|
||||
// find out if and which field was set by this `consecutive_statement`
|
||||
else if let Some((field_ident, assign_rhs)) =
|
||||
field_reassigned_by_stmt(consecutive_statement, binding_name)
|
||||
{
|
||||
// interrupt and cancel lint if assign_rhs references the original binding
|
||||
if contains_name(binding_name, assign_rhs) {
|
||||
cancel_lint = true;
|
||||
break;
|
||||
}
|
||||
|
||||
// if the field was previously assigned, replace the assignment, otherwise insert the assignment
|
||||
if let Some(prev) = assigned_fields
|
||||
.iter_mut()
|
||||
.find(|(field_name, _)| field_name == &field_ident.name)
|
||||
{
|
||||
*prev = (field_ident.name, assign_rhs);
|
||||
} else {
|
||||
assigned_fields.push((field_ident.name, assign_rhs));
|
||||
}
|
||||
|
||||
// also set first instance of error for help message
|
||||
if first_assign.is_none() {
|
||||
first_assign = Some(consecutive_statement);
|
||||
}
|
||||
}
|
||||
// interrupt also if no field was assigned, since we only want to look at consecutive statements
|
||||
else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// if there are incorrectly assigned fields, do a span_lint_and_note to suggest
|
||||
// construction using `Ty { fields, ..Default::default() }`
|
||||
if !assigned_fields.is_empty() && !cancel_lint {
|
||||
// take the original assignment as span
|
||||
let stmt = &block.stmts[stmt_idx];
|
||||
|
||||
if let StmtKind::Local(preceding_local) = &stmt.kind {
|
||||
// filter out fields like `= Default::default()`, because the FRU already covers them
|
||||
let assigned_fields = assigned_fields
|
||||
.into_iter()
|
||||
.filter(|(_, rhs)| !is_expr_default(rhs, cx))
|
||||
.collect::<Vec<(Symbol, &Expr<'_>)>>();
|
||||
|
||||
// if all fields of the struct are not assigned, add `.. Default::default()` to the suggestion.
|
||||
let ext_with_default = !fields_of_type(binding_type)
|
||||
.iter()
|
||||
.all(|field| assigned_fields.iter().any(|(a, _)| a == &field.name));
|
||||
|
||||
let field_list = assigned_fields
|
||||
.into_iter()
|
||||
.map(|(field, rhs)| {
|
||||
// extract and store the assigned value for help message
|
||||
let value_snippet = snippet(cx, rhs.span, "..");
|
||||
format!("{}: {}", field, value_snippet)
|
||||
})
|
||||
.collect::<Vec<String>>()
|
||||
.join(", ");
|
||||
|
||||
let sugg = if ext_with_default {
|
||||
if field_list.is_empty() {
|
||||
format!("{}::default()", binding_type)
|
||||
} else {
|
||||
format!("{} {{ {}, ..Default::default() }}", binding_type, field_list)
|
||||
}
|
||||
} else {
|
||||
format!("{} {{ {} }}", binding_type, field_list)
|
||||
};
|
||||
|
||||
// span lint once per statement that binds default
|
||||
span_lint_and_note(
|
||||
cx,
|
||||
FIELD_REASSIGN_WITH_DEFAULT,
|
||||
first_assign.unwrap().span,
|
||||
"field assignment outside of initializer for an instance created with Default::default()",
|
||||
Some(preceding_local.span),
|
||||
&format!(
|
||||
"consider initializing the variable with `{}` and removing relevant reassignments",
|
||||
sugg
|
||||
),
|
||||
);
|
||||
self.reassigned_linted.insert(span);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Checks if the given expression is the `default` method belonging to the `Default` trait.
|
||||
fn is_expr_default<'tcx>(expr: &'tcx Expr<'tcx>, cx: &LateContext<'tcx>) -> bool {
|
||||
if_chain! {
|
||||
if let ExprKind::Call(ref fn_expr, _) = &expr.kind;
|
||||
if let ExprKind::Path(qpath) = &fn_expr.kind;
|
||||
if let Res::Def(_, def_id) = qpath_res(cx, qpath, fn_expr.hir_id);
|
||||
then {
|
||||
// right hand side of assignment is `Default::default`
|
||||
match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD)
|
||||
} else {
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the block indices, identifiers and types of bindings set as `Default::default()`, except
|
||||
/// for when the pattern type is a tuple.
|
||||
fn enumerate_bindings_using_default<'tcx>(
|
||||
cx: &LateContext<'tcx>,
|
||||
block: &Block<'tcx>,
|
||||
) -> Vec<(usize, Symbol, Ty<'tcx>, Span)> {
|
||||
block
|
||||
.stmts
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter_map(|(idx, stmt)| {
|
||||
if_chain! {
|
||||
// only take `let ...` statements
|
||||
if let StmtKind::Local(ref local) = stmt.kind;
|
||||
// only take bindings to identifiers
|
||||
if let PatKind::Binding(_, _, ident, _) = local.pat.kind;
|
||||
// that are not tuples
|
||||
let ty = cx.typeck_results().pat_ty(local.pat);
|
||||
if !matches!(ty.kind(), ty::Tuple(_));
|
||||
// only when assigning `... = Default::default()`
|
||||
if let Some(ref expr) = local.init;
|
||||
if is_expr_default(expr, cx);
|
||||
then {
|
||||
Some((idx, ident.name, ty, expr.span))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
})
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn stmt_shadows_binding(this: &Stmt<'_>, shadowed: Symbol) -> bool {
|
||||
if let StmtKind::Local(local) = &this.kind {
|
||||
if let PatKind::Binding(_, _, ident, _) = local.pat.kind {
|
||||
return ident.name == shadowed;
|
||||
}
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
/// Returns the reassigned field and the assigning expression (right-hand side of assign).
|
||||
fn field_reassigned_by_stmt<'tcx>(this: &Stmt<'tcx>, binding_name: Symbol) -> Option<(Ident, &'tcx Expr<'tcx>)> {
|
||||
if_chain! {
|
||||
// only take assignments
|
||||
if let StmtKind::Semi(ref later_expr) = this.kind;
|
||||
if let ExprKind::Assign(ref assign_lhs, ref assign_rhs, _) = later_expr.kind;
|
||||
// only take assignments to fields where the left-hand side field is a field of
|
||||
// the same binding as the previous statement
|
||||
if let ExprKind::Field(ref binding, field_ident) = assign_lhs.kind;
|
||||
if let ExprKind::Path(ref qpath) = binding.kind;
|
||||
if let QPath::Resolved(_, path) = qpath;
|
||||
if let Some(second_binding_name) = path.segments.last();
|
||||
if second_binding_name.ident.name == binding_name;
|
||||
then {
|
||||
Some((field_ident, assign_rhs))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns the vec of fields for a struct and an empty vec for non-struct ADTs.
|
||||
fn fields_of_type(ty: Ty<'_>) -> Vec<Ident> {
|
||||
if let Adt(adt, _) = ty.kind() {
|
||||
if adt.is_struct() {
|
||||
let variant = &adt.non_enum_variant();
|
||||
return variant.fields.iter().map(|f| f.ident).collect();
|
||||
}
|
||||
}
|
||||
vec![]
|
||||
}
|
||||
@ -1,62 +0,0 @@
|
||||
use if_chain::if_chain;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Expr, ExprKind, QPath};
|
||||
use rustc_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty;
|
||||
use rustc_session::{declare_lint_pass, declare_tool_lint};
|
||||
|
||||
use crate::utils::{any_parent_is_automatically_derived, match_def_path, paths, span_lint_and_sugg};
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// **What it does:** Checks for literal calls to `Default::default()`.
|
||||
///
|
||||
/// **Why is this bad?** It's more clear to the reader to use the name of the type whose default is
|
||||
/// being gotten than the generic `Default`.
|
||||
///
|
||||
/// **Known problems:** None.
|
||||
///
|
||||
/// **Example:**
|
||||
/// ```rust
|
||||
/// // Bad
|
||||
/// let s: String = Default::default();
|
||||
///
|
||||
/// // Good
|
||||
/// let s = String::default();
|
||||
/// ```
|
||||
pub DEFAULT_TRAIT_ACCESS,
|
||||
pedantic,
|
||||
"checks for literal calls to `Default::default()`"
|
||||
}
|
||||
|
||||
declare_lint_pass!(DefaultTraitAccess => [DEFAULT_TRAIT_ACCESS]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for DefaultTraitAccess {
|
||||
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
|
||||
if_chain! {
|
||||
if let ExprKind::Call(ref path, ..) = expr.kind;
|
||||
if !any_parent_is_automatically_derived(cx.tcx, expr.hir_id);
|
||||
if let ExprKind::Path(ref qpath) = path.kind;
|
||||
if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id();
|
||||
if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
|
||||
// Detect and ignore <Foo as Default>::default() because these calls do explicitly name the type.
|
||||
if let QPath::Resolved(None, _path) = qpath;
|
||||
then {
|
||||
let expr_ty = cx.typeck_results().expr_ty(expr);
|
||||
if let ty::Adt(def, ..) = expr_ty.kind() {
|
||||
// TODO: Work out a way to put "whatever the imported way of referencing
|
||||
// this type in this file" rather than a fully-qualified type.
|
||||
let replacement = format!("{}::default()", cx.tcx.def_path_str(def.did));
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
DEFAULT_TRAIT_ACCESS,
|
||||
expr.span,
|
||||
&format!("calling `{}` is more clear than this expression", replacement),
|
||||
"try",
|
||||
replacement,
|
||||
Applicability::Unspecified, // First resolve the TODO above
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -176,7 +176,7 @@ mod copies;
|
||||
mod copy_iterator;
|
||||
mod create_dir;
|
||||
mod dbg_macro;
|
||||
mod default_trait_access;
|
||||
mod default;
|
||||
mod dereference;
|
||||
mod derive;
|
||||
mod disallowed_method;
|
||||
@ -537,7 +537,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
©_iterator::COPY_ITERATOR,
|
||||
&create_dir::CREATE_DIR,
|
||||
&dbg_macro::DBG_MACRO,
|
||||
&default_trait_access::DEFAULT_TRAIT_ACCESS,
|
||||
&default::DEFAULT_TRAIT_ACCESS,
|
||||
&default::FIELD_REASSIGN_WITH_DEFAULT,
|
||||
&dereference::EXPLICIT_DEREF_METHODS,
|
||||
&derive::DERIVE_HASH_XOR_EQ,
|
||||
&derive::DERIVE_ORD_XOR_PARTIAL_ORD,
|
||||
@ -1049,7 +1050,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
store.register_late_pass(|| box neg_cmp_op_on_partial_ord::NoNegCompOpForPartialOrd);
|
||||
store.register_late_pass(|| box unwrap::Unwrap);
|
||||
store.register_late_pass(|| box duration_subsec::DurationSubsec);
|
||||
store.register_late_pass(|| box default_trait_access::DefaultTraitAccess);
|
||||
store.register_late_pass(|| box indexing_slicing::IndexingSlicing);
|
||||
store.register_late_pass(|| box non_copy_const::NonCopyConst);
|
||||
store.register_late_pass(|| box ptr_offset_with_cast::PtrOffsetWithCast);
|
||||
@ -1100,6 +1100,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
let enum_variant_name_threshold = conf.enum_variant_name_threshold;
|
||||
store.register_early_pass(move || box enum_variants::EnumVariantNames::new(enum_variant_name_threshold));
|
||||
store.register_early_pass(|| box tabs_in_doc_comments::TabsInDocComments);
|
||||
store.register_late_pass(|| box default::Default::default());
|
||||
store.register_late_pass(|| box unused_self::UnusedSelf);
|
||||
store.register_late_pass(|| box mutable_debug_assertion::DebugAssertWithMutCall);
|
||||
store.register_late_pass(|| box exit::Exit);
|
||||
@ -1212,7 +1213,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
LintId::of(&checked_conversions::CHECKED_CONVERSIONS),
|
||||
LintId::of(&copies::SAME_FUNCTIONS_IN_IF_CONDITION),
|
||||
LintId::of(©_iterator::COPY_ITERATOR),
|
||||
LintId::of(&default_trait_access::DEFAULT_TRAIT_ACCESS),
|
||||
LintId::of(&default::DEFAULT_TRAIT_ACCESS),
|
||||
LintId::of(&dereference::EXPLICIT_DEREF_METHODS),
|
||||
LintId::of(&derive::EXPL_IMPL_CLONE_ON_COPY),
|
||||
LintId::of(&derive::UNSAFE_DERIVE_DESERIALIZE),
|
||||
@ -1321,6 +1322,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
LintId::of(&comparison_chain::COMPARISON_CHAIN),
|
||||
LintId::of(&copies::IFS_SAME_COND),
|
||||
LintId::of(&copies::IF_SAME_THEN_ELSE),
|
||||
LintId::of(&default::FIELD_REASSIGN_WITH_DEFAULT),
|
||||
LintId::of(&derive::DERIVE_HASH_XOR_EQ),
|
||||
LintId::of(&derive::DERIVE_ORD_XOR_PARTIAL_ORD),
|
||||
LintId::of(&doc::MISSING_SAFETY_DOC),
|
||||
@ -1581,6 +1583,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
||||
LintId::of(&blocks_in_if_conditions::BLOCKS_IN_IF_CONDITIONS),
|
||||
LintId::of(&collapsible_if::COLLAPSIBLE_IF),
|
||||
LintId::of(&comparison_chain::COMPARISON_CHAIN),
|
||||
LintId::of(&default::FIELD_REASSIGN_WITH_DEFAULT),
|
||||
LintId::of(&doc::MISSING_SAFETY_DOC),
|
||||
LintId::of(&doc::NEEDLESS_DOCTEST_MAIN),
|
||||
LintId::of(&enum_variants::ENUM_VARIANT_NAMES),
|
||||
|
||||
@ -359,7 +359,7 @@ vec![
|
||||
group: "pedantic",
|
||||
desc: "checks for literal calls to `Default::default()`",
|
||||
deprecation: None,
|
||||
module: "default_trait_access",
|
||||
module: "default",
|
||||
},
|
||||
Lint {
|
||||
name: "deprecated_cfg_attr",
|
||||
@ -627,6 +627,13 @@ vec![
|
||||
deprecation: None,
|
||||
module: "fallible_impl_from",
|
||||
},
|
||||
Lint {
|
||||
name: "field_reassign_with_default",
|
||||
group: "style",
|
||||
desc: "binding initialized with Default should have its fields set in the initializer",
|
||||
deprecation: None,
|
||||
module: "default",
|
||||
},
|
||||
Lint {
|
||||
name: "filetype_is_file",
|
||||
group: "restriction",
|
||||
|
||||
110
tests/ui/field_reassign_with_default.rs
Normal file
110
tests/ui/field_reassign_with_default.rs
Normal file
@ -0,0 +1,110 @@
|
||||
#![warn(clippy::field_reassign_with_default)]
|
||||
|
||||
#[derive(Default)]
|
||||
struct A {
|
||||
i: i32,
|
||||
j: i64,
|
||||
}
|
||||
|
||||
struct B {
|
||||
i: i32,
|
||||
j: i64,
|
||||
}
|
||||
|
||||
/// Implements .next() that returns a different number each time.
|
||||
struct SideEffect(i32);
|
||||
|
||||
impl SideEffect {
|
||||
fn new() -> SideEffect {
|
||||
SideEffect(0)
|
||||
}
|
||||
fn next(&mut self) -> i32 {
|
||||
self.0 += 1;
|
||||
self.0
|
||||
}
|
||||
}
|
||||
|
||||
fn main() {
|
||||
// wrong, produces first error in stderr
|
||||
let mut a: A = Default::default();
|
||||
a.i = 42;
|
||||
|
||||
// right
|
||||
let mut a: A = Default::default();
|
||||
|
||||
// right
|
||||
let a = A {
|
||||
i: 42,
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
// right
|
||||
let mut a: A = Default::default();
|
||||
if a.i == 0 {
|
||||
a.j = 12;
|
||||
}
|
||||
|
||||
// right
|
||||
let mut a: A = Default::default();
|
||||
let b = 5;
|
||||
|
||||
// right
|
||||
let mut b = 32;
|
||||
let mut a: A = Default::default();
|
||||
b = 2;
|
||||
|
||||
// right
|
||||
let b: B = B { i: 42, j: 24 };
|
||||
|
||||
// right
|
||||
let mut b: B = B { i: 42, j: 24 };
|
||||
b.i = 52;
|
||||
|
||||
// right
|
||||
let mut b = B { i: 15, j: 16 };
|
||||
let mut a: A = Default::default();
|
||||
b.i = 2;
|
||||
|
||||
// wrong, produces second error in stderr
|
||||
let mut a: A = Default::default();
|
||||
a.j = 43;
|
||||
a.i = 42;
|
||||
|
||||
// wrong, produces third error in stderr
|
||||
let mut a: A = Default::default();
|
||||
a.i = 42;
|
||||
a.j = 43;
|
||||
a.j = 44;
|
||||
|
||||
// wrong, produces fourth error in stderr
|
||||
let mut a = A::default();
|
||||
a.i = 42;
|
||||
|
||||
// wrong, but does not produce an error in stderr, because we can't produce a correct kind of
|
||||
// suggestion with current implementation
|
||||
let mut c: (i32, i32) = Default::default();
|
||||
c.0 = 42;
|
||||
c.1 = 21;
|
||||
|
||||
// wrong, produces the fifth error in stderr
|
||||
let mut a: A = Default::default();
|
||||
a.i = Default::default();
|
||||
|
||||
// wrong, produces the sixth error in stderr
|
||||
let mut a: A = Default::default();
|
||||
a.i = Default::default();
|
||||
a.j = 45;
|
||||
|
||||
// right, because an assignment refers to another field
|
||||
let mut x = A::default();
|
||||
x.i = 42;
|
||||
x.j = 21 + x.i as i64;
|
||||
|
||||
// right, we bail out if there's a reassignment to the same variable, since there is a risk of
|
||||
// side-effects affecting the outcome
|
||||
let mut x = A::default();
|
||||
let mut side_effect = SideEffect::new();
|
||||
x.i = side_effect.next();
|
||||
x.j = 2;
|
||||
x.i = side_effect.next();
|
||||
}
|
||||
75
tests/ui/field_reassign_with_default.stderr
Normal file
75
tests/ui/field_reassign_with_default.stderr
Normal file
@ -0,0 +1,75 @@
|
||||
error: field assignment outside of initializer for an instance created with Default::default()
|
||||
--> $DIR/field_reassign_with_default.rs:30:5
|
||||
|
|
||||
LL | a.i = 42;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
= note: `-D clippy::field-reassign-with-default` implied by `-D warnings`
|
||||
note: consider initializing the variable with `A { i: 42, ..Default::default() }` and removing relevant reassignments
|
||||
--> $DIR/field_reassign_with_default.rs:29:5
|
||||
|
|
||||
LL | let mut a: A = Default::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: field assignment outside of initializer for an instance created with Default::default()
|
||||
--> $DIR/field_reassign_with_default.rs:70:5
|
||||
|
|
||||
LL | a.j = 43;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
note: consider initializing the variable with `A { j: 43, i: 42 }` and removing relevant reassignments
|
||||
--> $DIR/field_reassign_with_default.rs:69:5
|
||||
|
|
||||
LL | let mut a: A = Default::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: field assignment outside of initializer for an instance created with Default::default()
|
||||
--> $DIR/field_reassign_with_default.rs:75:5
|
||||
|
|
||||
LL | a.i = 42;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
note: consider initializing the variable with `A { i: 42, j: 44 }` and removing relevant reassignments
|
||||
--> $DIR/field_reassign_with_default.rs:74:5
|
||||
|
|
||||
LL | let mut a: A = Default::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: field assignment outside of initializer for an instance created with Default::default()
|
||||
--> $DIR/field_reassign_with_default.rs:81:5
|
||||
|
|
||||
LL | a.i = 42;
|
||||
| ^^^^^^^^^
|
||||
|
|
||||
note: consider initializing the variable with `A { i: 42, ..Default::default() }` and removing relevant reassignments
|
||||
--> $DIR/field_reassign_with_default.rs:80:5
|
||||
|
|
||||
LL | let mut a = A::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: field assignment outside of initializer for an instance created with Default::default()
|
||||
--> $DIR/field_reassign_with_default.rs:91:5
|
||||
|
|
||||
LL | a.i = Default::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
note: consider initializing the variable with `A::default()` and removing relevant reassignments
|
||||
--> $DIR/field_reassign_with_default.rs:90:5
|
||||
|
|
||||
LL | let mut a: A = Default::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: field assignment outside of initializer for an instance created with Default::default()
|
||||
--> $DIR/field_reassign_with_default.rs:95:5
|
||||
|
|
||||
LL | a.i = Default::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
|
||||
note: consider initializing the variable with `A { j: 45, ..Default::default() }` and removing relevant reassignments
|
||||
--> $DIR/field_reassign_with_default.rs:94:5
|
||||
|
|
||||
LL | let mut a: A = Default::default();
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
|
||||
error: aborting due to 6 previous errors
|
||||
|
||||
Loading…
Reference in New Issue
Block a user