-
Notifications
You must be signed in to change notification settings - Fork 31
Create rule S7524 async for
should be used with asynchronous iterators in async
functions
#5116
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
base: master
Are you sure you want to change the base?
Conversation
…tors in `async` functions
2a099cc
to
2b398e9
Compare
async for
should be used with asynchronous iterators in async
functions
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.
I'm not sure about the overall message of the rule, to me we should be very close to S7515.
rules/S7524/python/rule.adoc
Outdated
@@ -0,0 +1,112 @@ | |||
This rule raises an issue when a synchronous `for` loop is used with an iterator implementing both synchronous and asynchronous iteration protocols. |
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.
I'm a bit confused why here (and throughout the rule description), we are focusing on iterators implementing both sync/async protocols.
I kind of expected a similar wording as S7515, where the message would be "if the iterator is async and we're in an async function, you should use the async protocol", no matter whether both protocols are implemented or not.
Here it seems we are saying that async for
should be used even outside of async
functions, which doesn't feel right (to be fair, we mention async functions in the "How to fix it" tab, but it's a bit late IMO).
rules/S7524/python/rule.adoc
Outdated
|
||
=== Message | ||
|
||
Use 'async for' to iterate over dual-protocol iterators in 'async' functions. |
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.
Do we really need to focus on dual protocol iterators as well?
I guess using a simple for
on an async iterator in an async function should be enough to raise, no? No need to ensure the iterator is dual? (I guess the "dual" part might influence whether we have a bug or a code smell, but that's it?).
I have reworded closer to S7515 |
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.
LGTM! I have just added a small comment on the severity
rules/S7524/python/metadata.json
Outdated
"tags": [ | ||
"async" | ||
], | ||
"defaultSeverity": "Major", |
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.
I think with Low the severity should be Minor
|
|
You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for: