Mutation in an assert statement

[alice-i-cecile]

What it does

Checks for methods that take &mut arguments inside of assert (or assert_eq) etc statements.

The mutation should be done first, and then the result should be checked.

This is probably a "suspicious" lint.

Advantage

• Clarifies the intent of the code, by making it clearer that mutation is occurring.
• Ensures that assert statements can be commented out or optimized away without breaking things.

Drawbacks

Your code will be slightly more verbose.

Example

Before:

let x = 5;

fn double(a: &mut u32) {
  a * 2
}

assert_eq!(10, double(x));
println!(x); // It's 10, because of our assertion!

After:

let x = 5;

fn double(a: &mut u32) {
  a * 2
}

double(x);

assert_eq!(10, x);
println!(x); // It's 10, because we explicitly doubled it in our control flow!

In the real world: bevyengine/bevy@4fbdc97

[samueltardieu]

You probably mean:

fn double(a: &mut u32) -> u32 {
  a *= 2;
  a
}

(including this for reference)

[lukaslueg]

fwiw, I tend to disagree with the rationale of such lint. If the mutation happens to bring the object into the asserted state, it's perfectly reasonable to mutate inside an assert. Consider

// Content-Policy processing for this request already done, don't forward it
assert!(headers.remove('X-Content-Policy').is_some())

Here, we remove an entry from the map, assert that the entry has actually been there before, and continue using that map. The advantages as stated above are actually reversed: We clearly state that we rely on the side-effect of the mutation (removal + check that the value was there before).