From 29a64790f0f24ac8d36614e0cb191d1fc835db1b Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Mon, 3 Aug 2026 16:52:18 -0400 Subject: [PATCH] Fix first-party CodeQL reliability errors --- news/+code-quality-reliability.bugfix.rst | 2 ++ pipenv/routines/check.py | 3 ++- pipenv/routines/scan.py | 3 ++- pipenv/utils/exceptions.py | 12 ++-------- pipenv/utils/locking.py | 2 +- tests/integration/test_project.py | 6 +++-- tests/unit/test_routine_context.py | 4 ++++ tests/unit/test_utils_exceptions.py | 28 +++++++++++++++++++++++ 8 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 news/+code-quality-reliability.bugfix.rst create mode 100644 tests/unit/test_utils_exceptions.py diff --git a/news/+code-quality-reliability.bugfix.rst b/news/+code-quality-reliability.bugfix.rst new file mode 100644 index 0000000000..3d8efe230c --- /dev/null +++ b/news/+code-quality-reliability.bugfix.rst @@ -0,0 +1,2 @@ +Fixed corrupt Pipfile and lockfile errors so they retain the affected path and +backup location while reporting the file-specific error message. diff --git a/pipenv/routines/check.py b/pipenv/routines/check.py index 1daa2f61a1..0aa5fc0952 100644 --- a/pipenv/routines/check.py +++ b/pipenv/routines/check.py @@ -74,7 +74,8 @@ def run_pep508_check(project, system, python): def check_pep508_requirements(project, results, quiet): - p = plette.Pipfile.load(open(project.pipfile_location)) + with open(project.pipfile_location) as pipfile: + p = plette.Pipfile.load(pipfile) p = plette.Lockfile.with_meta_from(p) failed = False diff --git a/pipenv/routines/scan.py b/pipenv/routines/scan.py index 18bccf02a7..1e31f28361 100644 --- a/pipenv/routines/scan.py +++ b/pipenv/routines/scan.py @@ -160,7 +160,8 @@ def run_pep508_check(project, system, python): def check_pep508_requirements(project, results, quiet): """Verify PEP 508 environment markers in Pipfile match the current environment.""" - p = plette.Pipfile.load(open(project.pipfile_location)) + with open(project.pipfile_location) as pipfile: + p = plette.Pipfile.load(pipfile) p = plette.Lockfile.with_meta_from(p) failed = False diff --git a/pipenv/utils/exceptions.py b/pipenv/utils/exceptions.py index 1729a47a50..1588c837c9 100644 --- a/pipenv/utils/exceptions.py +++ b/pipenv/utils/exceptions.py @@ -56,8 +56,7 @@ def show(self): class LockfileCorruptException(FileCorruptException): def __init__(self, path, backup_path=None): - self.message = self.get_message(path, backup_path=backup_path) - super().__init__(self.message) + super().__init__(path, backup_path=backup_path) def get_message(self, path, backup_path=None): message = f"ERROR: Failed to load lockfile at {path}" @@ -68,14 +67,10 @@ def get_message(self, path, backup_path=None): message = f"{message}\nYour lockfile is corrupt, {msg}" return message - def show(self, path, backup_path=None): - print(self.message, file=sys.stderr, flush=True) - class PipfileCorruptException(FileCorruptException): def __init__(self, path, backup_path=None): - self.message = self.get_message(path, backup_path=backup_path) - super().__init__(self.message) + super().__init__(path, backup_path=backup_path) def get_message(self, path, backup_path=None): message = f"ERROR: Failed to load Pipfile at {path}" @@ -86,9 +81,6 @@ def get_message(self, path, backup_path=None): message = f"{message}\nYour Pipfile is corrupt, {msg}" return message - def show(self, path, backup_path=None): - print(self.message, file=sys.stderr, flush=True) - class PipfileNotFound(FileNotFoundError): def __init__(self, path, *args, **kwargs): diff --git a/pipenv/utils/locking.py b/pipenv/utils/locking.py index b062468f5d..95ed2a1082 100644 --- a/pipenv/utils/locking.py +++ b/pipenv/utils/locking.py @@ -414,7 +414,7 @@ def load(cls, path: Optional[str], create: bool = True) -> "Lockfile": backup_path = f"{formatted_path}.bak" # Show error and create backup - LockfileCorruptException.show(formatted_path, backup_path=backup_path) + LockfileCorruptException(formatted_path, backup_path=backup_path).show() path_obj.rename(backup_path) # Try loading again after backing up corrupted file diff --git a/tests/integration/test_project.py b/tests/integration/test_project.py index 5593b5d64f..e7748e75fe 100644 --- a/tests/integration/test_project.py +++ b/tests/integration/test_project.py @@ -29,8 +29,10 @@ def test_pipfile_envvar_expansion(pipenv_instance_pypi): os.environ["TEST_HOST"] = "localhost:5000" project = Project() assert project.sources.all[0]["url"] == "https://localhost:5000/simple" - assert "localhost:5000" not in str(Pipfile.load(open(p.pipfile_path))) - print(str(Pipfile.load(open(p.pipfile_path)))) + with open(p.pipfile_path) as pipfile: + loaded = Pipfile.load(pipfile) + assert "localhost:5000" not in str(loaded) + print(str(loaded)) @pytest.mark.project diff --git a/tests/unit/test_routine_context.py b/tests/unit/test_routine_context.py index b3f43434cb..43b9a10160 100644 --- a/tests/unit/test_routine_context.py +++ b/tests/unit/test_routine_context.py @@ -290,10 +290,14 @@ class TestFromCliKeywordOnly: def test_from_cli_positional_raises(self): # The first positional after cls would land on `system`. with pytest.raises(TypeError): + # Deliberately violate the keyword-only API to test its contract. + # codeql[py/call/wrong-arguments] RoutineContext.from_cli(True) # type: ignore[misc] def test_from_cli_two_positionals_raises(self): with pytest.raises(TypeError): + # Deliberately violate the keyword-only API to test its contract. + # codeql[py/call/wrong-arguments] RoutineContext.from_cli(True, False) # type: ignore[misc] diff --git a/tests/unit/test_utils_exceptions.py b/tests/unit/test_utils_exceptions.py new file mode 100644 index 0000000000..ebe088a07a --- /dev/null +++ b/tests/unit/test_utils_exceptions.py @@ -0,0 +1,28 @@ +from pipenv.utils.exceptions import ( + LockfileCorruptException, + PipfileCorruptException, +) + + +def test_lockfile_corrupt_exception_preserves_specific_message(capsys, tmp_path): + path = tmp_path / "Pipfile.lock" + backup_path = tmp_path / "Pipfile.lock.bak" + + error = LockfileCorruptException(path, backup_path=backup_path) + error.show() + + assert "Failed to load lockfile" in error.message + assert str(path) in error.message + assert str(backup_path) in error.message + assert error.message in capsys.readouterr().err + + +def test_pipfile_corrupt_exception_preserves_specific_message(capsys, tmp_path): + path = tmp_path / "Pipfile" + + error = PipfileCorruptException(path) + error.show() + + assert "Failed to load Pipfile" in error.message + assert str(path) in error.message + assert error.message in capsys.readouterr().err