-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Turn Linters', etc. implicit into_iter()
s into explicit rules()
#5436
Conversation
1270d62
to
d3dd397
Compare
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinux
Windows
|
}, | ||
); | ||
linter_rules_match_arms.extend(quote! { | ||
Linter::#linter => vec![#(#rule_paths,)*].into_iter(), |
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.
Nit: Not sure if a good idea but using vec
creates an allocation for every rules
call. Should we instead return a static array? I'm not sure if this is a good idea because it may force Rust to copy the whole array which is rather large.
Another alternative could be to have a static (private) array of all rules somewhere and have rules()
return a &'static [Rule]
slice.
let rule_len = rule_paths.len();
Linter::#linter => vec![#(#rule_paths,)*].into_iter(), | |
Linter::#linter => [#(#rule_paths,)*; #rule_len].into_iter(), |
I'm also okay having this as a separate PR because the code already used a vec
before.
I'll draft this and make the array change suggested by @MichaReiser across the board 👍 EDIT: Okay, maybe we can do that later on, I'm getting a mysterious macro expansion syntax error I don't have the bandwidth for right now 😂 |
Summary
As discussed on
IRCDiscord, this will make it easier for e.g. the docs generation stuff to get all rules for a linter (usingall_rules()
) instead of just non-nursery ones, and it also makes it more Explicit Is Better Than Implicit to iterate over linter rules.Grepping for
Item = Rule
reveals some remaining implicitIntoIterator
s that I didn't feel were necessarily in scope for this (and honestly, iterating over aRuleSet
makes sense).