Skip to content

Allow assignment to an empty tuple #5617

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

Merged
merged 7 commits into from
Oct 18, 2020
Merged

Conversation

tyehle
Copy link
Contributor

@tyehle tyehle commented Sep 13, 2018

Fixes #5594. Simply removes the check in the semantic analyzer. I made a best guess on what to do about the tests.

@@ -373,11 +373,6 @@ main:1: error: can't assign to literal
[out]
main:1: error: can't assign to literal

[case testInvalidLvalues5]
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this still be an error saying 1 is not iterable or similar?

Also, I would like to see a new test that does things like () = [] or a, () = list() and asserts that there are no errors.

@@ -47,6 +47,14 @@ MypyFile:1(
NameExpr(f [__main__.f])
Args())))

[case testAssignEmptyTuple]
Copy link
Member

Choose a reason for hiding this comment

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

Not sure we need this test; these tests that print out internal data structures are harder to maintain and harder to map to user-visible behavior.

@gvanrossum
Copy link
Member

@tyehle This still looks like an easy win -- but it does seem reasonable to ask for some test that indeed works. Also note that in Python 2, assignment to () is not legal -- the parser generates the SyntaxError so a simple test that this in fact is rejected in Python 2 would make sense.

@tyehle
Copy link
Contributor Author

tyehle commented Sep 30, 2018

Hmm. I did some more tests, and it looks like () = [] produces a syntax error all the way up through python 3.5. Is I'm not sure how to write tests that check for different behavior on different versions of python. Is this even possible?

@JelleZijlstra
Copy link
Member

You can pass mypy's --python-version flag in the test by putting # flags: --python-version 3.6 as the first line.

@msullivan
Copy link
Collaborator

@tyehle do you think you'll have a chance to return to this?

@tyehle
Copy link
Contributor Author

tyehle commented Jan 3, 2019

@msullivan I forgot about this. I think I'll take another crack at it :)

@tyehle
Copy link
Contributor Author

tyehle commented Jan 3, 2019

I can't figure out how to make these two tests pass

() = 1  # E: 'builtins.int' object is not iterable
[typing fixtures/typing-full.pyi]

I don't know what is required to make the tests realize that int isn't an instance of Iterable. It throws KeyError: 'typing' which I suspected was related to the way the tests know about the typing module, or possibly the fact that the function type_is_iterable is checking is_subtype(type, self.named_generic_type('typing.Iterable', ....

# flags: --python-version 3.5
() = []  # E: can't assign to ()

The test passing with python 3.5 makes sense because I removed that check. I'm honestly not sure why it correctly produces an error when given --python-version 2.7.

@msullivan
Copy link
Collaborator

Sorry, I missed your comments here and dropped this.

When does it throw KeyError: 'typing'? If you try to run that test after your change?

@tyehle
Copy link
Contributor Author

tyehle commented Feb 5, 2019

Yeah it throws KeyError: 'typing' after the change. I would expect it to throw an error complaining that ints aren't iterable instead.

@msullivan
Copy link
Collaborator

I apologize that I took a year and a half to look at this again. Adding a builtins helped with the stupid typing thing. We run a lot of tests in a pretty broken mypy environment for speed, unfortunately. I moved the tests back into an existing file.

Thanks.

@msullivan msullivan merged commit 48f2b10 into python:master Oct 18, 2020
@tyehle tyehle deleted the assign-empty-tuple branch November 13, 2020 22:59
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.

Can't assign to ()
4 participants