From 18673874edb66340bb589ff5bf2f2e5065d94dd9 Mon Sep 17 00:00:00 2001 From: Jannic Warken Date: Mon, 24 Jan 2022 18:23:35 +0100 Subject: [PATCH 1/7] Report unreachable except blocks --- mypy/checker.py | 25 +++++++++++++++++ mypy/messages.py | 9 ++++++ test-data/unit/check-unreachable-code.test | 32 ++++++++++++++++++++++ 3 files changed, 66 insertions(+) diff --git a/mypy/checker.py b/mypy/checker.py index 2f99b9b4fece..da397f03e4bc 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2336,6 +2336,28 @@ def check_assignment(self, lvalue: Lvalue, rvalue: Expression, infer_lvalue_type self.infer_variable_type(inferred, lvalue, rvalue_type, rvalue) self.check_assignment_to_slots(lvalue) + def check_except_reachability(self, s: TryStmt) -> None: + """Perform except block reachability checks. + + 1. Check for duplicate exception clauses. + 2. Check if super class has already been caught. + """ + seen: List[Type] = [] + for expr in s.types: + if expr and isinstance(expr, NameExpr) and isinstance(expr.node, TypeInfo): + with self.expr_checker.msg.disable_errors(): + typ = get_proper_type(self.check_except_handler_test(expr)) + if isinstance(typ, AnyType): + continue + if typ in seen: + self.msg.already_caught(typ, expr) + continue + seen_superclass = next((s for s in seen if is_subtype(typ, s)), None) + if seen_superclass is not None: + self.msg.superclass_already_caught(typ, seen_superclass, expr) + continue + seen.append(typ) + # (type, operator) tuples for augmented assignments supported with partial types partial_type_augmented_ops: Final = { ('builtins.list', '+'), @@ -3608,6 +3630,9 @@ def visit_try_stmt(self, s: TryStmt) -> None: if not self.binder.is_unreachable(): self.accept(s.finally_body) + if self.should_report_unreachable_issues(): + self.check_except_reachability(s) + def visit_try_without_finally(self, s: TryStmt, try_frame: bool) -> None: """Type check a try statement, ignoring the finally block. diff --git a/mypy/messages.py b/mypy/messages.py index 406237783cf1..21dbe05f5721 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1386,6 +1386,15 @@ def note_call(self, def unreachable_statement(self, context: Context) -> None: self.fail("Statement is unreachable", context, code=codes.UNREACHABLE) + def already_caught(self, typ: Type, context: Context) -> None: + self.fail("Except is unreachable, {} has already been caught".format(format_type(typ)), + context, code=codes.UNREACHABLE) + + def superclass_already_caught(self, typ: Type, super_typ: Type, context: Context) -> None: + self.fail("Except is unreachable, superclass {} of {} has already been caught" + .format(format_type(super_typ), format_type(typ)), context, + code=codes.UNREACHABLE) + def redundant_left_operand(self, op_name: str, context: Context) -> None: """Indicates that the left operand of a boolean expression is redundant: it does not change the truth value of the entire condition as a whole. diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index de85f5188bb8..fe1037aad216 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -841,6 +841,38 @@ def baz(x: int) -> int: return x [builtins fixtures/exception.pyi] +[case testUnreachableFlagExcepts] +# flags: --warn-unreachable +from typing import Type, NoReturn, Tuple + +def error() -> NoReturn: ... + +ignore: Type[Exception] +exclude: Tuple[Type[Exception], ...] +class ContinueOnError: pass + +try: + error() +except ignore: + pass +except exclude: + pass +except ContinueOnError: # E: Exception type must be derived from BaseException + pass +except Exception: + pass +except Exception as err: # E: Except is unreachable, "Exception" has already been caught + pass +except RuntimeError: # E: Except is unreachable, superclass "Exception" of "RuntimeError" has already been caught + pass +except (NotImplementedError, ): + pass +except NotImplementedError: # E: Except is unreachable, superclass "Exception" of "NotImplementedError" has already been caught + pass +except NotImplementedError: # E: Except is unreachable, superclass "Exception" of "NotImplementedError" has already been caught + pass +[builtins fixtures/exception.pyi] + [case testUnreachableFlagIgnoresSemanticAnalysisUnreachable] # flags: --warn-unreachable --python-version 3.7 --platform win32 --always-false FOOBAR import sys From 14ee9a158f12e4cd3de55d23a02c1adeb0672dc2 Mon Sep 17 00:00:00 2001 From: Jannic Warken Date: Fri, 28 Jan 2022 16:40:56 +0100 Subject: [PATCH 2/7] Move errors to message_registry --- mypy/checker.py | 5 +++-- mypy/message_registry.py | 10 ++++++++++ mypy/messages.py | 9 --------- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index da397f03e4bc..f399e76d906d 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2350,11 +2350,12 @@ def check_except_reachability(self, s: TryStmt) -> None: if isinstance(typ, AnyType): continue if typ in seen: - self.msg.already_caught(typ, expr) + self.fail(message_registry.ALREADY_CAUGHT.format(format_type(typ)), expr) continue seen_superclass = next((s for s in seen if is_subtype(typ, s)), None) if seen_superclass is not None: - self.msg.superclass_already_caught(typ, seen_superclass, expr) + self.fail(message_registry.SUPERCLASS_ALREADY_CAUGHT.format( + format_type(seen_superclass), format_type(typ)), expr) continue seen.append(typ) diff --git a/mypy/message_registry.py b/mypy/message_registry.py index 1477cc4da575..df16bdaf4800 100644 --- a/mypy/message_registry.py +++ b/mypy/message_registry.py @@ -245,3 +245,13 @@ def format(self, *args: object, **kwargs: object) -> "ErrorMessage": CLASS_PATTERN_DUPLICATE_KEYWORD_PATTERN: Final = 'Duplicate keyword pattern "{}"' CLASS_PATTERN_UNKNOWN_KEYWORD: Final = 'Class "{}" has no attribute "{}"' MULTIPLE_ASSIGNMENTS_IN_PATTERN: Final = 'Multiple assignments to name "{}" in pattern' + +# Unreachable +ALREADY_CAUGHT: Final = ErrorMessage( + "Except is unreachable, {} has already been caught", + code=codes.UNREACHABLE, +) +SUPERCLASS_ALREADY_CAUGHT: Final = ErrorMessage( + "Except is unreachable, superclass {} of {} has already been caught", + code=codes.UNREACHABLE, +) diff --git a/mypy/messages.py b/mypy/messages.py index 21dbe05f5721..406237783cf1 100644 --- a/mypy/messages.py +++ b/mypy/messages.py @@ -1386,15 +1386,6 @@ def note_call(self, def unreachable_statement(self, context: Context) -> None: self.fail("Statement is unreachable", context, code=codes.UNREACHABLE) - def already_caught(self, typ: Type, context: Context) -> None: - self.fail("Except is unreachable, {} has already been caught".format(format_type(typ)), - context, code=codes.UNREACHABLE) - - def superclass_already_caught(self, typ: Type, super_typ: Type, context: Context) -> None: - self.fail("Except is unreachable, superclass {} of {} has already been caught" - .format(format_type(super_typ), format_type(typ)), context, - code=codes.UNREACHABLE) - def redundant_left_operand(self, op_name: str, context: Context) -> None: """Indicates that the left operand of a boolean expression is redundant: it does not change the truth value of the entire condition as a whole. From 374e680f2bbcaeb7838afb6cd58ca7983558c4f4 Mon Sep 17 00:00:00 2001 From: Jannic Warken Date: Fri, 28 Jan 2022 17:58:25 +0100 Subject: [PATCH 3/7] Split tests and make them more expressive --- test-data/unit/check-unreachable-code.test | 70 +++++++++++++++++++--- 1 file changed, 61 insertions(+), 9 deletions(-) diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index fe1037aad216..f0c964cfca21 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -843,13 +843,36 @@ def baz(x: int) -> int: [case testUnreachableFlagExcepts] # flags: --warn-unreachable -from typing import Type, NoReturn, Tuple +from typing import Type, NoReturn def error() -> NoReturn: ... +try: + error() +except Exception: + pass +except Exception as err: # E: Except is unreachable, "Exception" has already been caught + pass +except RuntimeError: # E: Except is unreachable, superclass "Exception" of "RuntimeError" has already been caught + pass +except (NotImplementedError, ): + pass +except NotImplementedError: # E: Except is unreachable, superclass "Exception" of "NotImplementedError" has already been caught + pass +except NotImplementedError: # E: Except is unreachable, superclass "Exception" of "NotImplementedError" has already been caught + pass +[builtins fixtures/exception.pyi] + +[case testUnreachableFlagIgnoreVariablesInExcept] +# flags: --warn-unreachable +from typing import NoReturn, Union, Type, Tuple + +def error() -> NoReturn: ... +class MyError(BaseException): ... + ignore: Type[Exception] exclude: Tuple[Type[Exception], ...] -class ContinueOnError: pass +omit: Union[Type[RuntimeError], Type[MyError]] try: error() @@ -857,19 +880,48 @@ except ignore: pass except exclude: pass -except ContinueOnError: # E: Exception type must be derived from BaseException +except omit: pass -except Exception: +except RuntimeError: pass -except Exception as err: # E: Except is unreachable, "Exception" has already been caught +[builtins fixtures/exception.pyi] + +[case testUnreachableFlagExceptWithUnknownBaseClass] +# flags: --warn-unreachable +from typing import Any, NoReturn + +Parent: Any +class MyError(Parent): ... +def error() -> NoReturn: ... + +try: + error() +except MyError: pass -except RuntimeError: # E: Except is unreachable, superclass "Exception" of "RuntimeError" has already been caught +except Exception: pass -except (NotImplementedError, ): +except MyError: # E: Except is unreachable, "MyError" has already been caught pass -except NotImplementedError: # E: Except is unreachable, superclass "Exception" of "NotImplementedError" has already been caught +[builtins fixtures/exception.pyi] + +[case testUnreachableFlagExceptWithError] +# flags: --warn-unreachable +from typing import NoReturn + +# The following class is not derived from 'BaseException'. Thus an error is raised +# resulting in Any as type for the class in the exception expression. Such cases +# must be ignored in the rest of the reachability analysis. +class NotFromBaseException: ... + +def error() -> NoReturn: ... + +try: + error() +except NotFromBaseException: # E: Exception type must be derived from BaseException pass -except NotImplementedError: # E: Except is unreachable, superclass "Exception" of "NotImplementedError" has already been caught +except RuntimeError: + pass +except NotImplementedError: # E: Except is unreachable, superclass "RuntimeError" of "NotImplementedError" has already been caught pass [builtins fixtures/exception.pyi] From 5b4d73ddfd6be314b61e7f677371bc424bce589c Mon Sep 17 00:00:00 2001 From: Jannic Warken Date: Fri, 28 Jan 2022 17:58:48 +0100 Subject: [PATCH 4/7] Add safety check --- mypy/checker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/checker.py b/mypy/checker.py index f399e76d906d..9904194f268e 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2347,7 +2347,7 @@ def check_except_reachability(self, s: TryStmt) -> None: if expr and isinstance(expr, NameExpr) and isinstance(expr.node, TypeInfo): with self.expr_checker.msg.disable_errors(): typ = get_proper_type(self.check_except_handler_test(expr)) - if isinstance(typ, AnyType): + if isinstance(typ, AnyType) or isinstance(typ, UnionType): continue if typ in seen: self.fail(message_registry.ALREADY_CAUGHT.format(format_type(typ)), expr) From 0576effcff4677d507c3f6e4ad700556d7042784 Mon Sep 17 00:00:00 2001 From: Jannic Warken Date: Fri, 28 Jan 2022 18:04:12 +0100 Subject: [PATCH 5/7] Add test for no errors without --warn-unreachable flag --- test-data/unit/check-unreachable-code.test | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test-data/unit/check-unreachable-code.test b/test-data/unit/check-unreachable-code.test index f0c964cfca21..cfe5021cc8dc 100644 --- a/test-data/unit/check-unreachable-code.test +++ b/test-data/unit/check-unreachable-code.test @@ -925,6 +925,19 @@ except NotImplementedError: # E: Except is unreachable, superclass "RuntimeEr pass [builtins fixtures/exception.pyi] +[case testUnreachableExceptWithoutUnreachableFlag] +from typing import NoReturn + +def error() -> NoReturn: ... + +try: + error() +except Exception: + pass +except RuntimeError: + pass +[builtins fixtures/exception.pyi] + [case testUnreachableFlagIgnoresSemanticAnalysisUnreachable] # flags: --warn-unreachable --python-version 3.7 --platform win32 --always-false FOOBAR import sys From 0c3d194dba40865d97c5ef0067f711eb3a412c9d Mon Sep 17 00:00:00 2001 From: Jannic Warken Date: Fri, 28 Jan 2022 18:14:26 +0100 Subject: [PATCH 6/7] Add type to 'UNREACHABLE' (to satisfy mypyc) --- mypy/errorcodes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mypy/errorcodes.py b/mypy/errorcodes.py index ba716608ae56..eb79fb60e6b4 100644 --- a/mypy/errorcodes.py +++ b/mypy/errorcodes.py @@ -121,7 +121,7 @@ def __str__(self) -> str: 'Reject returning value with "Any" type if return type is not "Any"', "General", ) -UNREACHABLE: Final = ErrorCode( +UNREACHABLE: Final[ErrorCode] = ErrorCode( "unreachable", "Warn about unreachable statements or expressions", "General" ) REDUNDANT_EXPR: Final = ErrorCode( From 329bd3658f0fea6c65333af9fcce6e08bcdaa96b Mon Sep 17 00:00:00 2001 From: Jannic Warken Date: Sun, 1 May 2022 17:08:29 +0200 Subject: [PATCH 7/7] Replace disable_errors call with new filter_errors call --- mypy/checker.py | 2 +- mypy/message_registry.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mypy/checker.py b/mypy/checker.py index c4388ed58629..3016a0eb237f 100644 --- a/mypy/checker.py +++ b/mypy/checker.py @@ -2424,7 +2424,7 @@ def check_except_reachability(self, s: TryStmt) -> None: seen: List[Type] = [] for expr in s.types: if expr and isinstance(expr, NameExpr) and isinstance(expr.node, TypeInfo): - with self.expr_checker.msg.disable_errors(): + with self.expr_checker.msg.filter_errors(): typ = get_proper_type(self.check_except_handler_test(expr)) if isinstance(typ, AnyType) or isinstance(typ, UnionType): continue diff --git a/mypy/message_registry.py b/mypy/message_registry.py index cab99a291ed7..1f554224207b 100644 --- a/mypy/message_registry.py +++ b/mypy/message_registry.py @@ -266,4 +266,4 @@ def format(self, *args: object, **kwargs: object) -> "ErrorMessage": SUPERCLASS_ALREADY_CAUGHT: Final = ErrorMessage( "Except is unreachable, superclass {} of {} has already been caught", code=codes.UNREACHABLE, -) \ No newline at end of file +)