Skip to content

Commit

Permalink
Refactoring, better descriptions, and documentation
Browse files Browse the repository at this point in the history
  • Loading branch information
andrecsilva committed Nov 27, 2023
1 parent dd81347 commit 9a3dbc2
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 47 deletions.
6 changes: 2 additions & 4 deletions src/codemodder/codemods/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,14 @@ def __init__(self, context: CodemodContext) -> None:
if dependencies:
wrapper = self.context.wrapper
if wrapper is None:
# pylint: disable-next=broad-exception-raised
raise Exception(
raise ValueError(
f"Attempting to instantiate {self.__class__.__name__} outside of "
+ "an active transform. This means that metadata hasn't been "
+ "calculated and we cannot successfully create this visitor."
)
for dep in dependencies:
if dep not in wrapper._metadata:
# pylint: disable-next=broad-exception-raised
raise Exception(
raise ValueError(
f"Attempting to access metadata {dep.__name__} that was not a "
+ "declared dependency of parent transform! This means it is "
+ "not possible to compute this value. Please ensure that all "
Expand Down
2 changes: 1 addition & 1 deletion src/core_codemods/docs/pixee_python_file-resource-leak.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
This codemod wraps assignments of `open` calls in a with statement. Without explicit closing, these resources will be "leaked", and won't be re-claimed until garbage collection. In situations where these resources are leaked rapidly (either through malicious repetitive action or unusually spiky usage), connection pool or file handle exhaustion will occur. These types of failures tend to be catastrophic, resulting in downtime and many times affect downstream applications.
This codemod wraps assignments of `open` calls in a with statement. Without explicit closing, these resources will be "leaked" and won't be re-claimed until garbage collection. In situations where these resources are leaked rapidly (either through malicious repetitive action or unusually spiky usage), connection pool or file handle exhaustion will occur. These types of failures tend to be catastrophic, resulting in downtime and many times affect downstream applications.

Our changes look something like this:

Expand Down
62 changes: 20 additions & 42 deletions src/core_codemods/file_resource_leak.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from typing import Optional, Sequence
from codemodder.utils.utils import extract_targets_of_assignment
import libcst as cst
from libcst import ensure_type, matchers
from libcst.codemod import (
Codemod,
CodemodContext,
ContextAwareVisitor,
)
Expand All @@ -14,34 +14,30 @@
)
from codemodder.change import Change
from codemodder.codemods.base_codemod import (
BaseCodemod,
CodemodMetadata,
ReviewGuidance,
)
from codemodder.codemods.base_visitor import UtilsMixin
from codemodder.codemods.api import BaseCodemod
from codemodder.codemods.utils import MetadataPreservingTransformer
from codemodder.codemods.utils_mixin import AncestorPatternsMixin, NameResolutionMixin
from codemodder.file_context import FileContext
from functools import partial


class FileResourceLeak(BaseCodemod, UtilsMixin, Codemod):
SUMMARY = "Automatically close resources"
METADATA = CodemodMetadata(
DESCRIPTION=SUMMARY,
NAME="file-resource-leak",
REVIEW_GUIDANCE=ReviewGuidance.MERGE_WITHOUT_REVIEW,
REFERENCES=[
{
"url": "https://cwe.mitre.org/data/definitions/772.html",
"description": "",
},
{
"url": "https://cwe.mitre.org/data/definitions/404.html",
"description": "",
},
],
)
class FileResourceLeak(BaseCodemod):
NAME = "file-resource-leak"
SUMMARY = "Automatically Close Resources"
REVIEW_GUIDANCE = ReviewGuidance.MERGE_WITHOUT_REVIEW
DESCRIPTION = SUMMARY
REFERENCES = [
{
"url": "https://cwe.mitre.org/data/definitions/772.html",
"description": "",
},
{
"url": "https://cwe.mitre.org/data/definitions/404.html",
"description": "",
},
]
CHANGE_DESCRIPTION = "Wrapped opened resource in a with statement."

METADATA_DEPENDENCIES = (
Expand All @@ -59,9 +55,7 @@ def __init__(
self.changed_nodes: dict[
cst.CSTNode, cst.CSTNode | cst.RemovalSentinel | cst.FlattenSentinel
] = {}
BaseCodemod.__init__(self, file_context, *codemod_args)
UtilsMixin.__init__(self, [])
Codemod.__init__(self, context)
BaseCodemod.__init__(self, context, file_context, *codemod_args)

def transform_module_impl(self, tree: cst.Module) -> cst.Module:
fr = FindResources(self.context)
Expand Down Expand Up @@ -250,7 +244,7 @@ def _find_direct_name_assignment_targets(
for node in (access.node for access in accesses):
maybe_assigned = self.is_value_of_assignment(node)
if maybe_assigned:
targets = self._extract_targets_of_assignment(maybe_assigned)
targets = extract_targets_of_assignment(maybe_assigned)
name_targets.extend(targets)
return name_targets

Expand Down Expand Up @@ -286,7 +280,7 @@ def _find_transitive_assignment_targets(
maybe_assigned = self.is_value_of_assignment(expr)
if maybe_assigned:
named_targets, other_targets = self._sieve_targets(
self._extract_targets_of_assignment(maybe_assigned)
extract_targets_of_assignment(maybe_assigned)
)
for n in named_targets:
n_named_targets, n_other_targets = self._find_name_assignment_targets(n)
Expand All @@ -295,22 +289,6 @@ def _find_transitive_assignment_targets(
return named_targets, other_targets
return ([], [])

def _extract_targets_of_assignment(
self, assignment: cst.AnnAssign | cst.Assign | cst.WithItem | cst.NamedExpr
) -> list[cst.BaseExpression]:
match assignment:
case cst.AnnAssign():
if assignment.target:
return [assignment.target]
case cst.Assign():
return [t.target for t in assignment.targets]
case cst.NamedExpr():
return [assignment.target]
case cst.WithItem():
if assignment.asname:
return [assignment.asname.name]
return []

# pylint: disable-next=too-many-arguments
def _wrap_in_with_statement(
self,
Expand Down

0 comments on commit 9a3dbc2

Please sign in to comment.