Lint on should_panic tests without reasons

[workingjubilee]

What it does

For a #[test] that has #[should_panic], this lint would fire unless given a reason, e.g. #[should_panic = "No such file or directory"].

Advantage

This prevents a should-panic test from accepting another reason when it fires, and there are many reasons such a test could panic.

Drawbacks

It could fire on tests that are genuinely expecting many possible panic reasons and don't care which one fires. These are probably very few in practice, but it could justify making it a "pedantic" lint.

Example

    #[should_panic]
    fn test_can_panic() {
        panic!("yup")
    }

Could be written as:

    #[should_panic = "yup"]
    fn test_can_panic() {
        panic!("yup")
    }

[tommy-gilligan]

Thank you! Love this.

[tommy-gilligan]

It could fire on tests that are genuinely expecting many possible panic reasons and don't care which one fires. These are probably very few in practice, but it could justify making it a "pedantic" lint.

imho this should be enabled by default.

I think something worth noting here is that expected = "..." does a substring match. So in the rare case that you're "expecting many possible panic reasons and don't care which one fires", it is easy to deal with (expected = "" lol).

This to me brings up a different issue that isn't really to do with Clippy at all: should there also be a strict equals version of expected = "..." in Rust?

[y21]

FWIW I chose to make it pedantic in the PR because it does seem rather strict and looks like it would warn a loooot of code (a quick github code search showed 30k instances of #[should_panic] without a reason), but I don't know if that's a great argument (I'm also not entirely sure where this would fit in otherwise).

[tommy-gilligan]

I haven't checked yet but my hunch is expected = must have been added recently? Just I also have been noticing that it is barely used and it seems to be from a lack of awareness. I'm definitely not qualified to offer authoritative guidance on whether or not it should be 'pedantic', just couldn't resist the urget to offer my 2 cents earlier. Thank you for your lovely work!

[y21]

I haven't checked yet but my hunch is expected = must have been added recently?

I did a little bit of digging and it seems this has existed for quite some time, it was added in rust-lang/rust#19560 (back then it was called #[should_fail], apparently).

Just I also have been noticing that it is barely used and it seems to be from a lack of awareness.

I agree, I feel like this is not very well known. I guess that if this was allowed by default (edit: warn-by-default*), this lint could also be useful to remind everyone that this feature exists

[tommy-gilligan]

Oh wow! I would have guessed more recent than that haha.

this lint could also be useful to remind everyone that this feature exists

Love this thinking

[workingjubilee]

A lot of people will simply be unamused by 1000 tests flickering a warning as they try to maintain a "clippy clean" codebase.

However, I think that there's reasonable justification of "the test is sufficiently risking incorrectness otherwise", and I guess I have a more relaxed relationship with clippy personally. I think this kind of lint is way more useful since it's about "real" correctness on some level instead of mere stylistic concerns, which is why I suggested it.