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

Make None compatible with SupportsBool protocol #15889

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

blablatdinov
Copy link

@blablatdinov blablatdinov commented Aug 16, 2023

None had special case for __str__ and __hash__, but it can be extened for __bool__ as well for supporting next code:

from typing import Protocol

class SupportsBool(Protocol):
    def __bool__(self) -> bool:
        ...
        
x: SupportsBool = None
>>> bool(None)
False

https://mypy-play.net/?mypy=latest&python=3.11&gist=42d8731eb61fd5b1e9150b693375015f

@TeamSpen210
Copy link
Contributor

Does this check the return type? None should probably be incompatible with -> Literal[True], but compatible with bool or Literal[False].

@blablatdinov
Copy link
Author

@TeamSpen210 Hi! No, this changes only add supports "Boolable" protocols for None

@github-actions

This comment has been minimized.

@TeamSpen210
Copy link
Contributor

TeamSpen210 commented Aug 16, 2023

I mean if you create a __bool__ protocol with a literal return type, that would be useful to distinguish between types that are always falsey. None shouldn’t match def __bool__(self) -> Literal[True]:.

@blablatdinov
Copy link
Author

__bool__ method not support Literal[False] return type.

note:     Expected:
note:         def __bool__(self) -> Literal[False]
note:     Got:
note:         def __bool__(self) -> bool

@@ -2890,6 +2890,12 @@ class SupportsStr(Protocol):
def ss(s: SupportsStr) -> None: pass
ss(None)

class SupportsBool(Protocol):
def __bool__(self) -> bool: ...
Copy link
Member

Choose a reason for hiding this comment

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

Please, add two more protocols to the test: def __bool__(self) -> Literal[True]: ... and def __bool__(self) -> Literal[False]: ...

Other than that - it looks good to me.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for review! I add tests with Literal return types, take a look, please


def sb(s: SupportsBool) -> None: pass
sb(None)
def sblt(s: SupportsBoolLiteralTrue) -> None: pass
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Yes, you're right, it's not correct behavior, but tests passed. How I can fix it? Maybe we have other bug?

Copy link
Member

Choose a reason for hiding this comment

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

None is quite complicated, because mypy uses pre-types.NoneType logic for None.
Check out checkmember and NoneType handling there.

Copy link
Author

Choose a reason for hiding this comment

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

I'm implemented check into visit_none_type method, because __bool__ of Protocol not handled by analyze_none_member_access

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

# None is also compatible with `SupportsStr` protocol.
# None is also compatible with `SupportsStr` and `SupportsBool` protocols.
if self.right.type.defn.info and "__bool__" in self.right.type.defn.info.names:
bool_method = self.right.type.defn.info.names["__bool__"]
Copy link
Member

Choose a reason for hiding this comment

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

I think that instead we should try changing None to have def __bool__(self) -> Literal[False] signature.

Copy link
Author

Choose a reason for hiding this comment

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

You mean the mypy.types.NoneType?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, the member __bool__ of NoneType

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.

3 participants