Skip to content

Remove some "if type is None" style checks #7120

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

Conversation

Michael0x2a
Copy link
Collaborator

This pull request is a continuation of my quest to make mypy cleanly type-check under the --warn-unreachable flag.

It removes some unnecessary checks we're performing against Type-related variables. Specifically:

  1. We had some old code that checked if t.accept(TypeJoinVisitor(s)) would return None. I believe this check was added back in 2014: after auditing our type visitors, it no longer seems relevant today.

  2. We had a few unnecessary if some_instance.type is not None style checks. I believe we always set the TypeInfo when constructing Instance classes these days, so this check also seems unnecessary.

  3. Finally, we had a "if TypeInfo.bases does not actually contain Instances" style check around some protocol-related code -- but TypeInfo.bases is of type List[Instance].

This pull request is a continuation of my quest to make mypy cleanly
type-check under the `--warn-unreachable` flag.

It removes some unnecessary checks we're performing against Type-related
variables. Specifically:

1. We had some old code that checked if `t.accept(TypeJoinVisitor(s))`
   would return None. I believe this check was added back in 2014: after
   auditing our type visitors, it no longer seems relevant today.

2. We had a few unnecessary `if some_instance.type is not None` style
   checks. I believe we always set the TypeInfo when constructing Instance
   classes these days, so this check also seems unnecessary.

3. Finally, we had a "if TypeInfo.bases does not actually contain Instances"
   style check around some protocol-related code. I'm not really sure
   why, but TypeInfo.bases is annotated to be of type `List[Instance]`
   so it should be fine to remove these checks as well.
Copy link
Collaborator

@JukkaL JukkaL 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 cleaning things up! All of these changes look like real improvements.

@JukkaL JukkaL merged commit ad6b4f6 into python:master Jul 2, 2019
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