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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Special case None.__bool__() #5780

Merged
merged 3 commits into from Oct 15, 2018

Conversation

Projects
None yet
2 participants
@darabos
Copy link
Contributor

darabos commented Oct 12, 2018

Resolves #5680.

Sorry, I don't know if I'm doing this right. We started using Mypy about half a year ago, so I thought I'd try to make some small contribution. The comment from @ilevkivskyi made this sound simple enough, and the test passes, but I'm not entirely confident of this. There are no other hacks like this in checkmember.py...

Also I have no idea where this test belongs. Seems like a "basic" thing, so I put it in check-basic.test. 馃槄
Thanks!

@ilevkivskyi
Copy link
Collaborator

ilevkivskyi left a comment

Thanks! The idea looks right, I have few comments.

none = None
b = none.__bool__()
s = 'hi'
s = b # E: Incompatible types in assignment (expression has type "bool", variable has type "str")

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 14, 2018

Collaborator

You can just use reveal_type() to check that the type is correct.

This comment has been minimized.

@darabos

darabos Oct 15, 2018

Contributor

You can just use reveal_type() to check that the type is correct.

Done, thanks!

is_super, is_operator, builtin_type, not_ready_callback, msg,
original_type=original_type, chk=chk)
if name == '__bool__':
# The only attribute of NoneType that is not inherited from object.

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 14, 2018

Collaborator

The position and content of this comment should be updated.

This comment has been minimized.

@darabos

darabos Oct 15, 2018

Contributor

The position and content of this comment should be updated.

I've updated the position and content. Hopefully for the better. 馃槄

return analyze_member_access(name, builtin_type('builtins.object'), node, is_lvalue,
is_super, is_operator, builtin_type, not_ready_callback, msg,
original_type=original_type, chk=chk)
if name == '__bool__':

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 14, 2018

Collaborator

Note that that this special case should be added only on Python 3.

Please also add a test that None.__bool__ fails on Python 2.

This comment has been minimized.

@darabos

darabos Oct 15, 2018

Contributor

Note that that this special case should be added only on Python 3.

Oops, I'd missed that. Thanks, fixed.

Please also add a test that None.__bool__ fails on Python 2.

Done.

@ilevkivskyi
Copy link
Collaborator

ilevkivskyi left a comment

Thanks for addressing the comments! This looks almost ready, I have one last suggestion.

is_python_2 = chk.options.python_version[0] == 2
# In Python 2 "None" has exactly the same attributes as "object". Python 3 adds a single
# extra attribute, "__bool__".
if not is_python_2 and name == '__bool__':

This comment has been minimized.

@ilevkivskyi

ilevkivskyi Oct 15, 2018

Collaborator

It would read more natural if you use if is_python_3 ... here and define it as is_python_3 = chk.options.python_version[0] >= 3 above.

This comment has been minimized.

@darabos

darabos Oct 15, 2018

Contributor

It would read more natural if you use if is_python_3 ... here and define it as is_python_3 = chk.options.python_version[0] >= 3 above.

Done. Though now is_python_3 will be True on Python 4. 馃樃
Thanks!

@ilevkivskyi ilevkivskyi merged commit 3951a86 into python:master Oct 15, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment