Skip to content

fix(check): honor composer.lock when installing from package_file - #2830

Merged
marschattha merged 3 commits into
mainfrom
ma/fix-composer-lock-honored
Jul 23, 2026
Merged

fix(check): honor composer.lock when installing from package_file#2830
marschattha merged 3 commits into
mainfrom
ma/fix-composer-lock-honored

Conversation

@marschattha

@marschattha marschattha commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

PHP counterpart to #2829: composer.lock was not honored when installing PHP linters with package_file, contradicting the documented behavior ("When using package_file, Qlty respects the locked versions for reliability").

Problem

Since #1658, update_composer_json copies the repo's composer.lock into the tool staging directory (when no package_filters are configured), but the package file was then installed with composer update --no-interaction --ignore-platform-reqs — and composer update re-resolves dependencies and rewrites the lock, ignoring the locked versions entirely.

Repro: a repo with "squizlabs/php_codesniffer": "^3.8" in composer.json, a lockfile pinning 3.8.0, and package_file = "composer.json" in qlty.toml installed phpcs 3.13.5 instead of the locked 3.8.0.

Fix

When the repository's composer.lock was staged, attempt composer install (which installs exactly what the lock records). If it fails — a stale lock, a missing package, a constraint mismatch, anything composer's lock validation rejects — fall back to composer update with a warning, restoring the previous behavior at the cost of one fast-failing composer run.

One wrinkle: qlty collapses require-dev into require in the staged composer.json, but composer install validates required prod packages against the lock's prod section — so a tool locked under packages-dev (the common case for linters) would hard-error with "Required package ... is not present in the lock file". The staged lock therefore gets the same treatment: packages-dev entries are collapsed into packages.

Testing

  • New unit tests: lock staged → composer install; install failure → falls back to composer update; package_filters set → lock not staged, composer update only; dev packages collapsed in the staged lock
  • Verified end-to-end: locked phpcs 3.8.0 installs (including when locked under require-dev); a stale lock (constraint ^3.9 vs locked 3.8.0) falls back to update and succeeds; without a lockfile, behavior is unchanged
  • phpstan, php-codesniffer, and php-cs-fixer plugin test suites pass locally
  • Full qlty-check test suite passes (249 tests)

🤖 Generated with Claude Code

The lockfile copied into the tool staging directory was being ignored
because the package file was installed with composer update, which
re-resolves dependencies and rewrites the lock. Linters were installed
at the latest versions matching the composer.json constraints instead
of the locked versions, contradicting the documented behavior.

Run composer install when the repository's composer.lock was staged and
records the tool package, falling back to composer update otherwise
since install would not add a package missing from the lock. Because
the staged composer.json has require-dev collapsed into require, the
staged lock file needs the same treatment: packages-dev entries are
collapsed into packages so composer install accepts required packages
locked as dev dependencies.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 23, 2026 16:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes PHP package_file installs in qlty-check so that a staged composer.lock is actually honored, aligning runtime behavior with the documented “lock files are respected for reliability” guarantee.

Changes:

  • Switches Composer package-file installs from unconditional composer update to composer install when a staged lockfile exists and contains the tool package, falling back to update otherwise.
  • When staging composer.lock, collapses packages-dev entries into packages to match the already-staged composer.json behavior (where require-dev is collapsed into require).
  • Adds unit tests covering install-vs-update selection and dev-package collapsing in the staged lockfile.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
qlty-check/src/tool/php/composer.rs Stages lockfile with dev-package collapsing and chooses install vs update based on staged lockfile contents.
qlty-check/src/tool/php.rs Minor formatting tweaks plus new tests asserting install is used when an appropriate lockfile is staged.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread qlty-check/src/tool/php/composer.rs Outdated
@qltysh

qltysh Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Qlty


Coverage Impact - ubuntu-latest

⬆️ Merging this pull request will increase total coverage on main by 0.05%.

Modified Files with Diff Coverage (2)

RatingFile% DiffUncovered Line #s
Coverage rating: D Coverage rating: C
qlty-check/src/tool/php.rs95.4%81-168, 359, 406, 465
Coverage rating: A Coverage rating: A
qlty-check/src/tool/php/composer.rs93.6%102, 157-161
Total94.8%
🤖 Increase coverage with AI coding...
In the `ma/fix-composer-lock-honored` branch, add test coverage for this new code:

- `qlty-check/src/tool/php.rs` -- Lines 81-168, 359, 406, and 465
- `qlty-check/src/tool/php/composer.rs` -- Lines 102 and 157-161

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@qltysh

qltysh Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Qlty


Coverage Impact - macos-15

⬆️ Merging this pull request will increase total coverage on main by 0.06%.

Modified Files with Diff Coverage (2)

RatingFile% DiffUncovered Line #s
Coverage rating: D Coverage rating: C
qlty-check/src/tool/php.rs95.4%81-168, 359, 406, 465
Coverage rating: A Coverage rating: A
qlty-check/src/tool/php/composer.rs93.6%102, 157-161
Total94.8%
🤖 Increase coverage with AI coding...
In the `ma/fix-composer-lock-honored` branch, add test coverage for this new code:

- `qlty-check/src/tool/php.rs` -- Lines 81-168, 359, 406, and 465
- `qlty-check/src/tool/php/composer.rs` -- Lines 102 and 157-161

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Checking only for the tool package in the lock file was insufficient:
composer install also hard-errors when any required package is missing
from the lock file or locked at a version outside its composer.json
constraint, which a stale repository lock file would trigger. Validating
all of that up front would mean reimplementing composer's constraint
resolution.

Instead, attempt composer install whenever the repository's lock file
was staged and fall back to composer update on any failure, restoring
the previous behavior at the cost of one fast-failing composer run.
This also makes the tool package presence check redundant, simplifying
the logic.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 23, 2026 17:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread qlty-check/src/tool/php/composer.rs
Comment thread qlty-check/src/tool/php/composer.rs
Include the composer subcommand and exit code in the failure message so
the install-to-update fallback warning is actionable, and reword the
lock staging debug log since the lock file is rewritten rather than
copied verbatim.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 23, 2026 18:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@marschattha
marschattha marked this pull request as ready for review July 23, 2026 18:47
@marschattha
marschattha requested a review from noahd1 July 23, 2026 18:47

@noahd1 noahd1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I find the collapsing of dev and prod packages a bit confusing but seems that this is pre-existing. I realized in looking through this too that the use case for "filtered" package files is much reduced with recent support for private packages. I think the only remaining use case is basically a performance optimization use case.

@marschattha
marschattha merged commit 1d38007 into main Jul 23, 2026
23 checks passed
@marschattha
marschattha deleted the ma/fix-composer-lock-honored branch July 23, 2026 19:48
@qlty-releases qlty-releases Bot mentioned this pull request Jul 23, 2026
marschattha pushed a commit that referenced this pull request Jul 23, 2026
Automated PR for release 0.639.0.

Review (and edit, if needed) the changelog entry below, then
merge this pull request to publish the release.

## Draft release notes


### Fixed

- Honor `package-lock.json` when installing node-based linters from
`package_file`, so the locked versions are respected instead of npm
resolving the latest versions matching the `package.json` semver ranges
(#2829)
- Honor `composer.lock` when installing PHP linters from `package_file`,
so the locked versions are respected instead of `composer update`
re-resolving to the latest matching versions (#2830)

Co-authored-by: qlty-releases[bot] <181762136+qlty-releases[bot]@users.noreply.github.com>
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.

3 participants