Skip to content

Special case None.__bool__() #5780

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 3 commits into from
Oct 15, 2018
Merged

Conversation

darabos
Copy link
Contributor

@darabos 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!

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

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")
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.
Copy link
Member

Choose a reason for hiding this comment

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

The position and content of this comment should be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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__':
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

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__':
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
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.

2 participants