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(manager/composer): use intersects() instead of matches() to compare tool constraint #34256

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

Conversation

ste93cry
Copy link

Changes

Within this pull request, I'm refactoring the getComposerArguments() function to make use of the newly added intersects() function to compare the tool version constraint.

Context

While developing an unrelated feature, I realized that the matches() function that is being used to compare the tool version constraint in the Composer package manager returns unwanted results when the version is lower than the minimum required one, even if it's using the caret. For example, if the minimum required version is ^2.6 and the tool constraint is ^2.7, the expected result should be true, but false is returned instead. The following table shows some examples of what's happening:

Function Tool Version Constraint Minimum Required Tool Version Result
matches ^2.6 ^2.7 false
matches ^2.8 ^2.7 true
matches 2.6.0 ^2.7 false
matches 2.7.0 ^2.7 true
matches 2.8.0 ^2.7 true

By switching to intersects() instead, the intersection of two ranges is handled correctly, as can be seen in the first row of the following table:

Function Tool Version Constraint Minimum Required Tool Version Result
intersects ^2.6 ^2.7 true
intersects ^2.8 ^2.7 true
intersects 2.6.0 ^2.7 false
intersects 2.7.0 ^2.7 true
intersects 2.8.0 ^2.7 true

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

@ste93cry ste93cry changed the title fix(manager/composer): use intersects() in place of matches() to compare tool constraint fix(manager/composer): use intersects() instead of matches() to compare tool constraint Feb 16, 2025
@rarkins rarkins requested a review from viceice February 17, 2025 06:48
rarkins
rarkins previously approved these changes Feb 17, 2025
viceice
viceice previously approved these changes Feb 18, 2025
@viceice viceice enabled auto-merge February 18, 2025 13:03
auto-merge was automatically disabled February 18, 2025 13:56

Head branch was pushed to by a user without write access

@ste93cry ste93cry dismissed stale reviews from viceice and rarkins via 1a71be4 February 18, 2025 13:56
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