Skip to content

Commit

Permalink
All places when a Change() is created should report findings (#734)
Browse files Browse the repository at this point in the history
* wip asserting findings

* report findings
  • Loading branch information
clavedeluna authored Jul 23, 2024
1 parent df0afef commit 5f4f575
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/codemodder/codemods/libcst_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ def add_change_from_position(
Change(
lineNumber=lineno,
description=description,
findings=self.file_context.get_findings_for_location(lineno),
)
)

Expand Down
8 changes: 8 additions & 0 deletions src/codemodder/codemods/test/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

from codemodder import registry
from codemodder.codemods.api import BaseCodemod
from codemodder.codetf import Change
from codemodder.context import CodemodExecutionContext
from codemodder.diff import create_diff
from codemodder.providers import load_providers
Expand Down Expand Up @@ -210,6 +211,8 @@ def run_and_assert(

self.assert_num_changes(changes, num_changes, min_num_changes)

self.assert_findings(changes[0].changes)

self.assert_changes(
tmpdir,
tmp_file_path,
Expand All @@ -219,3 +222,8 @@ def run_and_assert(
)

return changes

def assert_findings(self, changes: list[Change]):
assert all(
x.findings is not None for x in changes
), f"Expected all changes to have findings: {changes}"
18 changes: 14 additions & 4 deletions src/codemodder/codemods/xml_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,12 @@ class XMLTransformer(XMLGenerator, LexicalHandler):
def __init__(
self,
out,
file_context: FileContext,
encoding: str = "utf-8",
short_empty_elements: bool = False,
results: list[Result] | None = None,
) -> None:
self.file_context = file_context
self.results = results
self.changes: list[Change] = []
self._my_locator = Locator()
Expand Down Expand Up @@ -87,7 +89,11 @@ def match_result(self, line, column) -> bool:

def add_change(self, line):
self.changes.append(
Change(lineNumber=line, description=self.change_description)
Change(
lineNumber=line,
description=self.change_description,
findings=self.file_context.get_findings_for_location(line),
)
)


Expand All @@ -99,13 +105,14 @@ class ElementAttributeXMLTransformer(XMLTransformer):
def __init__(
self,
out,
file_context: FileContext,
name_attributes_map: dict[str, dict[str, str]],
encoding: str = "utf-8",
short_empty_elements: bool = False,
results: list[Result] | None = None,
) -> None:
self.name_attributes_map = name_attributes_map
super().__init__(out, encoding, short_empty_elements, results)
super().__init__(out, file_context, encoding, short_empty_elements, results)

def startElement(self, name, attrs):
new_attrs: AttributesImpl = attrs
Expand All @@ -131,12 +138,13 @@ class NewElementXMLTransformer(XMLTransformer):
def __init__(
self,
out,
file_context: FileContext,
encoding: str = "utf-8",
short_empty_elements: bool = False,
results: list[Result] | None = None,
new_elements: list[NewElement] | None = None,
) -> None:
super().__init__(out, encoding, short_empty_elements, results)
super().__init__(out, file_context, encoding, short_empty_elements, results)
self.new_elements = new_elements or []

def startElement(self, name, attrs):
Expand Down Expand Up @@ -175,7 +183,9 @@ def apply(
# this will fail fast for files that are not XML
try:
transformer_instance = self.xml_transformer(
out=output_file, results=results
out=output_file,
file_context=file_context,
results=results,
)
parser = make_parser()
parser.setContentHandler(transformer_instance)
Expand Down
9 changes: 8 additions & 1 deletion src/core_codemods/file_resource_leak.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ def line_filter(x):

for k, v in fr.assigned_resources.items():
fr.assigned_resources[k] = [t for t in v if line_filter(t)]
fixer = ResourceLeakFixer(self.context, fr.assigned_resources)
fixer = ResourceLeakFixer(
self.context, self.file_context, fr.assigned_resources
)
result = tree.visit(fixer)
self.file_context.codemod_changes.extend(fixer.changes)
return result
Expand Down Expand Up @@ -168,6 +170,7 @@ class ResourceLeakFixer(MetadataPreservingTransformer, NameAndAncestorResolution
def __init__(
self,
context: CodemodContext,
file_context: FileContext,
leaked_assigned_resources: dict[
cst.IndentedBlock | cst.Module,
list[
Expand All @@ -182,6 +185,7 @@ def __init__(
super().__init__(context)
self.leaked_assigned_resources = leaked_assigned_resources
self.changes: list[Change] = []
self.file_context = file_context

def _is_fixable(self, block, index, named_targets, other_targets) -> bool:
# assigned to something that is not a Name?
Expand Down Expand Up @@ -225,6 +229,9 @@ def _handle_block(
Change(
lineNumber=line_number,
description=FileResourceLeakTransformer.change_description,
findings=self.file_context.get_findings_for_location(
line_number
),
)
)

Expand Down
3 changes: 2 additions & 1 deletion src/core_codemods/fix_assert_tuple.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ def _report_new_lines(
for idx in range(newlines_count):
self.file_context.codemod_changes.append(
Change(
lineNumber=start_line + idx,
lineNumber=(line_number := start_line + idx),
description=self.change_description,
findings=self.file_context.get_findings_for_location(line_number),
)
)

Expand Down
3 changes: 3 additions & 0 deletions src/core_codemods/sql_parameterization.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,9 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
Change(
lineNumber=line_number,
description=SQLQueryParameterizationTransformer.change_description,
findings=self.file_context.get_findings_for_location(
line_number
),
)
)
# Normalization and cleanup
Expand Down
7 changes: 7 additions & 0 deletions src/core_codemods/url_sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ def __init__(
cst.CSTNode, Union[cst.CSTNode, cst.FlattenSentinel, cst.RemovalSentinel]
] = {}
self.changes_in_file: List[Change] = []
self.file_context = file_context
ContextAwareVisitor.__init__(self, codemod_context)
UtilsMixin.__init__(
self,
Expand Down Expand Up @@ -109,6 +110,9 @@ def leave_Call(self, original_node: cst.Call):
Change(
lineNumber=line_number,
description=UrlSandboxTransformer.change_description,
findings=self.file_context.get_findings_for_location(
line_number
),
)
)

Expand All @@ -126,6 +130,9 @@ def leave_Call(self, original_node: cst.Call):
Change(
lineNumber=line_number,
description=UrlSandboxTransformer.change_description,
findings=self.file_context.get_findings_for_location(
line_number
),
)
)

Expand Down
9 changes: 6 additions & 3 deletions tests/test_xml_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from textwrap import dedent
from xml.sax import handler

import mock
import pytest
from defusedxml import ExternalReferenceForbidden
from defusedxml.sax import make_parser
Expand All @@ -19,7 +20,7 @@ class TestXMLTransformer:
def run_and_assert(self, input_code, expected_output):
with StringIO() as result, StringIO(dedent(input_code)) as input_stream:
result = StringIO()
transformer = XMLTransformer(result)
transformer = XMLTransformer(result, mock.MagicMock())
parser = make_parser()
parser.setContentHandler(transformer)
parser.setProperty(handler.property_lexical_handler, transformer)
Expand Down Expand Up @@ -58,7 +59,7 @@ class TestElementAttributeXMLTransformer:
def run_and_assert(self, name_attr_map, input_code, expected_output):
with StringIO() as result, StringIO(dedent(input_code)) as input_stream:
transformer = ElementAttributeXMLTransformer(
result, name_attributes_map=name_attr_map
result, mock.MagicMock(), name_attributes_map=name_attr_map
)
parser = make_parser()
parser.setContentHandler(transformer)
Expand Down Expand Up @@ -101,7 +102,9 @@ class TestNewElementXMLTransformer:

def run_and_assert(self, new_elements, input_code, expected_output):
with StringIO() as result, StringIO(dedent(input_code)) as input_stream:
transformer = NewElementXMLTransformer(result, new_elements=new_elements)
transformer = NewElementXMLTransformer(
result, mock.MagicMock(), new_elements=new_elements
)
parser = make_parser()
parser.setContentHandler(transformer)
parser.setProperty(handler.property_lexical_handler, transformer)
Expand Down

0 comments on commit 5f4f575

Please sign in to comment.