Fix ``no-else-return false Negative for try/except/else pattern (#7888)

Co-authored-by: Pierre Sassoulas <[email protected]>

Author: clavedeluna

doc/whatsnew/fragments/7788.false_negative

@@ -0,0 +1,3 @@
+``no-else-return`` or ``no-else-raise`` will be emitted if ``except`` block always returns or raises.
+
+Closes #7788

pylint/checkers/refactoring/refactoring_checker.py

@@ -73,6 +73,16 @@ def _if_statement_is_always_returning(
     return any(isinstance(node, returning_node_class) for node in if_node.body)
 
 
+def _except_statement_is_always_returning(
+    node: nodes.TryExcept, returning_node_class: nodes.NodeNG
+) -> bool:
+    """Detect if all except statements return."""
+    return all(
+        any(isinstance(child, returning_node_class) for child in handler.body)
+        for handler in node.handlers
+    )
+
+
 def _is_trailing_comma(tokens: list[tokenize.TokenInfo], index: int) -> bool:
     """Check if the given token is a trailing comma.
 
@@ -531,7 +541,7 @@ def _is_bool_const(node: nodes.Return | nodes.Assign) -> bool:
             node.value.value, bool
         )
 
-    def _is_actual_elif(self, node: nodes.If) -> bool:
+    def _is_actual_elif(self, node: nodes.If | nodes.TryExcept) -> bool:
         """Check if the given node is an actual elif.
 
         This is a problem we're having with the builtin ast module,
@@ -642,10 +652,14 @@ def leave_module(self, _: nodes.Module) -> None:
         )
         self._init()
 
-    @utils.only_required_for_messages("too-many-nested-blocks")
-    def visit_tryexcept(self, node: nodes.TryExcept) -> None:
+    @utils.only_required_for_messages("too-many-nested-blocks", "no-else-return")
+    def visit_tryexcept(self, node: nodes.TryExcept | nodes.TryFinally) -> None:
         self._check_nested_blocks(node)
 
+        if isinstance(node, nodes.TryExcept):
+            self._check_superfluous_else_return(node)
+            self._check_superfluous_else_raise(node)
+
     visit_tryfinally = visit_tryexcept
     visit_while = visit_tryexcept
 
@@ -707,23 +721,38 @@ def visit_with(self, node: nodes.With) -> None:
                 self._check_redefined_argument_from_local(name)
 
     def _check_superfluous_else(
-        self, node: nodes.If, msg_id: str, returning_node_class: nodes.NodeNG
+        self,
+        node: nodes.If | nodes.TryExcept,
+        msg_id: str,
+        returning_node_class: nodes.NodeNG,
     ) -> None:
+        if isinstance(node, nodes.TryExcept) and isinstance(
+            node.parent, nodes.TryFinally
+        ):
+            # Not interested in try/except/else/finally statements.
+            return
+
         if not node.orelse:
-            # Not interested in if statements without else.
+            # Not interested in if/try statements without else.
             return
 
         if self._is_actual_elif(node):
             # Not interested in elif nodes; only if
             return
 
-        if _if_statement_is_always_returning(node, returning_node_class):
+        if (
+            isinstance(node, nodes.If)
+            and _if_statement_is_always_returning(node, returning_node_class)
+        ) or (
+            isinstance(node, nodes.TryExcept)
+            and _except_statement_is_always_returning(node, returning_node_class)
+        ):
             orelse = node.orelse[0]
             if (orelse.lineno, orelse.col_offset) in self._elifs:
                 args = ("elif", 'remove the leading "el" from "elif"')
             else:
                 args = ("else", 'remove the "else" and de-indent the code inside it')
-            self.add_message(msg_id, node=node, args=args)
+            self.add_message(msg_id, node=node, args=args, confidence=HIGH)
 
     def _check_superfluous_else_return(self, node: nodes.If) -> None:
         return self._check_superfluous_else(

pylint/lint/pylinter.py

@@ -454,8 +454,8 @@ def _load_reporter_by_name(self, reporter_name: str) -> reporters.BaseReporter:
             reporter_class = _load_reporter_by_class(reporter_name)
         except (ImportError, AttributeError, AssertionError) as e:
             raise exceptions.InvalidReporterError(name) from e
-        else:
-            return reporter_class()
+
+        return reporter_class()
 
     def set_reporter(
         self, reporter: reporters.BaseReporter | reporters.MultiReporter

tests/functional/n/no/no_else_break.txt

@@ -1,7 +1,7 @@
-no-else-break:8:8:11:17:foo1:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-break:16:8:21:17:foo2:"Unnecessary ""elif"" after ""break"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-break:28:12:33:21:foo3:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-break:41:8:48:17:foo4:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-break:54:8:63:17:foo5:"Unnecessary ""elif"" after ""break"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-break:70:12:74:21:foo6:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-break:110:8:116:21:bar4:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":UNDEFINED
+no-else-break:8:8:11:17:foo1:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-break:16:8:21:17:foo2:"Unnecessary ""elif"" after ""break"", remove the leading ""el"" from ""elif""":HIGH
+no-else-break:28:12:33:21:foo3:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-break:41:8:48:17:foo4:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-break:54:8:63:17:foo5:"Unnecessary ""elif"" after ""break"", remove the leading ""el"" from ""elif""":HIGH
+no-else-break:70:12:74:21:foo6:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-break:110:8:116:21:bar4:"Unnecessary ""else"" after ""break"", remove the ""else"" and de-indent the code inside it":HIGH

tests/functional/n/no/no_else_continue.txt

@@ -1,7 +1,7 @@
-no-else-continue:8:8:11:17:foo1:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-continue:16:8:21:17:foo2:"Unnecessary ""elif"" after ""continue"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-continue:28:12:33:24:foo3:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-continue:41:8:48:17:foo4:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-continue:54:8:63:17:foo5:"Unnecessary ""elif"" after ""continue"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-continue:70:12:74:21:foo6:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-continue:110:8:116:24:bar4:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":UNDEFINED
+no-else-continue:8:8:11:17:foo1:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-continue:16:8:21:17:foo2:"Unnecessary ""elif"" after ""continue"", remove the leading ""el"" from ""elif""":HIGH
+no-else-continue:28:12:33:24:foo3:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-continue:41:8:48:17:foo4:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-continue:54:8:63:17:foo5:"Unnecessary ""elif"" after ""continue"", remove the leading ""el"" from ""elif""":HIGH
+no-else-continue:70:12:74:21:foo6:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-continue:110:8:116:24:bar4:"Unnecessary ""else"" after ""continue"", remove the ""else"" and de-indent the code inside it":HIGH

tests/functional/n/no/no_else_raise.txt

@@ -1,7 +1,7 @@
-no-else-raise:6:4:11:26:foo1:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-raise:15:4:23:26:foo2:"Unnecessary ""elif"" after ""raise"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-raise:29:8:34:30:foo3:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-raise:41:4:48:13:foo4:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-raise:53:4:62:13:foo5:"Unnecessary ""elif"" after ""raise"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-raise:68:8:72:17:foo6:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-raise:104:4:110:33:bar4:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":UNDEFINED
+no-else-raise:6:4:11:26:foo1:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-raise:15:4:23:26:foo2:"Unnecessary ""elif"" after ""raise"", remove the leading ""el"" from ""elif""":HIGH
+no-else-raise:29:8:34:30:foo3:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-raise:41:4:48:13:foo4:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-raise:53:4:62:13:foo5:"Unnecessary ""elif"" after ""raise"", remove the leading ""el"" from ""elif""":HIGH
+no-else-raise:68:8:72:17:foo6:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-raise:104:4:110:33:bar4:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":HIGH

tests/functional/n/no/no_else_return.py

@@ -108,3 +108,87 @@ def bar4(x):
             return False
         except ValueError:
             return None
+
+# pylint: disable = bare-except
+def try_one_except() -> bool:
+    try:  # [no-else-return]
+        print('asdf')
+    except:
+        print("bad")
+        return False
+    else:
+        return True
+
+
+def try_multiple_except() -> bool:
+    try:  # [no-else-return]
+        print('asdf')
+    except TypeError:
+        print("bad")
+        return False
+    except ValueError:
+        print("bad second")
+        return False
+    else:
+        return True
+
+def try_not_all_except_return() -> bool:  # [inconsistent-return-statements]
+    try:
+        print('asdf')
+    except TypeError:
+        print("bad")
+        return False
+    except ValueError:
+        val = "something"
+    else:
+        return True
+
+# pylint: disable = raise-missing-from
+def try_except_raises() -> bool:
+    try:  # [no-else-raise]
+        print('asdf')
+    except:
+        raise ValueError
+    else:
+        return True
+
+def try_except_raises2() -> bool:
+    try:  # [no-else-raise]
+        print('asdf')
+    except TypeError:
+        raise ValueError
+    except ValueError:
+        raise TypeError
+    else:
+        return True
+
+def test() -> bool:  # [inconsistent-return-statements]
+    try:
+        print('asdf')
+    except RuntimeError:
+        return False
+    finally:
+        print("in finally")
+
+
+def try_finally_return() -> bool:  # [inconsistent-return-statements]
+    try:
+        print('asdf')
+    except RuntimeError:
+        return False
+    else:
+        print("inside else")
+    finally:
+        print("in finally")
+
+def try_finally_raise():
+    current_tags = {}
+    try:
+        yield current_tags
+    except Exception:
+        current_tags["result"] = "failure"
+        raise
+    else:
+        current_tags["result"] = "success"
+    finally:
+        print("inside finally")

tests/functional/n/no/no_else_return.txt

@@ -1,7 +1,14 @@
-no-else-return:6:4:11:16:foo1:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-return:15:4:23:16:foo2:"Unnecessary ""elif"" after ""return"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-return:29:8:34:20:foo3:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-return:41:4:48:13:foo4:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-return:53:4:62:13:foo5:"Unnecessary ""elif"" after ""return"", remove the leading ""el"" from ""elif""":UNDEFINED
-no-else-return:68:8:72:17:foo6:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":UNDEFINED
-no-else-return:104:4:110:23:bar4:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":UNDEFINED
+no-else-return:6:4:11:16:foo1:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-return:15:4:23:16:foo2:"Unnecessary ""elif"" after ""return"", remove the leading ""el"" from ""elif""":HIGH
+no-else-return:29:8:34:20:foo3:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-return:41:4:48:13:foo4:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-return:53:4:62:13:foo5:"Unnecessary ""elif"" after ""return"", remove the leading ""el"" from ""elif""":HIGH
+no-else-return:68:8:72:17:foo6:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-return:104:4:110:23:bar4:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-return:114:4:120:19:try_one_except:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-return:124:4:133:19:try_multiple_except:"Unnecessary ""else"" after ""return"", remove the ""else"" and de-indent the code inside it":HIGH
+inconsistent-return-statements:135:0:135:29:try_not_all_except_return:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
+no-else-raise:148:4:153:19:try_except_raises:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":HIGH
+no-else-raise:156:4:163:19:try_except_raises2:"Unnecessary ""else"" after ""raise"", remove the ""else"" and de-indent the code inside it":HIGH
+inconsistent-return-statements:165:0:165:8:test:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED
+inconsistent-return-statements:174:0:174:22:try_finally_return:Either all return statements in a function should return an expression, or none of them should.:UNDEFINED

tests/functional/r/raise_missing_from.py

@@ -1,5 +1,5 @@
 # pylint:disable=missing-docstring, unreachable, using-constant-test, invalid-name, bare-except
-# pylint:disable=try-except-raise, undefined-variable, too-few-public-methods, superfluous-parens
+# pylint:disable=try-except-raise, undefined-variable, too-few-public-methods, superfluous-parens, no-else-raise
 
 try:
     1 / 0