Skip to content

Commit

Permalink
Update to account for branches within exception blocks
Browse files Browse the repository at this point in the history 
Also rename checker to "do-not-log-raised-errors"
  • Loading branch information
@JessicaBell00
JessicaBell00 committed Aug 19, 2024
1 parent 9103a29 commit 5f6be69
Show file tree
Hide file tree
Showing 3 changed files with 246 additions and 58 deletions.
4 changes: 2 additions & 2 deletions tools/pylint-extensions/azure-pylint-guidelines-checker/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ In the case of a false positive, use the disable command to remove the pylint er
## Rules List

| Pylint checker name | How to fix this | How to disable this rule | Link to python guideline |
| -------------------------------------------------- |----------------------------------------------------------------------------------------------------------------------------------------------------------------------| ------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- |
|----------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------------------------------------------------------| ------------------------------------------------------------------------------------------------------- |
| client-method-should-not-use-static-method | Use module level functions instead. | # pylint:disable=client-method-should-not-use-static-method | [link](https://azure.github.io/azure-sdk/python_implementation.html#method-signatures) |
| missing-client-constructor-parameter-credential | Add a credential parameter to the client constructor. Do not use plural form "credentials". | # pylint:disable=missing-client-constructor-parameter-credential | [link](https://azure.github.io/azure-sdk/python_design.html#client-configuration) |
| missing-client-constructor-parameter-kwargs | Add a \*\*kwargs parameter to the client constructor. | # pylint:disable=missing-client-constructor-parameter-kwargs | [link](https://azure.github.io/azure-sdk/python_design.html#client-configuration) |
Expand Down Expand Up @@ -92,4 +92,4 @@ In the case of a false positive, use the disable command to remove the pylint er
| docstring-keyword-should-match-keyword-only | Docstring keyword arguments and keyword-only method arguments should match. | pylint:disable=docstring-keyword-should-match-keyword-only | [link](https://azure.github.io/azure-sdk/python_documentation.html#docstrings) |
| docstring-type-do-not-use-class | Docstring type is formatted incorrectly. Do not use `:class` in docstring type. | pylint:disable=docstring-type-do-not-use-class | [link](https://sphinx-rtd-tutorial.readthedocs.io/en/latest/docstrings.html) |
| no-typing-import-in-type-check | Do not import typing under TYPE_CHECKING. | pylint:disable=no-typing-import-in-type-check | No Link. |
| do-not-log-errors-that-get-raised | Do not log errors that get rasied in an exception block. | pylint:disable=do-not-log-errors-that-get-raised | No Link. |
| do-not-log-raised-errors | Do not log errors at `error` or `warning` level when error is raised in an exception block. | pylint:disable=do-not-log-raised-errors | No Link. |
65 changes: 46 additions & 19 deletions tools/pylint-extensions/azure-pylint-guidelines-checker/pylint_guidelines_checker.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2712,36 +2712,63 @@ class DoNotLogErrorsEndUpRaising(BaseChecker):

"""Rule to check that errors that get raised aren't logged"""

name = "do-not-log-errors-that-get-raised"
name = "do-not-log-raised-errors"
priority = -1
msgs = {"C4762": (
"Do not log errors that get raised in an exception block.",
"do-not-log-errors-that-get-raised",
"Do not log errors that get raised in an exception block. Do not log at error or warning levels",
"do-not-log-raised-errors",
"Do not log errors at error or warning level when error is raised in an exception block",
),
}

def visit_try(self, node):
"""Check that raised errors aren't logged at 'error' or 'warning' levels in exception blocks.
Go through each line in the exception block and make sure it hasn't been logged if exception is raised.
Go through exception block and branches and ensure error hasn't been logged if exception is raised.
"""
matches = [".warning", ".error"]
# Get the exception block - returns a list
# Return a list of exception blocks
except_block = node.handlers
# Iterate through each exception block
for nod in except_block:
# Split into list of strings
split = nod.as_string().splitlines()
for i in split:
# Search to check if 'raise' is in the exception block
raise_found = i.find("raise")
if raise_found != -1:
for j in split:
if any(x in j for x in matches):
self.add_message(
msgid=f"do-not-log-errors-that-get-raised",
node=node,
confidence=None,
)
# Get the nodes in each block (e.g. nodes Expr and Raise)
exception_block_body = nod.body
self.check_for_raise(exception_block_body)

def check_for_raise(self, node):
""" Helper function - checks for instance of 'Raise' node
Also checks 'If' and nested 'If' branches
"""
for i in node:
if isinstance(i, astroid.Raise):
self.check_for_logging(node)
# Check for any nested 'If' branches
if isinstance(i, astroid.If):
self.check_for_raise(i.body)
# Check any 'elif' and 'else' branches
elif_statements = i.orelse
while len(elif_statements) == 1:
if isinstance(elif_statements[0], astroid.If):
for x in elif_statements:
self.check_for_raise(x.body)
elif_statements = x.orelse
else:
break
# Check 'else' body for raise
self.check_for_raise(elif_statements)

def check_for_logging(self, node):
""" Helper function - checks 'Expr' nodes to see if logging has occurred at 'warning' or 'error'
levels. Called from 'check_for_raise' function
"""
matches = [".warning", ".error"]
for j in node:
if isinstance(j, astroid.Expr):
expression = j.as_string()
if any(x in expression for x in matches):
self.add_message(
msgid=f"do-not-log-raised-errors",
node=j,
confidence=None,
)


class NoImportTypingFromTypeCheck(BaseChecker):
Expand Down
Loading

0 comments on commit 5f6be69

Please sign in to comment.