Skip to content
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

Support narrowing of walrus in most cases #8458

Merged
merged 2 commits into from Feb 28, 2020
Merged

Support narrowing of walrus in most cases #8458

merged 2 commits into from Feb 28, 2020

Conversation

msullivan
Copy link
Collaborator

It is a pretty simple matter of pulling out the assignment target from
the walrus. We don't bother handling things like x := (y := z) since
I can't imagine they are common enough to be worth bothering but we
could in the future if anyone cares.

Fixes #8447.

It is a pretty simple matter of pulling out the assignment target from
the walrus. We don't bother handling things like `x := (y := z)` since
I can't imagine they are common enough to be worth bothering but we
could in the future if anyone cares.

Fixes #8447.
mypy/checker.py Outdated
@@ -3923,11 +3923,11 @@ def find_isinstance_check_helper(self, node: Expression) -> Tuple[TypeMap, TypeM
return {}, None
elif is_false_literal(node):
return None, {}
elif isinstance(node, CallExpr):
elif isinstance(node, CallExpr) and len(node.args) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead add a separate 'if' statement under here that does if len(node.args) == 0: return {}, {}?

So that way, we don't have to do a bunch of extra isinstance(...) checks if the node happens to be an unrelated CallExpr.

[builtins fixtures/f_string.pyi]

if (y := x) is not None:
reveal_type(y) # N: Revealed type is 'builtins.int'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will 'x' also end up being narrowed here?

If it doesn't, I think it's probably fine for now, but we should probably file a follow-up task.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, but I think that's probably fine? I don't think it's actually idiomatic or makes sense to be using a walrus operator when the right hand side is a value.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, that's true. I guess we can just not worry about it unless somebody brings it up.

@msullivan msullivan merged commit 1e1b22d into master Feb 28, 2020
@msullivan msullivan deleted the walrus branch February 28, 2020 23:43
sthagen added a commit to sthagen/python-mypy that referenced this pull request Feb 29, 2020
Support narrowing of walrus in most cases (python#8458)
@msullivan msullivan mentioned this pull request Mar 7, 2020
msullivan added a commit that referenced this pull request Mar 9, 2020
It is a pretty simple matter of pulling out the assignment target from
the walrus. We don't bother handling things like `x := (y := z)` since
I can't imagine they are common enough to be worth bothering but we
could in the future if anyone cares.

Fixes #8447.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PEP 572 Walrus Operator Regex Assignment in Condition
2 participants