Copy lockfiles when no package_filters

[marschattha]

A common issue I have noticed in testing this is that if the version of the linter in the lockfile is specified to be different than in qlty.toml (often the default known_good_version) we get the version mismatch error during version check post tool install.

Maybe we should skip version check in this case?

[Copilot]

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

Comments suppressed due to low confidence (1)

qlty-check/src/tool/php/composer.rs:216

• [nitpick] Consider using consistent log formatting specifiers (e.g. using {} with .display() like in other parts) for lock file paths to improve consistency across the codebase.

debug!(
                        "Copying lock file from {:?} to {:?}",
                        lock_file, staging_lock_file
                    );

[qltysh]

Diff Coverage for cli: The code coverage on the diff in this pull request is 94.9%.

Total Coverage for cli: This PR will increase coverage by 0.08%.

File Coverage Changes

Path	File Coverage Δ	Indirect
qlty-check/src/tool/node/package_json.rs	-1.0
qlty-check/src/tool/php.rs	-0.6
qlty-check/src/tool/php/composer.rs	0.3
qlty-check/src/tool/ruby/gemfile.rs	-0.3
qlty-config/src/library.rs	0.5	√

🛟 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]

2 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with high complexity (count = 20): update_package_json	2

[brynary]

@marschattha on the version check, what do you think of this?:

• IF a version is specified in the qlty.toml we check for it
• ELSE we do not run the version check

[marschattha]

@marschattha on the version check, what do you think of this?:

• IF a version is specified in the qlty.toml we check for it
• ELSE we do not run the version check

Thinking and doing the prototype of that, might not be a good idea as it would stop checking for a lot more cases than it needs to as it is find to do that when the version is latest or known_good, just not ideal when installing using lockfile.
I think the version check provides a good constraint and should prevent bugs.
Maybe limiting it to cases where we copy lockfiles might be enough?

[brynary]

Hmm, I think the issue here is that the version check does two different, important things:

1. Make sure the linter is installed properly
2. Validate the version is expected

We can get the benefit of number 1 by running the version command and simply not validating the output.

So, what if we break down the concept of a version check into a validated version check which checks the output vs. an invalidated version check which just makes sure it exits 0.

Would that help?

[marschattha]

Ah we were already skipping version check for ruby and node packages when using package files, was just missing it in php.

[marschattha]

Hmm, I think the issue here is that the version check does two different, important things:

1. Make sure the linter is installed properly
2. Validate the version is expected

We can get the benefit of number 1 by running the version command and simply not validating the output.

So, what if we break down the concept of a version check into a validated version check which checks the output vs. an invalidated version check which just makes sure it exits 0.

Would that help?

Just saw this, yup that makes sense, we should do that in a separate PR.