From a9ecfe174953f12c71b93466995d820c223b6e3a Mon Sep 17 00:00:00 2001 From: Daniel D'Avella Date: Mon, 5 Feb 2024 17:54:45 -0500 Subject: [PATCH] Add default value to Change.diffSide in CodeTF --- src/codemodder/change.py | 11 ++++++ .../base_dependency_writer.py | 4 ++ src/codemodder/report/codetf_reporter.py | 1 + .../test_pyproject_writer.py | 11 ++++-- .../test_requirements_txt_writer.py | 9 ++++- .../test_setup_py_writer.py | 25 +++++++----- .../test_setupcfgt_writer.py | 19 +++++---- tests/report/test_codetf_reporter.py | 2 + tests/test_change.py | 39 +++++++++++++++++++ 9 files changed, 99 insertions(+), 22 deletions(-) create mode 100644 tests/test_change.py diff --git a/src/codemodder/change.py b/src/codemodder/change.py index 993ac74e..25c012d7 100644 --- a/src/codemodder/change.py +++ b/src/codemodder/change.py @@ -13,6 +13,11 @@ class Result(Enum): SKIPPED = "skipped" +class DiffSide(Enum): + LEFT = "left" + RIGHT = "right" + + @dataclass class PackageAction: action: Action @@ -31,6 +36,11 @@ def to_json(self): class Change: lineNumber: int description: str + # All of our changes are currently treated as additive, so it makes sense + # for the comments to appear on the RIGHT side of the split diff. Eventually we + # may want to differentiate between LEFT and RIGHT, but for now we'll just + # default to RIGHT. + diffSide: DiffSide = field(default=DiffSide.RIGHT) properties: dict = field(default_factory=dict) packageActions: list[PackageAction] = field(default_factory=list) @@ -40,6 +50,7 @@ def to_json(self): "lineNumber": str(self.lineNumber), "description": self.description, "properties": self.properties, + "diffSide": self.diffSide.value.lower(), "packageActions": [pa.to_json() for pa in self.packageActions], } diff --git a/src/codemodder/dependency_management/base_dependency_writer.py b/src/codemodder/dependency_management/base_dependency_writer.py index 060835d8..debcb263 100644 --- a/src/codemodder/dependency_management/base_dependency_writer.py +++ b/src/codemodder/dependency_management/base_dependency_writer.py @@ -53,6 +53,10 @@ def build_changes( # Contextual comments should be added to the right side of split diffs properties={ "contextual_description": True, + # TODO: `contextual_description_position` is deprecated in + # favor of Change.diffSide. + # We're keeping it here for backwards compatibility but it + # should eventually be removed. "contextual_description_position": "right", }, packageActions=[ diff --git a/src/codemodder/report/codetf_reporter.py b/src/codemodder/report/codetf_reporter.py index 0e4c3a30..5ee15e5e 100644 --- a/src/codemodder/report/codetf_reporter.py +++ b/src/codemodder/report/codetf_reporter.py @@ -1,5 +1,6 @@ import json from os.path import abspath + from codemodder import __version__ from codemodder.logging import logger diff --git a/tests/dependency_management/test_pyproject_writer.py b/tests/dependency_management/test_pyproject_writer.py index d872423c..ec819021 100644 --- a/tests/dependency_management/test_pyproject_writer.py +++ b/tests/dependency_management/test_pyproject_writer.py @@ -1,6 +1,7 @@ from textwrap import dedent import pytest +from codemodder.change import DiffSide from codemodder.dependency_management.pyproject_writer import PyprojectWriter from codemodder.dependency import DefusedXML, Security from codemodder.project_analysis.file_parsers.package_store import ( @@ -35,7 +36,7 @@ def test_update_pyproject_dependencies(tmpdir, dry_run): store = PackageStore( type=FileType.REQ_TXT, - file=str(pyproject_toml), + file=pyproject_toml, dependencies=set(), py_versions=[">=3.10.0"], ) @@ -88,6 +89,7 @@ def test_update_pyproject_dependencies(tmpdir, dry_run): assert change_one.lineNumber == 16 assert change_one.description == DefusedXML.build_description() + assert change_one.diffSide == DiffSide.RIGHT assert change_one.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -95,6 +97,7 @@ def test_update_pyproject_dependencies(tmpdir, dry_run): change_two = changeset.changes[1] assert change_two.lineNumber == 17 assert change_two.description == Security.build_description() + assert change_two.diffSide == DiffSide.RIGHT assert change_two.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -124,7 +127,7 @@ def test_add_same_dependency_only_once(tmpdir): store = PackageStore( type=FileType.REQ_TXT, - file=str(pyproject_toml), + file=pyproject_toml, dependencies=set(), py_versions=[">=3.10.0"], ) @@ -178,7 +181,7 @@ def test_dont_add_existing_dependency(tmpdir): store = PackageStore( type=FileType.REQ_TXT, - file=str(pyproject_toml), + file=pyproject_toml, dependencies=set([Security.requirement]), py_versions=[">=3.10.0"], ) @@ -204,7 +207,7 @@ def test_pyproject_no_dependencies(tmpdir): store = PackageStore( type=FileType.REQ_TXT, - file=str(pyproject_toml), + file=pyproject_toml, dependencies=set(), py_versions=[">=3.10.0"], ) diff --git a/tests/dependency_management/test_requirements_txt_writer.py b/tests/dependency_management/test_requirements_txt_writer.py index 3607c2ce..ee04529f 100644 --- a/tests/dependency_management/test_requirements_txt_writer.py +++ b/tests/dependency_management/test_requirements_txt_writer.py @@ -1,5 +1,8 @@ -import pytest from pathlib import Path + +import pytest + +from codemodder.change import DiffSide from codemodder.dependency_management.requirements_txt_writer import ( RequirementsTxtWriter, ) @@ -48,6 +51,7 @@ def test_add_dependencies_preserve_comments(self, tmpdir, dry_run): change_one = changeset.changes[0] assert change_one.lineNumber == 4 assert change_one.description == DefusedXML.build_description() + assert change_one.diffSide == DiffSide.RIGHT assert change_one.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -55,6 +59,7 @@ def test_add_dependencies_preserve_comments(self, tmpdir, dry_run): change_two = changeset.changes[1] assert change_two.lineNumber == 5 assert change_two.description == Security.build_description() + assert change_two.diffSide == DiffSide.RIGHT assert change_two.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -123,7 +128,7 @@ def test_dependency_file_no_terminating_newline(self, tmpdir): store = PackageStore( type=FileType.REQ_TXT, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[], ) diff --git a/tests/dependency_management/test_setup_py_writer.py b/tests/dependency_management/test_setup_py_writer.py index affc5955..59c45a18 100644 --- a/tests/dependency_management/test_setup_py_writer.py +++ b/tests/dependency_management/test_setup_py_writer.py @@ -1,5 +1,8 @@ -import pytest from textwrap import dedent + +import pytest + +from codemodder.change import DiffSide from codemodder.dependency_management.setup_py_writer import SetupPyWriter from codemodder.project_analysis.file_parsers.package_store import ( PackageStore, @@ -34,7 +37,7 @@ def test_update_setuppy_comma_single_element_newline(tmpdir): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -138,7 +141,7 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -191,6 +194,7 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): assert change_one.lineNumber == 14 assert change_one.description == DefusedXML.build_description() + assert change_one.diffSide == DiffSide.RIGHT assert change_one.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -198,6 +202,7 @@ def test_update_setuppy_dependencies(tmpdir, dry_run): change_two = changeset.changes[1] assert change_two.lineNumber == 14 assert change_two.description == Security.build_description() + assert change_two.diffSide == DiffSide.RIGHT assert change_two.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -224,7 +229,7 @@ def test_other_setup_func(tmpdir): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -262,7 +267,7 @@ def test_not_setup_file(tmpdir): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -293,7 +298,7 @@ def test_setup_call_no_install_requires(tmpdir): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -325,7 +330,7 @@ def test_setup_no_existing_requirements(tmpdir): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -357,7 +362,7 @@ def test_setup_call_bad_install_requires(tmpdir): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -397,7 +402,7 @@ def test_setup_call_requirements_separate(tmpdir): store = PackageStore( type=FileType.SETUP_PY, - file=str(dependency_file), + file=dependency_file, dependencies=set(), py_versions=[">=3.6"], ) @@ -453,6 +458,7 @@ def test_setup_call_requirements_separate(tmpdir): assert change_one.lineNumber == 14 assert change_one.description == DefusedXML.build_description() + assert change_one.diffSide == DiffSide.RIGHT assert change_one.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -460,6 +466,7 @@ def test_setup_call_requirements_separate(tmpdir): change_two = changeset.changes[1] assert change_two.lineNumber == 14 assert change_two.description == Security.build_description() + assert change_two.diffSide == DiffSide.RIGHT assert change_two.properties == { "contextual_description": True, "contextual_description_position": "right", diff --git a/tests/dependency_management/test_setupcfgt_writer.py b/tests/dependency_management/test_setupcfgt_writer.py index c392ad5c..48b665ac 100644 --- a/tests/dependency_management/test_setupcfgt_writer.py +++ b/tests/dependency_management/test_setupcfgt_writer.py @@ -1,7 +1,9 @@ from textwrap import dedent + import mock import pytest +from codemodder.change import DiffSide from codemodder.dependency_management.setupcfg_writer import SetupCfgWriter from codemodder.dependency import DefusedXML, Security from codemodder.project_analysis.file_parsers.package_store import ( @@ -32,7 +34,7 @@ def test_update_dependencies(tmpdir, dry_run): store = PackageStore( type=FileType.SETUP_CFG, - file=str(setup_cfg), + file=setup_cfg, dependencies=set(), py_versions=[">=3.7"], ) @@ -80,6 +82,7 @@ def test_update_dependencies(tmpdir, dry_run): assert change_one.lineNumber == 13 assert change_one.description == DefusedXML.build_description() + assert change_one.diffSide == DiffSide.RIGHT assert change_one.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -87,6 +90,7 @@ def test_update_dependencies(tmpdir, dry_run): change_two = changeset.changes[1] assert change_two.lineNumber == 14 assert change_two.description == Security.build_description() + assert change_two.diffSide == DiffSide.RIGHT assert change_two.properties == { "contextual_description": True, "contextual_description_position": "right", @@ -114,7 +118,7 @@ def test_add_same_dependency_only_once(tmpdir): store = PackageStore( type=FileType.SETUP_CFG, - file=str(setup_cfg), + file=setup_cfg, dependencies=set(), py_versions=[">=3.7"], ) @@ -164,7 +168,7 @@ def test_dont_add_existing_dependency(tmpdir): store = PackageStore( type=FileType.SETUP_CFG, - file=str(setup_cfg), + file=setup_cfg, dependencies=set([Security.requirement]), py_versions=[">=3.7"], ) @@ -194,7 +198,7 @@ def test_no_dependencies(tmpdir): store = PackageStore( type=FileType.SETUP_CFG, - file=str(setup_cfg), + file=setup_cfg, dependencies=set(), py_versions=[">=3.7"], ) @@ -227,7 +231,7 @@ def test_cfg_bad_formatting(tmpdir): store = PackageStore( type=FileType.SETUP_CFG, - file=str(setup_cfg), + file=setup_cfg, dependencies=set(), py_versions=[">=3.7"], ) @@ -263,7 +267,7 @@ def test_cfg_cant_build_newlines(_, tmpdir): store = PackageStore( type=FileType.SETUP_CFG, - file=str(setup_cfg), + file=setup_cfg, dependencies=set(), py_versions=[">=3.7"], ) @@ -293,7 +297,7 @@ def test_cfg_inline_dependencies(tmpdir): store = PackageStore( type=FileType.SETUP_CFG, - file=str(setup_cfg), + file=setup_cfg, dependencies=set(), py_versions=[">=3.7"], ) @@ -333,6 +337,7 @@ def test_cfg_inline_dependencies(tmpdir): assert change_one.lineNumber == 10 assert change_one.description == Security.build_description() + assert change_one.diffSide == DiffSide.RIGHT assert change_one.properties == { "contextual_description": True, "contextual_description_position": "right", diff --git a/tests/report/test_codetf_reporter.py b/tests/report/test_codetf_reporter.py index 3281b019..e128f486 100644 --- a/tests/report/test_codetf_reporter.py +++ b/tests/report/test_codetf_reporter.py @@ -1,6 +1,8 @@ import json + import mock import pytest + from codemodder.report.codetf_reporter import CodeTF diff --git a/tests/test_change.py b/tests/test_change.py new file mode 100644 index 00000000..a1729e8b --- /dev/null +++ b/tests/test_change.py @@ -0,0 +1,39 @@ +import pytest + +from codemodder.change import Change, ChangeSet, DiffSide + + +def test_change(): + diff = "--- a/test\n+++ b/test\n@@ -1,1 +1,1 @@\n-1\n+2\n" + changeset = ChangeSet( + path="test", + diff=diff, + changes=[ + Change( + lineNumber=1, + description="Change 1 to 2", + ), + ], + ) + + result = changeset.to_json() + + assert result["path"] == "test" + assert result["diff"] == diff + assert result["changes"][0]["lineNumber"] == str(1) + assert result["changes"][0]["description"] == "Change 1 to 2" + assert result["changes"][0]["diffSide"] == "right" + assert result["changes"][0]["properties"] == {} + assert result["changes"][0]["packageActions"] == [] + + +@pytest.mark.parametrize("side", [DiffSide.LEFT, DiffSide.RIGHT]) +def test_change_diffside(side): + change = Change( + lineNumber=1, + description="Change 1 to 2", + diffSide=side, + ) + + assert change.diffSide == side + assert change.to_json()["diffSide"] == side.value