Skip to content

Commit

Permalink
feat(fixers): implement fixer for reraise no cause
Browse files Browse the repository at this point in the history
  • Loading branch information
guilatrova committed Sep 12, 2021
1 parent 18af429 commit 2f9d46d
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 12 deletions.
6 changes: 5 additions & 1 deletion src/tests/analyzers_exception_block_test.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import ast
from functools import partial

from tryceratops import analyzers
Expand All @@ -15,8 +16,11 @@ def test_reraise_no_cause():

violations = analyzer.check(tree, "filename")
assert len(violations) == 1
single = violations[0]

assert_no_cause(16, 8, violations[0])
assert_no_cause(16, 8, single)
assert single.exception_name is None
assert isinstance(single.except_node, ast.ExceptHandler)


def test_verbose_reraise():
Expand Down
36 changes: 31 additions & 5 deletions src/tests/fixers_test.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import ast
from typing import List, Tuple
from unittest.mock import MagicMock

from tryceratops.fixers import VerboseReraiseFixer
from tryceratops.violations import VerboseReraiseViolation, codes
from tryceratops.fixers import RaiseWithoutCauseFixer, VerboseReraiseFixer
from tryceratops.violations import RaiseWithoutCauseViolation, VerboseReraiseViolation, codes

from .analyzer_helpers import read_sample_lines

Expand All @@ -11,16 +12,26 @@ def create_verbose_reraise_violation(code: Tuple[str, str], line: int):
return VerboseReraiseViolation(code[0], line, 0, "", "", None, "ex")


def create_raise_no_cause_violation(line: int, except_line: int):
code, _ = codes.RERAISE_NO_CAUSE
node_mock = MagicMock(spec=ast.Raise, lineno=line)
except_node_mock = MagicMock(spec=ast.ExceptHandler, lineno=except_line)
return RaiseWithoutCauseViolation(code, line, 0, "", "", node_mock, except_node_mock)


def assert_ast_is_valid(results: List[str]):
content = "\n".join(results)
result = ast.parse(content)
assert result is not None


def assert_unmodified_lines(initial: List[str], results: List[str], modified_offset: int):
def assert_unmodified_lines(initial: List[str], results: List[str], *modified_offsets: int):
assert len(results) == len(initial)
assert results[:modified_offset] == initial[:modified_offset]
assert results[modified_offset + 1 :] == initial[modified_offset + 1 :]
for idx, initial_line in enumerate(initial):
if idx in modified_offsets:
continue

assert initial_line == results[idx], f"Line {idx+1} got modified"


def test_verbose_fixer():
Expand All @@ -35,3 +46,18 @@ def test_verbose_fixer():
assert_ast_is_valid(results)
assert_unmodified_lines(lines, results, expected_modified_offset)
assert results[expected_modified_offset].endswith("raise # This is verbose\n")


def test_raise_without_cause_fixer_without_exception_name():
fixer = RaiseWithoutCauseFixer()
lines = read_sample_lines("except_reraise_no_cause")
expected_modified_offsets = {14, 15}
dependent_offset, offending_offset = expected_modified_offsets
violation = create_raise_no_cause_violation(offending_offset + 1, dependent_offset + 1)

results = fixer.perform_fix(lines, violation)

assert_ast_is_valid(results)
assert_unmodified_lines(lines, results, *expected_modified_offsets)
assert results[dependent_offset].endswith("except Exception as ex:\n")
assert results[offending_offset].endswith("raise MyException() from ex\n")
6 changes: 3 additions & 3 deletions src/tryceratops/analyzers/exception_block.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import ast
from typing import Iterable, Optional

from tryceratops.violations import codes
from tryceratops.violations.violations import VerboseReraiseViolation
from tryceratops.violations import RaiseWithoutCauseViolation, VerboseReraiseViolation, codes

from .base import BaseAnalyzer, visit_error_handler


class ExceptReraiseWithoutCauseAnalyzer(BaseAnalyzer):
violation_code = codes.RERAISE_NO_CAUSE
violation_type = RaiseWithoutCauseViolation

@visit_error_handler
def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None:
Expand All @@ -18,7 +18,7 @@ def is_raise_without_cause(node: ast.AST):
return False

reraises_no_cause = [stm for stm in ast.walk(node) if is_raise_without_cause(stm)]
self._mark_violation(*reraises_no_cause)
self._mark_violation(*reraises_no_cause, exception_name=node.name, except_node=node)

self.generic_visit(node)

Expand Down
2 changes: 1 addition & 1 deletion src/tryceratops/fixers/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
from .exception_block import VerboseReraiseFixer
from .exception_block import RaiseWithoutCauseFixer, VerboseReraiseFixer
31 changes: 30 additions & 1 deletion src/tryceratops/fixers/exception_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from typing import Generic, Iterable, List, Tuple, TypeVar

from tryceratops.violations import Violation, codes
from tryceratops.violations.violations import VerboseReraiseViolation
from tryceratops.violations.violations import RaiseWithoutCauseViolation, VerboseReraiseViolation


class FileFixerHandler:
Expand Down Expand Up @@ -79,3 +79,32 @@ def perform_fix(self, lines: List[str], violation: VerboseReraiseViolation) -> L
all_lines[violation.line - 1] = new_line

return all_lines


class RaiseWithoutCauseFixer(BaseFixer[RaiseWithoutCauseViolation]):
violation_code = codes.RERAISE_NO_CAUSE
exception_name_to_create = "ex"

def _add_name_to_except_handler(self, line: str) -> str:
# TODO: refactor function name
new_line = re.sub(r"except (.*):", rf"except \1 as {self.exception_name_to_create}:", line)
return new_line

def perform_fix(self, lines: List[str], violation: RaiseWithoutCauseViolation) -> List[str]:
all_lines = lines[:]
exception_name = violation.exception_name

if violation.except_node and exception_name is None:
# TODO: refactor: calc line, get line, modify line, write line
dependent_line_offset = violation.except_node.lineno - 1
dependent_line = all_lines[dependent_line_offset]
dependent_fixed_line = self._add_name_to_except_handler(dependent_line)
all_lines[dependent_line_offset] = dependent_fixed_line
exception_name = self.exception_name_to_create

guilty_line = all_lines[violation.line - 1]
# TODO: What if this raise is multi-line?
new_line = re.sub(r"raise (.*)", rf"raise \1 from {exception_name}", guilty_line)
all_lines[violation.line - 1] = new_line

return all_lines
2 changes: 1 addition & 1 deletion src/tryceratops/violations/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
from .codes import *
from .violations import VerboseReraiseViolation, Violation
from .violations import RaiseWithoutCauseViolation, VerboseReraiseViolation, Violation
21 changes: 21 additions & 0 deletions src/tryceratops/violations/violations.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,24 @@ def build(
):
code, msg = vio_details
return cls(code, node.lineno, node.col_offset, msg, filename, node, exception_name)


@dataclass
class RaiseWithoutCauseViolation(Violation):
except_node: Optional[ast.AST] = None
exception_name: Optional[str] = None

@classmethod
def build(
cls,
filename: str,
vio_details: Tuple[str, str],
node: ast.AST,
except_node: Optional[ast.AST] = None,
exception_name: Optional[str] = None,
**kwargs
):
code, msg = vio_details
return cls(
code, node.lineno, node.col_offset, msg, filename, node, except_node, exception_name
)

0 comments on commit 2f9d46d

Please sign in to comment.