-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Make str unpacking check opt-in #15511
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -416,3 +416,24 @@ Example: | |
# The following will not generate an error on either | ||
# Python 3.8, or Python 3.9 | ||
42 + "testing..." # type: ignore | ||
|
||
|
||
.. _code-str-unpacking: | ||
|
||
Check that ``str`` is explicitly unpacked [str-unpacking] | ||
--------------------------------------------------------- | ||
|
||
It can sometimes be surprising that ``str`` is iterable, especially when unpacking. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Explicitly enumerate the contexts where this matters (for loop, assignment)? |
||
|
||
Example: | ||
|
||
.. code-block:: python | ||
|
||
# Use "mypy --enable-error-code str-unpacking ..." | ||
|
||
def print_dict(d: dict[str, str]) -> int: | ||
# We meant to do d.items(), but instead we're unpacking the str keys of d | ||
|
||
# Error: Unpacking a string is disallowed | ||
for k, v in d: | ||
print(k, v) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3491,7 +3491,11 @@ def check_multi_assignment( | |
self.check_multi_assignment_from_union( | ||
lvalues, rvalue, rvalue_type, context, infer_lvalue_type | ||
) | ||
elif isinstance(rvalue_type, Instance) and rvalue_type.type.fullname == "builtins.str": | ||
elif ( | ||
self.msg.errors.is_error_code_enabled(codes.STR_UNPACKING) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this included in |
||
and isinstance(rvalue_type, Instance) | ||
and rvalue_type.type.fullname == "builtins.str" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will this check unpacking on Literal strings? Can you add a testcase for that please? |
||
): | ||
self.msg.unpacking_strings_disallowed(context) | ||
else: | ||
self.check_multi_assignment_from_iterable( | ||
|
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.
The title is a bit unclear. What about something like "Check that 'str' is not unpacked?". Or "... is not unpacked implicitly", though I'm not sure what's the difference between explicit and implicit unpacking here.
Bikeshedding: what about renaming the error code to
str-unpack
, which would be a bit shorter?