From 0611a89eb6a402a5f39669b753549c5fc8feb4c2 Mon Sep 17 00:00:00 2001 From: Dan D'Avella Date: Fri, 22 Mar 2024 12:21:15 -0400 Subject: [PATCH] Fix generated diff when adding hashes to requirements.txt (#403) --- .../test_flask_enable_csrf_protection.py | 17 ++++--- integration_tests/test_harden_pickle_load.py | 17 ++++--- integration_tests/test_process_sandbox.py | 17 ++++--- integration_tests/test_url_sandbox.py | 17 ++++--- integration_tests/test_use_defusedxml.py | 17 ++++--- src/codemodder/dependency.py | 4 +- .../requirements_txt_writer.py | 8 ++-- .../test_requirements_txt_writer.py | 44 +++++++++++++++---- 8 files changed, 92 insertions(+), 49 deletions(-) diff --git a/integration_tests/test_flask_enable_csrf_protection.py b/integration_tests/test_flask_enable_csrf_protection.py index 12914205..caa1834f 100644 --- a/integration_tests/test_flask_enable_csrf_protection.py +++ b/integration_tests/test_flask_enable_csrf_protection.py @@ -39,11 +39,14 @@ class TestFlaskEnableCSRFProtection(BaseIntegrationTest): requirements_path = "tests/samples/requirements.txt" original_requirements = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\n" expected_new_reqs = ( - "# file used to test dependency management\n" - "requests==2.31.0\n" - "black==23.7.*\n" - "mypy~=1.4\n" - "pylint>1\n" - f"{FlaskWTF.requirement} \\\n" - f"{FlaskWTF.build_hashes()}" + ( + "# file used to test dependency management\n" + "requests==2.31.0\n" + "black==23.7.*\n" + "mypy~=1.4\n" + "pylint>1\n" + f"{FlaskWTF.requirement} \\\n" + ) + + "\n".join(FlaskWTF.build_hashes()) + + "\n" ) diff --git a/integration_tests/test_harden_pickle_load.py b/integration_tests/test_harden_pickle_load.py index 8e77a819..f72999a1 100644 --- a/integration_tests/test_harden_pickle_load.py +++ b/integration_tests/test_harden_pickle_load.py @@ -41,11 +41,14 @@ class TestHardenPickleLoad(BaseIntegrationTest): requirements_path = "tests/samples/requirements.txt" original_requirements = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\n" expected_new_reqs = ( - "# file used to test dependency management\n" - "requests==2.31.0\n" - "black==23.7.*\n" - "mypy~=1.4\n" - "pylint>1\n" - f"{Fickling.requirement} \\\n" - f"{Fickling.build_hashes()}" + ( + "# file used to test dependency management\n" + "requests==2.31.0\n" + "black==23.7.*\n" + "mypy~=1.4\n" + "pylint>1\n" + f"{Fickling.requirement} \\\n" + ) + + "\n".join(Fickling.build_hashes()) + + "\n" ) diff --git a/integration_tests/test_process_sandbox.py b/integration_tests/test_process_sandbox.py index 5f0cfa4e..0c6fa9a4 100644 --- a/integration_tests/test_process_sandbox.py +++ b/integration_tests/test_process_sandbox.py @@ -28,11 +28,14 @@ class TestProcessSandbox(BaseIntegrationTest): requirements_path = "tests/samples/requirements.txt" original_requirements = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\n" expected_new_reqs = ( - "# file used to test dependency management\n" - "requests==2.31.0\n" - "black==23.7.*\n" - "mypy~=1.4\n" - "pylint>1\n" - f"{Security.requirement} \\\n" - f"{Security.build_hashes()}" + ( + "# file used to test dependency management\n" + "requests==2.31.0\n" + "black==23.7.*\n" + "mypy~=1.4\n" + "pylint>1\n" + f"{Security.requirement} \\\n" + ) + + "\n".join(Security.build_hashes()) + + "\n" ) diff --git a/integration_tests/test_url_sandbox.py b/integration_tests/test_url_sandbox.py index be4e8b13..aa4576cb 100644 --- a/integration_tests/test_url_sandbox.py +++ b/integration_tests/test_url_sandbox.py @@ -38,11 +38,14 @@ class TestUrlSandbox(BaseIntegrationTest): requirements_path = "tests/samples/requirements.txt" original_requirements = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\n" expected_new_reqs = ( - "# file used to test dependency management\n" - "requests==2.31.0\n" - "black==23.7.*\n" - "mypy~=1.4\n" - "pylint>1\n" - f"{Security.requirement} \\\n" - f"{Security.build_hashes()}" + ( + "# file used to test dependency management\n" + "requests==2.31.0\n" + "black==23.7.*\n" + "mypy~=1.4\n" + "pylint>1\n" + f"{Security.requirement} \\\n" + ) + + "\n".join(Security.build_hashes()) + + "\n" ) diff --git a/integration_tests/test_use_defusedxml.py b/integration_tests/test_use_defusedxml.py index a1eca25c..ae33e8be 100644 --- a/integration_tests/test_use_defusedxml.py +++ b/integration_tests/test_use_defusedxml.py @@ -41,11 +41,14 @@ class TestUseDefusedXml(BaseIntegrationTest): requirements_path = "tests/samples/requirements.txt" original_requirements = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\n" expected_new_reqs = ( - "# file used to test dependency management\n" - "requests==2.31.0\n" - "black==23.7.*\n" - "mypy~=1.4\n" - "pylint>1\n" - f"{DefusedXML.requirement} \\\n" - f"{DefusedXML.build_hashes()}" + ( + "# file used to test dependency management\n" + "requests==2.31.0\n" + "black==23.7.*\n" + "mypy~=1.4\n" + "pylint>1\n" + f"{DefusedXML.requirement} \\\n" + ) + + "\n".join(DefusedXML.build_hashes()) + + "\n" ) diff --git a/src/codemodder/dependency.py b/src/codemodder/dependency.py index d55d0e94..b26aca7b 100644 --- a/src/codemodder/dependency.py +++ b/src/codemodder/dependency.py @@ -34,8 +34,8 @@ def build_description(self) -> str: [More facts]({self.package_link}) """ - def build_hashes(self) -> str: - return " \\\n".join(f"{' '*4}--hash=sha256:{sha256}" for sha256 in self.hashes) + def build_hashes(self) -> list[str]: + return [f"{' '*4}--hash=sha256:{sha256}" for sha256 in self.hashes] def __hash__(self): return hash(self.requirement) diff --git a/src/codemodder/dependency_management/requirements_txt_writer.py b/src/codemodder/dependency_management/requirements_txt_writer.py index 646bdd93..656da5f9 100644 --- a/src/codemodder/dependency_management/requirements_txt_writer.py +++ b/src/codemodder/dependency_management/requirements_txt_writer.py @@ -21,9 +21,11 @@ def add_to_file( if not original_lines[-1].endswith("\n"): original_lines[-1] += "\n" - requirement_lines = [ - f"{dep.requirement} \\\n{dep.build_hashes()}" for dep in dependencies - ] + requirement_lines = [] + for dep in dependencies: + requirement_lines.append(f"{dep.requirement} \\\n") + for hash_line in dep.build_hashes(): + requirement_lines.append(f"{hash_line}\n") updated_lines = original_lines + requirement_lines diff --git a/tests/dependency_management/test_requirements_txt_writer.py b/tests/dependency_management/test_requirements_txt_writer.py index dd7c68bc..08381b38 100644 --- a/tests/dependency_management/test_requirements_txt_writer.py +++ b/tests/dependency_management/test_requirements_txt_writer.py @@ -32,22 +32,35 @@ def test_add_dependencies_preserve_comments(self, tmpdir, dry_run): assert dependency_file.read_text(encoding="utf-8") == ( contents if dry_run - else f"# comment\n\nrequests\n{DefusedXML.requirement} \\\n{DefusedXML.build_hashes()}{Security.requirement} \\\n{Security.build_hashes()}" + else ( + "# comment\n\nrequests\n" + + f"{DefusedXML.requirement} \\\n" + + "\n".join(DefusedXML.build_hashes()) + + "\n" + + f"{Security.requirement} \\\n" + + "\n".join(Security.build_hashes()) + + "\n" + ) ) assert changeset is not None assert changeset.path == dependency_file.name + + defused_xml_hashes = DefusedXML.build_hashes() + security_hashes = Security.build_hashes() assert changeset.diff == ( "--- \n" "+++ \n" - "@@ -1,3 +1,5 @@\n" + "@@ -1,3 +1,9 @@\n" " # comment\n" " \n" " requests\n" f"+{DefusedXML.requirement} \\\n" - f"{DefusedXML.build_hashes()}\n" + f"+{defused_xml_hashes[0]}\n" + f"+{defused_xml_hashes[1]}\n" f"+{Security.requirement} \\\n" - f"{Security.build_hashes()}" + f"+{security_hashes[0]}\n" + f"+{security_hashes[1]}\n" ) assert len(changeset.changes) == 2 change_one = changeset.changes[0] @@ -83,7 +96,9 @@ def test_add_same_dependency_only_once(self, tmpdir): assert len(changeset.changes) == 1 assert dependency_file.read_text(encoding="utf-8") == ( - f"requests\n{Security.requirement} \\\n{Security.build_hashes()}" + f"requests\n{Security.requirement} \\\n" + + "\n".join(Security.build_hashes()) + + "\n" ) def test_dont_add_existing_dependency(self, tmpdir): @@ -140,20 +155,31 @@ def test_dependency_file_no_terminating_newline(self, tmpdir): assert ( dependency_file.read_text(encoding="utf-8") - == f"# comment\n\nrequests\n{DefusedXML.requirement} \\\n{DefusedXML.build_hashes()}{Security.requirement} \\\n{Security.build_hashes()}" + == "# comment\n\nrequests\n" + + f"{DefusedXML.requirement} \\\n" + + "\n".join(DefusedXML.build_hashes()) + + "\n" + + f"{Security.requirement} \\\n" + + "\n".join(Security.build_hashes()) + + "\n" ) assert changeset is not None assert changeset.path == dependency_file.name + + defused_xml_hashes = DefusedXML.build_hashes() + security_hashes = Security.build_hashes() assert changeset.diff == ( "--- \n" "+++ \n" - "@@ -1,3 +1,5 @@\n" + "@@ -1,3 +1,9 @@\n" " # comment\n" " \n" " requests\n" f"+{DefusedXML.requirement} \\\n" - f"{DefusedXML.build_hashes()}\n" + f"+{defused_xml_hashes[0]}\n" + f"+{defused_xml_hashes[1]}\n" f"+{Security.requirement} \\\n" - f"{Security.build_hashes()}" + f"+{security_hashes[0]}\n" + f"+{security_hashes[1]}\n" )