Skip to content

Commit

Permalink
Refactor and prettify
Browse files Browse the repository at this point in the history
  • Loading branch information
Henri Lunnikivi committed Sep 5, 2020
1 parent b92fac4 commit b9ac8b5
Showing 1 changed file with 79 additions and 60 deletions.
139 changes: 79 additions & 60 deletions clippy_lints/src/field_reassign_with_default.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::utils::{match_def_path, paths, qpath_res, snippet, span_lint_and_note};
use if_chain::if_chain;
use rustc_hir::def::Res;
use rustc_hir::{Block, ExprKind, PatKind, QPath, StmtKind};
use rustc_span::symbol::Symbol;
use rustc_hir::{Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind};
use rustc_span::symbol::{Ident, Symbol};

use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
Expand All @@ -11,7 +11,7 @@ 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 [functional update syntax](https://doc.rust-lang.org/reference/expressions/struct-expr.html#functional-update-syntax).
/// **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:** None.
///
Expand Down Expand Up @@ -39,36 +39,13 @@ declare_lint_pass!(FieldReassignWithDefault => [FIELD_REASSIGN_WITH_DEFAULT]);

impl LateLintPass<'_> for FieldReassignWithDefault {
fn check_block(&mut self, cx: &LateContext<'_>, block: &Block<'_>) {
// store statement index and name of binding for all statements like
// `let mut binding = Default::default();`
let binding_statements_using_default: Vec<(usize, Symbol)> = 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;
// only when assigning `... = Default::default()`
if let Some(ref expr) = local.init;
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);
// right hand side of assignment is `Default::default`
if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
then {
Some((idx, ident.name))
}
else {
None
}
}
})
.collect::<Vec<_>>();

// start from the `let mut binding = Default::default();` and look at all the following
// 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: Vec<(usize, Symbol)> = 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) in binding_statements_using_default {
// last statement of block cannot trigger the lint
Expand All @@ -80,40 +57,24 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
// Default::default() get reassigned
let mut first_assign = None;
let mut assigned_fields = vec![];
for post_defassign_stmt in &block.stmts[stmt_idx + 1..] {
for consequtive_statement in &block.stmts[stmt_idx + 1..] {
// interrupt if the statement is a let binding (`Local`) that shadows the original
// binding
if let StmtKind::Local(local) = &post_defassign_stmt.kind {
if let PatKind::Binding(_, _, ident, _) = local.pat.kind {
if ident.name == binding_name {
break;
}
}
if stmt_shadows_binding(consequtive_statement, &binding_name) {
break;
}
// statement kinds other than `StmtKind::Local` are valid, because they cannot
// shadow a binding
else {
if_chain! {
// only take assignments
if let StmtKind::Semi(ref later_expr) = post_defassign_stmt.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, later_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 {
// extract and store the name of the field and the assigned value for help message
let field = later_field_ident.name;
let value_snippet = snippet(cx, assign_rhs.span, "..");
assigned_fields.push((field, value_snippet));
// find out if and which field was set by this `consequtive_statement`
if let Some((field_ident, assign_rhs)) = field_reassigned_by_stmt(consequtive_statement, &binding_name) {
// extract and store the assigned value for help message
let value_snippet = snippet(cx, assign_rhs.span, "..");
assigned_fields.push((field_ident.name, value_snippet));

// also set first instance of error for help message
if first_assign.is_none() {
first_assign = Some(post_defassign_stmt);
}
// also set first instance of error for help message
if first_assign.is_none() {
first_assign = Some(consequtive_statement);
}
}
}
Expand All @@ -122,7 +83,7 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
// if there are incorrectly assigned fields, do a span_lint_and_note to suggest
// immutable construction using `Ty { fields, ..Default::default() }`
if !assigned_fields.is_empty() {
// take the preceding statement as span
// take the original assignment as span
let stmt = &block.stmts[stmt_idx];
if let StmtKind::Local(preceding_local) = &stmt.kind {
if let Some(ty) = &preceding_local.ty {
Expand Down Expand Up @@ -151,3 +112,61 @@ impl LateLintPass<'_> for FieldReassignWithDefault {
}
}
}

fn enumerate_bindings_using_default(cx: &LateContext<'_>, block: &Block<'_>) -> Vec<(usize, Symbol)> {
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;
// only when assigning `... = Default::default()`
if let Some(ref expr) = local.init;
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);
// right hand side of assignment is `Default::default`
if match_def_path(cx, def_id, &paths::DEFAULT_TRAIT_METHOD);
then {
Some((idx, ident.name))
}
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<'hir>(this: &Stmt<'hir>, binding_name: &Symbol) -> Option<(Ident, &'hir Expr<'hir>)> {
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
}
}
}

0 comments on commit b9ac8b5

Please sign in to comment.