-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Add error-code for truthy-iterable
#13762
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
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0f61110 to
412a3a9
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@hauntsaninja This is related to #13686, this time for |
412a3a9 to
64065e4
Compare
This comment has been minimized.
This comment has been minimized.
64065e4 to
38ae87a
Compare
38ae87a to
d146b47
Compare
This comment has been minimized.
This comment has been minimized.
|
Pushed a new commit to disable the error-code by default. That should minimize the impact of the PR. |
|
@JelleZijlstra Would you mind taking a look? The actual code change is quite small but I think it will be an improvement never the less. The PR is ready from my POV. |
| @@ -153,6 +153,10 @@ def with_additional_msg(self, info: str) -> ErrorMessage: | |||
| FUNCTION_ALWAYS_TRUE: Final = ErrorMessage( | |||
| "Function {} could always be true in boolean context", code=codes.TRUTHY_FUNCTION | |||
| ) | |||
| ITERABLE_ALWAYS_TRUE: Final = ErrorMessage( | |||
| "{} which can always be true in boolean context. Consider using {} instead.", | |||
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.
This message reads a bit weird to me but I guess it's consistent with the other ALWAYS_TRUE messages.
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.
On its own, I agree. It gets better once the variables are inserted. E.g.
"var" has type "Iterable[str]" which can always be true in boolean context.
Consider using "Collection[str]" instead. [truthy-iterable]
docs/source/error_code_list2.rst
Outdated
| This is similar in concept to ensuring that an expression's type implements an expected interface (e.g. ``Sized``), | ||
| except that attempting to invoke an undefined method (e.g. ``__len__``) results in an error, | ||
| while attempting to evaluate an object in boolean context without a concrete implementation results in a truthy value. |
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 don't understand what this sentence is trying to say. What is "this" exactly?
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.
As best as I can tell, "this" refers to the check itself and the fact that it doesn't highlight a concrete issue but rather only a potential one.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Add a separate error code for
truthy-iterable.Iterabledoes not implement__len__, so passing aGeneratorwould always evaluate totrue. SuggestCollectionas alternative.In most cases this isn't an issue as the function is often called with
listanyway. However, the dedicated check can pinpoint subtle errors that might get unnoticed otherwise.