-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not spawn blacklisted_name lint in test context #7379
Conversation
r? @phansch (rust-highfive has picked a reviewer for you, use r? to override) |
r? @flip1995 (reassigning since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We already have some lints, that shouldn't trigger in test code. This is how we usually handle those situations:
When going into a test context, save this information in the lint struct:
rust-clippy/clippy_lints/src/wildcard_imports.rs
Lines 108 to 111 in 48fa1dc
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) { | |
if is_test_module_or_function(item) { | |
self.test_modules_deep = self.test_modules_deep.saturating_add(1); | |
} |
When leaving the test context:
rust-clippy/clippy_lints/src/wildcard_imports.rs
Lines 186 to 190 in 48fa1dc
fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) { | |
if is_test_module_or_function(item) { | |
self.test_modules_deep = self.test_modules_deep.saturating_sub(1); | |
} | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM overall. One small thing left. Please squash the commits after fixing that.
Fix detecting of the 'test' attribute Update UI test to actually check that warning is not triggered in the test code Fix approach for detecting the test module Add nested test case Remove code duplication by extracting 'is_test_module_or_function' into 'clippy_utils' Cleanup the code
@bors r+ Thanks! |
📌 Commit 28d3873 has been approved by |
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
fixed #7305
Please write a short comment explaining your change (or "none" for internal only changes)
changelog:
blacklisted_name
lint is not spawned in the test context anymore.