chore: enforce clippy::allow_attributes for optimizer and macros#19310
chore: enforce clippy::allow_attributes for optimizer and macros#19310Jefffrey merged 4 commits intoapache:mainfrom
clippy::allow_attributes for optimizer and macros#19310Conversation
github-actions
bot
added
optimizer
| child_filter.input, | ||
| )?); | ||
| #[allow(clippy::used_underscore_binding)] | ||
| #[expect(clippy::used_underscore_binding)] |
There was a problem hiding this comment.
Can we fix the code here to not need this expect?
|
|
||
| // recursively reapply the rule on the new plan | ||
| #[allow(clippy::used_underscore_binding)] | ||
| #[expect(clippy::used_underscore_binding)] |
| #[derive(Debug, StableAbi)] | ||
| #[allow(non_camel_case_types)] | ||
| #[expect(clippy::large_enum_variant)] | ||
| #[allow(clippy::large_enum_variant)] |
There was a problem hiding this comment.
For FFI I think we're missing the deny at the lib.rs level
There was a problem hiding this comment.
When I add #![deny(clippy::allow_attributes)] in ffi, I get several unfulfilled expectations, so we’d need to also enable the relevant lints (#![warn(non_camel_case_types, clippy::type_complexity, clippy::disallowed_methods, clippy::large_enum_variant)]) so the #[expect]s actually fire. Is there any alternative you have in mind?
There was a problem hiding this comment.
So from what I've seen, the main problem is that expect doesn't play well with non_camel_case_types? I haven't had time to look into it properly, but we'll definitely want to figure something out for this instead of omitting the #![deny(clippy::allow_attributes)] in ffi entirely. Maybe for this PR we can exclude ffi until we figure out a proper solution.
There was a problem hiding this comment.
I will revert the changes for the FFI from this PR and see if I can figure something out for it and open a seperate PR.
|
fyi @chakkk309 since I think you were planning to work on the FFI part too? |
Thanks for the heads up! I'm working on the FFI part, but to avoid more duplicate work, I won't submit the pr ：) |
If you have made progress on the FFI crate I can revert my changes to FFI and you can open the PR for that. |
kumarUjjawal
changed the title
clippy::allow_attributes for ffi, optimizer and macrosclippy::allow_attributes for optimizer and macros
|
Thanks @kumarUjjawal |
No worries, please feel free to take the FFI part. |
3 participants
Which issue does this PR close?
clippy::allow_attributes#18881Rationale for this change
We want to use
#[expect]in place of#[allow]on the workspace level but to get there we wanted to do this in few crates at a time before enabling it in workspace.What changes are included in this PR?
Attributes changes for the following crates:
Are these changes tested?
Yes
Are there any user-facing changes?