Skip to content

Support diffing against a non-existing composer.lock file#46

Open
wikando-ck wants to merge 4 commits into
IonBazan:mainfrom
wikando-ck:feat/allow-empty-lockfile-diff
Open

Support diffing against a non-existing composer.lock file#46
wikando-ck wants to merge 4 commits into
IonBazan:mainfrom
wikando-ck:feat/allow-empty-lockfile-diff

Conversation

@wikando-ck
Copy link
Copy Markdown

@wikando-ck wikando-ck commented Mar 18, 2026

Summary

When comparing against a composer.lock that doesn't exist yet (e.g. a path on a branch where no lockfile has been committed), the diff now treats it as an empty lockfile ({}) instead of throwing an error. All packages in the target are reported as new installs.

This covers two scenarios:

  • Local file path — the path looks like a composer.lock file but doesn't exist on disk (e.g. composer diff /new/project/composer.lock composer.lock)
  • Git ref path — the file doesn't exist in the referenced commit (e.g. composer diff HEAD:new-dir/composer.lock)

Motivation

We use composer-bin to manage tooling in separate directories away from the main composer.json/composer.lock. When a new tool directory is introduced on a branch, there is no corresponding composer.lock on the base branch yet. Running composer diff against that non-existing base lockfile currently throws an error, making it impossible to review the newly added dependencies.

Changes

  • Handle non-existing local composer.lock paths by returning {} instead of falling through to git
  • Detect missing-file errors from git show and return {} instead of throwing
  • Add tests for both local and git-ref missing lockfile scenarios
  • Document the usage in README

@wikando-ck wikando-ck force-pushed the feat/allow-empty-lockfile-diff branch from 1d7c65b to b43eea6 Compare March 18, 2026 19:32
@wikando-ck wikando-ck changed the title fix: handle missing composer.lock paths as empty base Support diffing against a non-existing composer.lock file Mar 18, 2026
@wikando-ck wikando-ck force-pushed the feat/allow-empty-lockfile-diff branch from b43eea6 to fe68bd4 Compare March 18, 2026 20:15
@IonBazan
Copy link
Copy Markdown
Owner

Thanks for the bug report and your contribution. Could you help to check why the Windows build is failing? This must have something to do with using : as both git separator and disk identifier.
Please ignore Composer v1 failures for now.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (cb9542e) to head (c8e7b3e).

Additional details and impacted files
@@             Coverage Diff             @@
##                main       #46   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity       242       247    +5     
===========================================
  Files             22        22           
  Lines            611       632   +21     
===========================================
+ Hits             611       632   +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread README.md Outdated
@wikando-ck wikando-ck force-pushed the feat/allow-empty-lockfile-diff branch from 7dddef0 to 2d13833 Compare March 20, 2026 10:57
@wikando-ck
Copy link
Copy Markdown
Author

Fixed the windows issue and covered that by tests.
Squashed a fix for the duplicate empty line to fix the styleci into the first commit.

@wikando-ck wikando-ck requested a review from IonBazan March 20, 2026 10:58
@wikando-ck wikando-ck force-pushed the feat/allow-empty-lockfile-diff branch from 1820246 to c8e7b3e Compare April 23, 2026 18:37
Copy link
Copy Markdown
Owner

@IonBazan IonBazan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for following it up and sorry for late review. This looks almost good but needs some clarification (see below).

Comment thread src/PackageDiff.php

/* @infection-ignore-all False-positive */
return '{}'; // Do not throw exception for composer.json as it might not exist and that's fine
if (!$this->looksLikeJsonDocument($outputString)) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will cause decoding the json twice. Also, if the file exists but is not a well-formed JSON, I wouldn't consider it a skippable IO error and throw exception here. Let me know what you think.

Comment thread src/PackageDiff.php
return substr($path, 0, -strlen(self::EXTENSION_LOCK)).self::EXTENSION_JSON;
}

return $path;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somehow there is an escaped mutant here now 🤔

Comment thread src/PackageDiff.php

if ($lockFile) {
$candidates[] = $path;
$candidates[] = $path.self::GIT_SEPARATOR.self::COMPOSER.self::EXTENSION_LOCK;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously there was a GIT_SEPARATOR strpos() check. Is it replaced by trial and error instead to better handle Windows absolute paths?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants