feat: useless try-except

src/tests/analyzers_exception_block_test.py

@@ -56,6 +56,25 @@ def test_broad_pass():
     assert_broad(35, 12, third)
 
 
+def test_useless_except():
+    tree = read_sample("try_useless_except")
+
+    analyzer = analyzers.exception_block.UselessTryExceptAnalyzer()
+    code, msg = codes.USELESS_TRY_EXCEPT
+
+    violations = analyzer.check(tree, "filename")
+    assert_useless_except = partial(assert_violation, code, msg)
+
+    assert len(violations) == 7
+    assert_useless_except(22, 4, violations[0])
+    assert_useless_except(54, 4, violations[1])
+    assert_useless_except(62, 4, violations[2])
+    assert_useless_except(70, 4, violations[3])
+    assert_useless_except(78, 4, violations[4])
+    assert_useless_except(88, 4, violations[5])
+    assert_useless_except(153, 4, violations[6])
+
+
 def test_log_error():
     tree = read_sample("log_error")
     analyzer = analyzers.exception_block.LogErrorAnalyzer()

src/tests/samples/violations/try_useless_except.py

@@ -0,0 +1,165 @@
+"""
+Violation:
+
+Capturing an exception just to raise it again has no effect in runtime,
+and is considered a bad practice.
+
+It's recommended to either log the error, change the behavior on capture,
+or just remove the try-except block.
+
+"""
+import logging
+
+logger = logging.getLogger(__name__)
+
+
+class MyException(Exception):
+    pass
+
+
+def func_blanket():
+    # ERROR
+    try:
+        ...
+    except:
+        raise
+
+
+def func_blanket_custom():
+    # OK - raises other
+    try:
+        ...
+    except:
+        raise MyException
+
+
+def func_blanket_custom_e():
+    # OK - raises other
+    try:
+        ...
+    except Exception as e:
+        raise MyException
+
+
+def func_blanket_custom_from_e():
+    # OK - raises other
+    try:
+        ...
+    except Exception as e:
+        raise MyException from e
+
+
+def func_blanket_as_e_reraise():
+    # ERROR
+    try:
+        ...
+    except Exception as e:
+        raise e
+
+
+def func_blanket_exception():
+    # ERROR
+    try:
+        ...
+    except Exception:
+        raise
+
+
+def func_specific_exception():
+    # ERROR
+    try:
+        ...
+    except MyException:
+        raise
+
+
+def func_custom_then_blanket():
+    # ERROR - all are useless
+    try:
+        ...
+    except MyException:
+        raise
+    except:
+        raise
+
+
+def func_three_useless():
+    # ERROR - all are useless
+    try:
+        ...
+    except MyException:
+        raise
+    except Exception:
+        raise
+    except:
+        raise
+
+
+def func_blanket_then_code():
+    # OK - more than just a `raise` handler
+    try:
+        ...
+    except:
+        ...
+        raise
+
+
+def func_only_second_useless():
+    # OK - more than just a `raise` (1st handler)
+    try:
+        ...
+    except MyException:
+        ...
+    except:
+        raise
+
+
+def func_only_first_useless():
+    # OK - more than just a `raise` (2nd handler)
+    try:
+        ...
+    except MyException:
+        raise
+    except:
+        ...
+        raise
+
+
+def func_multiple_second_and_third_useless():
+    # OK - more than just a `raise` (1st handler)
+    try:
+        ...
+    except MyException:
+        ...
+        raise
+    except Exception:
+        raise
+    except:
+        raise
+
+
+def func_except_finally_vaild():
+    # OK - not useless
+    try:
+        ...
+    except:
+        ...
+    finally:
+        ...
+
+
+def func_except_finally_useless():
+    # ERROR - useless
+    try:
+        ...
+    except:
+        raise
+    finally:
+        ...
+
+
+def func_single_finally():
+    try:
+        ...
+    finally:
+        ...

src/tryceratops/analyzers/__init__.py

@@ -20,6 +20,7 @@
     exception_block.ExceptBroadPassAnalyzer,
     exception_block.LogErrorAnalyzer,
     exception_block.LogObjectAnalyzer,
+    exception_block.UselessTryExceptAnalyzer,
     try_block.TryConsiderElseAnalyzer,
     try_block.TryShouldntRaiseAnalyzer,
 }

src/tryceratops/analyzers/exception_block.py

@@ -148,3 +148,31 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None:
             self._find_violations(node)
 
         self.generic_visit(node)
+
+
+class UselessTryExceptAnalyzer(BaseAnalyzer):
+    violation_code = codes.USELESS_TRY_EXCEPT
+
+    @visit_error_handler
+    def visit_Try(self, node: ast.Try) -> None:
+        def is_handler_useless(handler: ast.ExceptHandler) -> bool:
+            # a handler whose body is just `raise` is considered useless
+
+            if len(handler.body) == 1 and isinstance(handler.body[0], ast.Raise):
+                raise_argument = handler.body[0].exc
+                return (
+                    raise_argument
+                    is None
+                    # `except ... as e: raise`, no argument
+                ) or (
+                    isinstance(raise_argument, ast.Name)
+                    and raise_argument.id == handler.name
+                    # `except ... as e: raise e`
+                )
+            return False
+
+        if node.handlers and all(is_handler_useless(handler) for handler in node.handlers):
+            # the `if node.handlers` is for allowing try-finally cases, without except
+            self._mark_violation(node)
+
+        self.generic_visit(node)

src/tryceratops/violations/codes.py

@@ -20,6 +20,7 @@
     "Simply use 'raise' without specifying exception object again",
 )
 IGNORING_EXCEPTION = ("TRY202", "You're ignoring a broad exception without even logging")
+USELESS_TRY_EXCEPT = ("TRY203", "Useless try-except, remove it or handle the exception")
 
 # TRY3xx - Try
 CONSIDER_ELSE = ("TRY300", "Consider moving this statement to an 'else' block")
@@ -39,6 +40,7 @@
     "TRY202",
     "TRY300",
     "TRY301",
+    "TRY302",
     "TRY400",
     "TRY401",
 }