Add empty_test lint - #17626
Conversation
rustbot
added
the
S-waiting-on-community-reviews
|
Thanks for the pull request, and welcome! You should hear from one of our reviewers after this PR gets at least 2 reviews from the community. Please see the contribution instructions for more information. |
rustbot
added
needs-fcp
47c2516 to
26a7bd7
Compare
|
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Empty test functions execute no test code and cannot verify behaviour. Provide an opt-in restriction lint so projects can detect these no-op tests while allowing temporary placeholders where appropriate.
7e53400 to
d672949
Compare
There was a problem hiding this comment.
Community review
From the issue:
This can be a more general lint for tests which can never fail or always fail. Empty tests aren't particularly unique other than being the degenerate case.
I think you missed this part.
the lint I guess can start out with only linting for the degenerate case, but it should have the ambition to also lint for the provably true/false cases..
🤔
unless..
Actually I think type tests (where you ensure that for example a types size always is below a cache line for perf) cannot happen.
But that sounds a bit like an edge case
CC @Jarcho if that would be something where a "false positive" (is it one?) is good or bad?
| LL | / fn empty_with_comment() { | ||
| LL | | // This is still an empty body. | ||
| LL | | } | ||
| | |_^ |
There was a problem hiding this comment.
it would be very nice to get the #[test] attr into this span, but that sounds like some work, so this is also fine likely.
There was a problem hiding this comment.
Thanks, I agree that including #[test] in the span would make the diagnostic clearer. I looked into it, but it seems that #[test] has already been expanded by the time this late lint runs. I left the current function span as is for now. Please let me know if there is a simpler approach I have missed.
Empty test functions pass without exercising behaviour and can create a false sense of coverage. Classify the lint as suspicious and document a concrete assertion-based replacement.
e93506a to
304373a
Compare
3 participants
Fixes #17488
Empty test functions pass automatically without asserting behavior, which can give a false sense of coverage. This adds
empty_test, a suspicious lint that detects test functions with empty bodies.changelog: [
empty_test]: add a suspicious lint for test functions with empty bodies.stderrfilecargo testpasses locallycargo dev update_lintscargo dev fmt