Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle parsing versions in composer.lock files that are numbers rather than strings #1139

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

G-Rath
Copy link
Collaborator

@G-Rath G-Rath commented Jul 24, 2024

It seems it's technically possible for composer.lock files to have a number for version in packages - I can't actually reproduce this with composer itself, but if I manually edit a lockfile to have a number instead of a string it doesn't complain or change the value.

It would be good to understand more about how this could happen in the wild, but it's easy enough to support either way.

Resolves #1138

@codecov-commenter
Copy link

codecov-commenter commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.77%. Comparing base (9fcf53f) to head (0dbc586).

Files with missing lines Patch % Lines
pkg/lockfile/parse-composer-lock.go 84.21% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1139      +/-   ##
==========================================
+ Coverage   68.75%   68.77%   +0.01%     
==========================================
  Files         184      184              
  Lines       17714    17733      +19     
==========================================
+ Hits        12180    12196      +16     
- Misses       4875     4877       +2     
- Partials      659      660       +1     

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

@another-rex
Copy link
Collaborator

I think we should try to figure out why this is generated first before merging this fix. I'm not sure if we can confidently support this before figuring out the root cause. (e.g. will the number ever have a decimal point in it?)

@cuixq
Copy link
Contributor

cuixq commented Aug 5, 2024

It seems composer does not complain about strings and valid numbers (for example "version":20190220 or "version":1.2) but complains about invalid numbers (for example "version":1.2.3).

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.

Composer version not always a string
4 participants