assertions_on_result_states leads to less clear tests

[Nemo157]

Summary

IMO using assert!(foo().is_err()) is clearer within a test to imply that the assertion is the point of the test rather than foo().unwrap_err().

Lint Name

assertions_on_result_states

Reproducer

I tried this code:

    #[test]
    fn error() {
        struct Foo;

        impl stylish::Display for Foo {
            fn fmt(&self, _: &mut stylish::Formatter<'_>) -> std::fmt::Result {
                Err(std::fmt::Error)
            }
        }

        let mut s = Vec::<u8>::new();
        let mut writer = stylish::io::Plain::new(&mut s);
        assert!(stylish::writeln!(writer, "{:s}", Foo).is_err());
    }

I saw this happen:

error: called `assert!` with `Result::is_err`
Error:   --> tests/tests.rs:70:9
   |
70 |         assert!(stylish::writeln!(writer, "{:s}", Foo).is_err());
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `stylish::writeln!(writer, "{:s}", Foo).unwrap_err()`
   |
   = note: `-D clippy::assertions-on-result-states` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#assertions_on_result_states

I expected to see this happen:

Version

info: syncing channel updates for 'nightly-x86_64-unknown-linux-gnu'
info: latest update on 2022-07-30, rust version 1.64.0-nightly (3924dac7b 2022-07-29)

Additional Labels

No response

[Nemo157]

Alternatively, in this sort of situation maybe it'd be better to recommend .expect_err(..).

[Jarcho]

Assuming stylish::writeln has a unit type for Result::Ok then the reasoning for the lint doesn't apply.

It might also be worth putting the lint is restriction as well. assert will print the failing expression which might be more useful than printing the debug output of the value.

[Nemo157]

Assuming stylish::writeln has a unit type for Result::Ok

It is indeed, Result<(), std::fmt::Error>.

For comparison here's the two outputs if the test fails:

assert!(stylish::writeln!(writer, "{:s}", Foo).is_err());
// 'assertion failed: stylish::writeln!(writer, \"{:s}\", Foo).is_err()', tests/tests.rs:71:9

stylish::writeln!(writer, "{:s}", Foo).unwrap_err();
// 'called `Result::unwrap_err()` on an `Ok` value: ()', tests/tests.rs:71:48

[tabokie]

Disabling it here: #9273