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.

Sign up for GitHub

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

Jump to bottom

NewConstraint method faulty behavior #185

Closed
benjaminbear opened this issue Aug 10, 2022 · 6 comments · Fixed by #186
Closed

NewConstraint method faulty behavior #185

benjaminbear opened this issue Aug 10, 2022 · 6 comments · Fixed by #186
Labels
bug

Comments

@benjaminbear
Copy link

benjaminbear commented Aug 10, 2022 •
edited

Used version: 3.1.1
When I try to create a constraint with NewConstraint method I get following returns:

good Example
c, cErr := semver.NewConstraint("12.3.4.1234")
c.String() = nil
cErr = "improper constraint: 12.3.4.1234"

faulty behavior
c, cErr := semver.NewConstraint("12.23.4.1234")
c.String() = "12.23.4 1234"
cErr = nil

or

c, cErr := semver.NewConstraint("12.3.34.1234")
c.String() = "12.3.34 1234"
cErr = nil

I would expect the same behavior as with "12.3.4.1234", means telling me its not a contraint..
@benjaminbear
Copy link
Author

FYI: the used regex to validate the contstraint fails: https://regex101.com/r/3fEGeK/1

@mattfarina
Copy link
Member

I can explain what's going on here. 12.3.4.1234 is invalid because it has 4 parts and semver is only 3 parts.

12.3.34 1234 is read as 2 separate constraints. 12.3.34 and 1234. Those are added together. See https://github.com/Masterminds/semver#basic-comparisons

@benjaminbear
Copy link
Author

benjaminbear commented Aug 10, 2022 •
edited

Ah I'm sorry, had to correct the 2 faulty examples, there were copy paste typos.
Is this:
c, cErr := semver.NewConstraint("12.23.4.1234")
c.String() = "12.23.4 1234"
cErr = nil

expected behavior? Its a non semver string with 4 parts like in the first example, but handled differently. The third dot is removed and its a valid range?
I would expect an error here.

Wy is
12.3.4.1234 and
12.23.4.1234
handled differently? the only difference is that the second part is 2 digits

@mattfarina mattfarina added bug and removed question labels Aug 15, 2022
@mattfarina
Copy link
Member

I have verified the issue. https://go.dev/play/p/wMEsKXb5KEx

mattfarina added a commit to mattfarina/semver that referenced this issue Aug 15, 2022
@mattfarina
Fixing issue where validation of constraint setion gave false positives
ee95afb 
Closes Masterminds#185

Validation of strings was producing false positives when there were too
many sections. A string like 1.2.3.1234 would be found in error but on
like 1.23.3.1234 or 1.2.34.1234 could pass even though it had too many
segments. This is because a string like 1.23.3.1234 would be found as
two different constraints of 1.1 and 3.3.1234.

The fix was to treat validation for the the first and following
constraints separately. For for the following ones a space or command
is now required before it to show it's a second or further one.

Signed-off-by: Matt Farina <matt@mattfarina.com>
@mattfarina mattfarina mentioned this issue Aug 15, 2022
Merged
@mattfarina
Copy link
Member

@benjaminbear The fix can be found in #186 and the commit message explains what happened to cause this.

I am planning on getting a new release out by the end of this week with some fixes and new functionality.

Feel free to double check my fix and tell me if it didn't work.

@benjaminbear
Copy link
Author

Great to hear! Thank you very much @mattfarina

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixing issue where validation of constraint section gave false positives mattfarina/semver
2 participants
@mattfarina @benjaminbear