SONARPY-2870: Create rule S7492: List comprehension in <any/all>() prevents short-circuiting #5013
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Seppli11
force-pushed
the
rule/add-RSPEC-S7492
branch
2 times, most recently
from
286bc00 to
5499fcc
Compare
Seppli11
force-pushed
the
rule/add-RSPEC-S7492
branch
from
5499fcc to
546ce60
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Seppli11
requested review from
maksim-grebeniuk-sonarsource and
joke1196
and removed request for
maksim-grebeniuk-sonarsource
rules/S7492/python/rule.adoc
Outdated
| res_all = all([x > 2 for x in numbers]) # Noncompliant | ||
| res_any = any([x > 2 for x in numbers]) # Noncompliant |
There was a problem hiding this comment.
Here I think it is a good idea to say what is the issue. Also I would keep the example to only 1 use case either any or all. It is just an example so there is no need to be exhaustive.
rules/S7492/python/rule.adoc
Outdated
| ---- | ||
| numbers = [1, 5, 0, 10] | ||
|
|
||
| res_all = all(x > 2 for x in numbers) # Compliant: Stops at first False (0) |
There was a problem hiding this comment.
Here on the other hand I would remove the commentary about when the iteration stops, as it can be read as: it is compliant as it stop on the first False, instead of it is compliant because it allows for the short-circuiting behavior to take place.
rules/S7492/python/rule.adoc
Outdated
| * With `any(...)`, evaluation stops as soon as a truthy value is encountered | ||
| * With `all(...)`, evaluation stops as soon as a falsy value is encountered | ||
|
|
||
| This approach saves both processing time and memory, especially for large iterables or when the condition has side effects or is computationally expensive. |
There was a problem hiding this comment.
I feel this sentence should be part of Why is it an issue because you do not state why it is a problem that the short-circuiting mechanism is not available when using a list. So I would suggest move this sentence and phrase it in the way that it could be a memory and processing time loss. We could then think of removing that section if it does not make sense anymore, I leave it up to you.
Seppli11
changed the title
Seppli11
changed the title
Seppli11
force-pushed
the
rule/add-RSPEC-S7492
branch
from
ac35dc7 to
840644a
Compare
Seppli11
force-pushed
the
rule/add-RSPEC-S7492
branch
from
840644a to
adbe981
Compare
rules/S7492/python/rule.adoc
Outdated
| == Resources | ||
| === Documentation | ||
| * Python Wiki - https://wiki.python.org/moin/Generators[Generators] | ||
| * Python Doc - https://docs.python.org/3/glossary.html#term-generator[Generator Glossary Entry] |
There was a problem hiding this comment.
I did not catch this one during the first path but it should be Python Documentation
Seppli11
changed the title
|
|
You can preview this rule here (updated a few minutes after each push).
Review
A dedicated reviewer checked the rule description successfully for: