(bugfix) Issue 10100: [C1802] should not be emitted for generators. #10107
Conversation
Pierre-Sassoulas
added
False Positive π¦
Codecov ReportAll modified and coverable lines are covered by tests β
Additional details and impacted files@@ Coverage Diff @@
## main #10107 +/- ##
==========================================
- Coverage 95.80% 95.80% -0.01%
==========================================
Files 174 174
Lines 18973 18972 -1
==========================================
- Hits 18177 18176 -1
Misses 796 796
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Closing #10100 should be done in a follow-up MR so we can have it in 4.0 and not when backporting this FP fix in 3.3.3. Which is why I'm approving / merging as is.
| Fix a false positive for `use-implicit-booleaness-not-len`. No lint should be emitted for | ||
| generators (`len` is not defined for generators). | ||
|
|
||
| Closes #10100 |
There was a problem hiding this comment.
| Closes #10100 | |
| Refs #10100 |
I feel like to close #10100 a message should be raised about the type error (add the if isinstance(len_arg, (nodes.GeneratorExp)): condition and raise the proper message in this case), because nothing is raised at the moment. But I don't know what to use in the existing list of message (probably should create a new message like "invalid-len-call" ?). What do everyone think ?
There was a problem hiding this comment.
We could certainly perform the check within the current ImplicitBooleanessChecker, but I believe it would make more sense to create a dedicated checker for invalid uses of len. The reason for this is that raising an invalid-len-call message in a class designed for ImplicitBooleaness might lead to confusion or unnecessary coupling of unrelated concerns. By creating a separate checker, we can more clearly handle cases in which len is used improperly while keeping the scope of the existing checker focused on its intended task.
There was a problem hiding this comment.
Should I implement the new checker, maybe adding it to pylint/checkers with the name invalid_calls.py?
There was a problem hiding this comment.
Thank you for proposing. It's an error level message so it could should (?) be in a builtin checker really (BasicErrorChecker maybe ?). What do @jacobtylerwalls or @DanielNoord think ?
|
π€ According to the primer, this change has no effect on the checked open source code. π€π This comment was generated for commit 2432c93 |
|
Congratulation on becoming a pylint contributor ;) ! |
Co-authored-by: Nedelcu <Nedelcu Ioan-Andrei> Co-authored-by: Pierre Sassoulas <[email protected]> (cherry picked from commit a4e91d7)
#10109) Co-authored-by: Nedelcu <Nedelcu Ioan-Andrei> Co-authored-by: Pierre Sassoulas <[email protected]> (cherry picked from commit a4e91d7) Co-authored-by: Nedelcu Ioan-Andrei <[email protected]>
Type of Changes
Description
Refs #10100