used_underscore_bindings: respect lint levels on the binding definition #11523
Conversation
|
r? @Jarcho (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
clippy_lints/src/misc.rs
Outdated
| Level::Allow => return, | ||
| Level::Expect(id) => { | ||
| cx.fulfill_expectation(id); | ||
| return; |
There was a problem hiding this comment.
This check might miss a second expectation, if there is also one on the statement level. I think in 99.9% of the cases it should be fine, but it might be safer to also check the level at expr and potentially fulfill a second expectation. :)
Otherwise very nice check :)
There was a problem hiding this comment.
Aha makes sense, does the ForceWarn expectation ever need to be fulfilled?
There was a problem hiding this comment.
The force warn expectation should be handled exactly the same as a normal one. That one becomes relevant if there is an #[expect] and the user specifies a --force-warn in via the console arguments
There was a problem hiding this comment.
Am I understanding correctly that --force-warn foo turns all the #[expect(foo)]s into Level::ForceWarn(Some(_))?
Personally I would still expect the lint to emit if --force-warn was passed, but it could probably still fulfill the expectation to not create a new warning/error
There was a problem hiding this comment.
Yeah, sorry, I typed my answer on the go 😅. With --force-warn the lint should be emitted as normal, that will emit the lint and fulfill the lint expectation IIRC
|
r? @xFrednet |
Alexendoo
force-pushed
the
used-underscore-bindings-lint-levels
branch
from
c0b5f07 to
32d3387
Compare
|
LGTM, thank you for the update. I really like the Looking forward to have this fix in the next rustup of Marker :D @bors r+ |
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
Fixes #11520
Fixes #947
Also ignores usages of
PhantomDatachangelog: none