Fix generated diff when adding hashes to requirements.txt (#403)

Author: drdavella

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"
     )

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"
     )

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"
     )

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"
     )

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"
     )

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)

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
 

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"
         )