Skip to content

Make warn-unreachable understand exception-swallowing contextmanagers #7317

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.

Sign up for GitHub

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
Michael0x2a merged 4 commits into from
Aug 25, 2019
Merged

Make warn-unreachable understand exception-swallowing contextmanagers #7317

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b667bd1
Make warn-unreachable understand exception-swallowing contextmanagers
Michael0x2a Jul 29, 2019
24ad85f
Merge branch 'master' into refine-unreachable-checks-against-with
ilevkivskyi Aug 16, 2019
b75f886
Respond to code review; partially disable heuristics when strict-opti…
Michael0x2a Aug 25, 2019
73edaf8
Merge branch 'master' into refine-unreachable-checks-against-with
Michael0x2a Aug 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Respond to code review; partially disable heuristics when strict-opti… 
…onal is disabled
  • Loading branch information
@Michael0x2a
Michael0x2a committed Aug 25, 2019
commit b75f886789552a3d4983390e8c9aec21c36912fa
24 changes: 20 additions & 4 deletions mypy/checker.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3312,14 +3312,30 @@ def visit_with_stmt(self, s: WithStmt) -> None:
exit_ret_type = self.check_async_with_item(expr, target, s.unanalyzed_type is None)
else:
exit_ret_type = self.check_with_item(expr, target, s.unanalyzed_type is None)

# Based on the return type, determine if this context manager 'swallows'
# exceptions or not. We determine this using a heuristic based on the
# return type of the __exit__ method -- see the discussion in
# https://github.com/python/mypy/issues/7214 and the section about context managers
# in https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#conventions
# for more details.

exit_ret_type = get_proper_type(exit_ret_type)
if is_literal_type(exit_ret_type, "builtins.bool", False):
Copy link
Copy Markdown
Member

@ilevkivskyi ilevkivskyi Aug 23, 2019

Choose a reason for hiding this comment

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

Are these rules/conventions documented somewhere? I think they should be, otherwise we will just forget them (either in mypy docs, btw do we have a section about context managers?, or in typeshed). Maybe also add a link here.

Copy link
Copy Markdown
Collaborator Author

@Michael0x2a Michael0x2a Aug 25, 2019

Choose a reason for hiding this comment

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

I did add some notes about these conventions to the typeshed contribution page (at the bottom of the conventions section), but this conventions currently aren't otherwise documented.

We do mention context managers once on the Protocols page -- I'll add a note about this there in a follow-up PR.

continue
if is_literal_type(exit_ret_type, "builtins.bool", True):
exceptions_maybe_suppressed = True
elif (isinstance(exit_ret_type, Instance)
and exit_ret_type.type.fullname() == 'builtins.bool'):

if (is_literal_type(exit_ret_type, "builtins.bool", True)
or (isinstance(exit_ret_type, Instance)
and exit_ret_type.type.fullname() == 'builtins.bool'
and state.strict_optional)):
# Note: if strict-optional is disabled, this bool instance
# could actually be an Optional[bool].
exceptions_maybe_suppressed = True
Comment thread
Michael0x2a marked this conversation as resolved.

if exceptions_maybe_suppressed:
# Treat this 'with' block in the same way we'd treat a 'try: BODY; except: pass'
# block. This means control flow can continue after the 'with' even if the 'with'
# block immediately returns.
with self.binder.frame_context(can_skip=True, try_frame=True):
Comment thread
Michael0x2a marked this conversation as resolved.
self.accept(s.body)
else:
Expand Down
6 changes: 2 additions & 4 deletions mypy/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2405,10 +2405,8 @@ def remove_optional(typ: Type) -> ProperType:
return typ


def is_literal_type(typ: Type, fallback_fullname: str, value: LiteralValue) -> bool:
"""Returns 'true' if this type is a LiteralType with the given value
and underlying base fallback type.
"""
def is_literal_type(typ: ProperType, fallback_fullname: str, value: LiteralValue) -> bool:
"""Check if this type is a LiteralType with the given fallback type and value."""
if isinstance(typ, Instance) and typ.last_known_value:
typ = typ.last_known_value
if not isinstance(typ, LiteralType):
Expand Down
Loading