(bugfix) Issue 10100: [C1802] should not be emitted for generators.#10107
(bugfix) Issue 10100: [C1802] should not be emitted for generators.#10107Pierre-Sassoulas merged 5 commits intopylint-dev:mainfrom
Conversation
Codecov Report✅ All 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
🚀 New features to boost your workflow:
|
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
LGTM, thank you ! Can you add a changelog in doc/whatsnew/fragments, please ?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Pierre-Sassoulas
left a comment
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 <pierre.sassoulas@gmail.com> (cherry picked from commit a4e91d7)
Type of Changes
Description
Refs #10100