field_reassign_with_default false positive with closure capturing variable that was initialized to default

[Johan-Mi]

Summary

The field_reassign_with_default lint performs a check to make sure that the the reassignment does not involve the variable that was initialized with default(), but this check fails if the variable is captured by a closure.

Lint Name

field_reassign_with_default

Reproducer

I tried this code:

struct Collection {
    items: Vec<i32>,
    len: usize,
}

impl Default for Collection {
    fn default() -> Self {
        Self {
            items: vec![1, 2, 3],
            len: 0,
        }
    }
}

fn main() {
    let mut c = Collection::default();
    c.len = (|| c.items.len())();
}

I saw this happen:

warning: field assignment outside of initializer for an instance created with Default::default()
  --> src/main.rs:17:5
   |
17 |     c.len = (|| c.items.len())();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
note: consider initializing the variable with `Collection { len: (|| c.items.len())(), ..Default::default() }` and removing relevant reassignments
  --> src/main.rs:16:5
   |
16 |     let mut c = Collection::default();
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#field_reassign_with_default
   = note: `#[warn(clippy::field_reassign_with_default)]` on by default

I expected to see this happen:

The lint should not trigger since the expression that c.len is being reassigned to involves c, making the suggestion invalid.

Version

rustc 1.66.0 (69f9c33d7 2022-12-12)
binary: rustc
commit-hash: 69f9c33d71c871fc16ac445211281c6e7a340943
commit-date: 2022-12-12
host: x86_64-unknown-linux-gnu
release: 1.66.0
LLVM version: 15.0.2

Additional Labels

@rustbot label +I-suggestion-causes-error

[ericwu17]

Interesting bug! In the example you gave, clippy also gives the suggestion to write

-    c.len = (|| c.items.len())();
+    c.len = c.items.len();

instead (thanks to [clippy::redundant_closure_call]). After accepting this suggestion, the false positive for [field_reassign_with_default] goes away. (However, it's true that the [field_reassign_with_default] should never trigger in the first place for the example you gave)

I can imagine that in some more complicated cases, it might actually be necessary to use some field of c in a closure. Could you give a minimal example of where you'd need the closure?

[Johan-Mi]

I haven't encountered any example in my own code where it is actually necessary, but where I ran into it, it is much more concise than the functional update syntax: https://github.com/Johan-Mi/scratch-compiler/blob/a1325e4daec30408e106788a796651702acc9052/src/codegen/x86_64.rs#L54-L62 (here I would first need to create the UID generator separately, which is possible in this case but one could imagine a scenario where it didn't have any way of being constructed other than AsmProgram's default impl).