From 5f4f5753378770eaf09cd2b1033b13b8a8e02acf Mon Sep 17 00:00:00 2001 From: Dani Alcala <112832187+clavedeluna@users.noreply.github.com> Date: Tue, 23 Jul 2024 14:04:55 -0300 Subject: [PATCH] All places when a Change() is created should report findings (#734) * wip asserting findings * report findings --- src/codemodder/codemods/libcst_transformer.py | 1 + src/codemodder/codemods/test/utils.py | 8 ++++++++ src/codemodder/codemods/xml_transformer.py | 18 ++++++++++++++---- src/core_codemods/file_resource_leak.py | 9 ++++++++- src/core_codemods/fix_assert_tuple.py | 3 ++- src/core_codemods/sql_parameterization.py | 3 +++ src/core_codemods/url_sandbox.py | 7 +++++++ tests/test_xml_transformer.py | 9 ++++++--- 8 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/codemodder/codemods/libcst_transformer.py b/src/codemodder/codemods/libcst_transformer.py index ad1f807c..54c99bea 100644 --- a/src/codemodder/codemods/libcst_transformer.py +++ b/src/codemodder/codemods/libcst_transformer.py @@ -109,6 +109,7 @@ def add_change_from_position( Change( lineNumber=lineno, description=description, + findings=self.file_context.get_findings_for_location(lineno), ) ) diff --git a/src/codemodder/codemods/test/utils.py b/src/codemodder/codemods/test/utils.py index 82c2ce3d..44246c51 100644 --- a/src/codemodder/codemods/test/utils.py +++ b/src/codemodder/codemods/test/utils.py @@ -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 @@ -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, @@ -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}" diff --git a/src/codemodder/codemods/xml_transformer.py b/src/codemodder/codemods/xml_transformer.py index 2c6d16e9..b7715cd4 100644 --- a/src/codemodder/codemods/xml_transformer.py +++ b/src/codemodder/codemods/xml_transformer.py @@ -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() @@ -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), + ) ) @@ -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 @@ -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): @@ -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) diff --git a/src/core_codemods/file_resource_leak.py b/src/core_codemods/file_resource_leak.py index 288b4008..89fab3ec 100644 --- a/src/core_codemods/file_resource_leak.py +++ b/src/core_codemods/file_resource_leak.py @@ -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 @@ -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[ @@ -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? @@ -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 + ), ) ) diff --git a/src/core_codemods/fix_assert_tuple.py b/src/core_codemods/fix_assert_tuple.py index 8dce30bb..d0c4a791 100644 --- a/src/core_codemods/fix_assert_tuple.py +++ b/src/core_codemods/fix_assert_tuple.py @@ -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), ) ) diff --git a/src/core_codemods/sql_parameterization.py b/src/core_codemods/sql_parameterization.py index 67cdab3d..5c87073c 100644 --- a/src/core_codemods/sql_parameterization.py +++ b/src/core_codemods/sql_parameterization.py @@ -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 diff --git a/src/core_codemods/url_sandbox.py b/src/core_codemods/url_sandbox.py index 6bf2e44a..9e217fa4 100644 --- a/src/core_codemods/url_sandbox.py +++ b/src/core_codemods/url_sandbox.py @@ -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, @@ -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 + ), ) ) @@ -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 + ), ) ) diff --git a/tests/test_xml_transformer.py b/tests/test_xml_transformer.py index 61cca2ea..8329863c 100644 --- a/tests/test_xml_transformer.py +++ b/tests/test_xml_transformer.py @@ -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 @@ -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) @@ -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) @@ -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)