New lint: uninformative_asserts
#6258
Conversation
|
r? @llogiq (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Test error appears to be unrelated, we may have to wait for a rustup & rebase. I'll have time to look at the code this evening. |
| @@ -352,6 +353,7 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore) { | |||
| store.register_pre_expansion_pass(|| box write::Write::default()); | |||
| store.register_pre_expansion_pass(|| box attrs::EarlyAttributes); | |||
| store.register_pre_expansion_pass(|| box dbg_macro::DbgMacro); | |||
| store.register_pre_expansion_pass(|| box uninformative_asserts::UninformativeAsserts::default()); | |||
| } | ||
|
|
||
| fn check_item(&mut self, _: &EarlyContext<'_>, item: &Item) { | ||
| if item.attrs.iter().any(|attr| attr.has_name(sym!(test))) { |
There was a problem hiding this comment.
What about #[cfg(test)]? I think we may want to check for that, too.
Perhaps that could be a fn is_test_related(&[Attribute]) -> bool helper function.
| } | ||
|
|
||
| fn check_item_post(&mut self, _: &EarlyContext<'_>, item: &Item) { | ||
| if item.attrs.iter().any(|attr| attr.has_name(sym!(test))) { |
There was a problem hiding this comment.
Since this check is duplicated here, it should really be a helper.
| "`assert!` called without custom panic message", | ||
| None, | ||
| "consider adding a custom panic message", | ||
| ); |
There was a problem hiding this comment.
We may even want a suggestion like {macro}({args}, "<your panic message here>") (with suitable snippets for macro and args). This would help people if they haven't added a message before.
| /// # let foo = 0u8; | ||
| /// # let a = 0u8; | ||
| /// # let b = 0u8; | ||
| /// assert!(some_condition(foo)); |
There was a problem hiding this comment.
I think this test should have a ///# #![allow(clippy::uninformative_assert)] line pretended. Otherwise clippy-checking the doctests would fail.
|
☔ The latest upstream changes (presumably #6197) made this pull request unmergeable. Please resolve the merge conflicts. Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels: |
|
Just chiming in, but shouldn't this be a |
|
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author |
|
ping from triage @llogiq. This seems to be still waiting for review. Could you have any update on this? |
|
Been swamped with school work for the past few weeks, but I am still working on rewriting this. |
|
@giraffate I already reviewed this. Waiting for changes to review again. |
rustbot
added
S-waiting-on-author
|
ping from triage @HMPerson1. Could you have any update on this? |
|
ping from triage @HMPerson1. I'm closing this according to triage procedure, because this didn't have any update in 2 weeks from previous ping. If you have more time to continue working on this, feel free to reopen. |
giraffate
added
S-inactive-closed
Fixes #6207
changelog: New lint:
uninformative_asserts