Skip to content

Commit

Permalink
Auto merge of #7160 - flip1995:field_reassign_macros, r=xFrednet,cams…
Browse files Browse the repository at this point in the history
…teffen

Don't trigger `field_reassign_with_default` in macros

Fixes #7155

Producing a good suggestion for this lint is already hard when no macros
are involved. With macros the lint message and the suggestion are just
confusing. Since both, producing a good suggestion and figuring out if
this pattern can be re-written inside a macro is nearly impossible, just
bail out.

changelog: [`field_reassign_with_default`] No longer triggers in macros

---

No that our reviewing queue is under control, I want to start hacking on Clippy myself again. Starting with an easy issue to get back in :)
  • Loading branch information
bors committed Jun 11, 2021
2 parents f7d09b4 + 0854f0c commit f1f5ccd
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 20 deletions.
3 changes: 1 addition & 2 deletions clippy_lints/src/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ 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::lint::in_external_macro;
use rustc_middle::ty;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::symbol::{Ident, Symbol};
Expand Down Expand Up @@ -122,7 +121,7 @@ impl LateLintPass<'_> for Default {
if let StmtKind::Local(local) = stmt.kind;
if let Some(expr) = local.init;
if !any_parent_is_automatically_derived(cx.tcx, expr.hir_id);
if !in_external_macro(cx.tcx.sess, expr.span);
if !in_macro(expr.span);
// only take bindings to identifiers
if let PatKind::Binding(_, binding_id, ident, _) = local.pat.kind;
// only when assigning `... = Default::default()`
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/field_reassign_with_default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,21 @@ struct C {
i: Vec<i32>,
j: i64,
}

#[derive(Default)]
struct D {
a: Option<i32>,
b: Option<i32>,
}

macro_rules! m {
($key:ident: $value:tt) => {{
let mut data = $crate::D::default();
data.$key = Some($value);
data
}};
}

/// Implements .next() that returns a different number each time.
struct SideEffect(i32);

Expand Down Expand Up @@ -143,6 +158,11 @@ fn main() {

let mut a: WrapperMulti<i32, i64> = Default::default();
a.i = 42;

// Don't lint in macros
m! {
a: 42
};
}

mod m {
Expand Down
36 changes: 18 additions & 18 deletions tests/ui/field_reassign_with_default.stderr
Original file line number Diff line number Diff line change
@@ -1,108 +1,108 @@
error: field assignment outside of initializer for an instance created with Default::default()
--> $DIR/field_reassign_with_default.rs:48:5
--> $DIR/field_reassign_with_default.rs:63:5
|
LL | a.i = 42;
| ^^^^^^^^^
|
= note: `-D clippy::field-reassign-with-default` implied by `-D warnings`
note: consider initializing the variable with `main::A { i: 42, ..Default::default() }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:47:5
--> $DIR/field_reassign_with_default.rs:62: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:88:5
--> $DIR/field_reassign_with_default.rs:103:5
|
LL | a.j = 43;
| ^^^^^^^^^
|
note: consider initializing the variable with `main::A { j: 43, i: 42 }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:87:5
--> $DIR/field_reassign_with_default.rs:102: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:93:5
--> $DIR/field_reassign_with_default.rs:108:5
|
LL | a.i = 42;
| ^^^^^^^^^
|
note: consider initializing the variable with `main::A { i: 42, j: 44 }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:92:5
--> $DIR/field_reassign_with_default.rs:107: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:99:5
--> $DIR/field_reassign_with_default.rs:114:5
|
LL | a.i = 42;
| ^^^^^^^^^
|
note: consider initializing the variable with `main::A { i: 42, ..Default::default() }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:98:5
--> $DIR/field_reassign_with_default.rs:113: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:109:5
--> $DIR/field_reassign_with_default.rs:124:5
|
LL | a.i = Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: consider initializing the variable with `main::A { i: Default::default(), ..Default::default() }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:108:5
--> $DIR/field_reassign_with_default.rs:123: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:113:5
--> $DIR/field_reassign_with_default.rs:128:5
|
LL | a.i = Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
note: consider initializing the variable with `main::A { i: Default::default(), j: 45 }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:112:5
--> $DIR/field_reassign_with_default.rs:127: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:135:5
--> $DIR/field_reassign_with_default.rs:150:5
|
LL | a.i = vec![1];
| ^^^^^^^^^^^^^^
|
note: consider initializing the variable with `C { i: vec![1], ..Default::default() }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:134:5
--> $DIR/field_reassign_with_default.rs:149:5
|
LL | let mut a: C = C::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: field assignment outside of initializer for an instance created with Default::default()
--> $DIR/field_reassign_with_default.rs:142:5
--> $DIR/field_reassign_with_default.rs:157:5
|
LL | a.i = true;
| ^^^^^^^^^^^
|
note: consider initializing the variable with `Wrapper::<bool> { i: true }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:141:5
--> $DIR/field_reassign_with_default.rs:156:5
|
LL | let mut a: Wrapper<bool> = Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: field assignment outside of initializer for an instance created with Default::default()
--> $DIR/field_reassign_with_default.rs:145:5
--> $DIR/field_reassign_with_default.rs:160:5
|
LL | a.i = 42;
| ^^^^^^^^^
|
note: consider initializing the variable with `WrapperMulti::<i32, i64> { i: 42, ..Default::default() }` and removing relevant reassignments
--> $DIR/field_reassign_with_default.rs:144:5
--> $DIR/field_reassign_with_default.rs:159:5
|
LL | let mut a: WrapperMulti<i32, i64> = Default::default();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down

0 comments on commit f1f5ccd

Please sign in to comment.