Skip to content

Commit

Permalink
feat(analyzers): add analyzer log exception object
Browse files Browse the repository at this point in the history
  • Loading branch information
guilatrova committed Jul 31, 2021
1 parent 7467b0c commit b7385da
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 2 deletions.
16 changes: 16 additions & 0 deletions src/tests/analyzers_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,3 +171,19 @@ def test_log_error():
violation = violations[0]

assert_log_error(15, 8, violation)


def test_log_object():
tree = read_sample("log_object")
analyzer = analyzers.LogObjectAnalyzer()
code, msg = codes.VERBOSE_LOG_MESSAGE

assert_log_error = partial(assert_violation, code, msg)

violations = analyzer.check(tree, "filename")
assert len(violations) == 3
fstr, concat, comma = violations

assert_log_error(16, 40, fstr)
assert_log_error(23, 47, concat)
assert_log_error(30, 40, comma)
47 changes: 46 additions & 1 deletion src/tryceratops/analyzers/exception_block.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import ast
from typing import Optional
from typing import Iterable, Optional

from tryceratops.violations import codes

Expand Down Expand Up @@ -100,3 +100,48 @@ def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None:
self._mark_violation(possible_log_node)

self.generic_visit(node)


class LogObjectAnalyzer(BaseAnalyzer):
violation_code = codes.VERBOSE_LOG_MESSAGE

def _maybe_get_possible_log_func(self, node: ast.AST) -> Optional[ast.Call]:
if isinstance(node, ast.Expr):
if isinstance(node.value, ast.Call):
if isinstance(node.value.func, ast.Attribute):
return node.value

return None

def _has_object_reference(self, exception_object_name: str, node: ast.AST) -> bool:
if isinstance(node, ast.Name):
if node.id == exception_object_name:
return True

return False

def _check_args(self, exception_object_name: str, log_args: Iterable[ast.AST]):
for arg in log_args:
for node in ast.walk(arg):
if self._has_object_reference(exception_object_name, node):
self._mark_violation(node)

def _find_violations(self, exception_object_name: str, node: ast.ExceptHandler):
for stm in ast.walk(node):
if possible_log_func := self._maybe_get_possible_log_func(stm):
possible_log_node = possible_log_func.func

if isinstance(possible_log_node, ast.Attribute):
object_method = possible_log_node.attr

if object_method == "exception":
self._check_args(exception_object_name, possible_log_func.args)

@visit_error_handler
def visit_ExceptHandler(self, node: ast.ExceptHandler) -> None:
# If no name is set, then it's impossible to be verbose
# since you don't have the object
if node.name:
self._find_violations(node.name, node)

self.generic_visit(node)
1 change: 1 addition & 0 deletions src/tryceratops/analyzers/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
except_analyzers.ExceptVerboseReraiseAnalyzer,
except_analyzers.ExceptBroadPassAnalyzer,
except_analyzers.LogErrorAnalyzer,
except_analyzers.LogObjectAnalyzer,
try_analyzers.TryConsiderElseAnalyzer,
try_analyzers.TryShouldntRaiseAnalyzer,
}
Expand Down
4 changes: 3 additions & 1 deletion src/tryceratops/violations/codes.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
RAISE_WITHIN_TRY = ("TC301", "Abstract raise to an inner function")

# TC4xx - Logging
USE_LOGGING_EXCEPTION = ("TC400", "Use logging '.exception' instead of 'error'")
USE_LOGGING_EXCEPTION = ("TC400", "Use logging '.exception' instead of '.error'")
VERBOSE_LOG_MESSAGE = ("TC401", "Do not log exception object, give context instead")

CODE_CHOICES = {
"TC002",
Expand All @@ -38,4 +39,5 @@
"TC300",
"TC301",
"TC400",
"TC401",
}

0 comments on commit b7385da

Please sign in to comment.