Fix dependency manager and add more/better tests

integration_tests/test_process_sandbox.py

@@ -26,4 +26,4 @@ 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\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity~=1.2.0"
+    expected_new_reqs = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity~=1.2.0\n"

integration_tests/test_url_sandbox.py

@@ -23,4 +23,4 @@ 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\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity~=1.2.0"
+    expected_new_reqs = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity~=1.2.0\n"

integration_tests/test_use_defusedxml.py

@@ -39,4 +39,4 @@ 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\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\ndefusedxml~=0.7.1"
+    expected_new_reqs = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\ndefusedxml~=0.7.1\n"

src/codemodder/dependency_manager.py

@@ -28,8 +28,8 @@ def new_requirements(self) -> list[str]:
     def add(self, dependencies: list[Dependency]):
         """add any number of dependencies to the end of list of dependencies."""
         for dep in dependencies:
-            if dep not in self.dependencies:
-                self.dependencies.update({dep.requirement: None})
+            if dep.requirement.name not in self.dependencies:
+                self.dependencies.update({dep.requirement.name: dep.requirement})
                 self._new_requirements.append(dep)
 
     def write(self, dry_run: bool = False) -> Optional[ChangeSet]:
@@ -58,7 +58,7 @@ def write(self, dry_run: bool = False) -> Optional[ChangeSet]:
         if not dry_run:
             with open(self.dependency_file, "w", encoding="utf-8") as f:
                 f.writelines(self._lines)
-                f.writelines(self.new_requirements)
+                f.writelines([f"{line}\n" for line in self.new_requirements])
 
         self.dependency_file_changed = True
         return ChangeSet(str(self.dependency_file), diff, changes=changes)
@@ -77,7 +77,7 @@ def dependency_file(self) -> Optional[Path]:
         return None
 
     @cached_property
-    def dependencies(self) -> dict[Requirement, None]:
+    def dependencies(self) -> dict[str, Requirement]:
         """
         Extract list of dependencies from requirements.txt file.
         Same order of requirements is maintained, no alphabetical sorting is done.
@@ -89,8 +89,10 @@ def dependencies(self) -> dict[Requirement, None]:
             self._lines = f.readlines()
 
         return {
-            Requirement(line): None
+            requirement.name: requirement
             for x in self._lines
             # Skip empty lines and comments
-            if (line := x.strip()) and not line.startswith("#")
+            if (line := x.strip())
+            and not line.startswith("#")
+            and (requirement := Requirement(line))
         }

tests/test_dependency_manager.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from codemodder.dependency import DefusedXML
+from codemodder.dependency import DefusedXML, Security
 from codemodder.dependency_manager import DependencyManager, Requirement
 
 
@@ -19,7 +19,7 @@ def test_read_dependency_file(self, tmpdir):
         dependency_file.write_text("requests\n", encoding="utf-8")
 
         dm = DependencyManager(Path(tmpdir))
-        assert dm.dependencies == {Requirement("requests"): None}
+        assert dm.dependencies == {"requests": Requirement("requests")}
 
     @pytest.mark.parametrize("dry_run", [True, False])
     def test_add_dependency_preserve_comments(self, tmpdir, dry_run):
@@ -32,7 +32,7 @@ def test_add_dependency_preserve_comments(self, tmpdir, dry_run):
         changeset = dm.write(dry_run=dry_run)
 
         assert dependency_file.read_text(encoding="utf-8") == (
-            contents if dry_run else "# comment\n\nrequests\ndefusedxml~=0.7.1"
+            contents if dry_run else "# comment\n\nrequests\ndefusedxml~=0.7.1\n"
         )
 
         assert changeset is not None
@@ -50,3 +50,42 @@ def test_add_dependency_preserve_comments(self, tmpdir, dry_run):
         assert changeset.changes[0].lineNumber == 4
         assert changeset.changes[0].description == DefusedXML.build_description()
         assert changeset.changes[0].properties == {"contextual_description": True}
+
+    def test_add_multiple_dependencies(self, tmpdir):
+        dependency_file = Path(tmpdir) / "requirements.txt"
+        dependency_file.write_text("requests\n", encoding="utf-8")
+
+        for dep in [DefusedXML, Security]:
+            dm = DependencyManager(Path(tmpdir))
+            dm.add([dep])
+            dm.write()
+
+        assert dependency_file.read_text(encoding="utf-8") == (
+            "requests\ndefusedxml~=0.7.1\nsecurity~=1.2.0\n"
+        )
+
+    def test_add_same_dependency_only_once(self, tmpdir):
+        dependency_file = Path(tmpdir) / "requirements.txt"
+        dependency_file.write_text("requests\n", encoding="utf-8")
+
+        for dep in [Security, Security]:
+            dm = DependencyManager(Path(tmpdir))
+            dm.add([dep])
+            dm.write()
+
+        assert dependency_file.read_text(encoding="utf-8") == (
+            "requests\nsecurity~=1.2.0\n"
+        )
+
+    @pytest.mark.parametrize("version", ["1.2.0", "1.0.1"])
+    def test_dont_add_existing_dependency(self, version, tmpdir):
+        dependency_file = Path(tmpdir) / "requirements.txt"
+        dependency_file.write_text(f"requests\nsecurity~={version}\n", encoding="utf-8")
+
+        dm = DependencyManager(Path(tmpdir))
+        dm.add([Security])
+        dm.write()
+
+        assert dependency_file.read_text(encoding="utf-8") == (
+            f"requests\nsecurity~={version}\n"
+        )