Skip to content

Comments

Update semver regex to not swallow trailing .#129

Merged
peterhuene merged 1 commit intobytecodealliance:mainfrom
itowlson:fix-semver-parse-glitch
Jun 25, 2024
Merged

Update semver regex to not swallow trailing .#129
peterhuene merged 1 commit intobytecodealliance:mainfrom
itowlson:fix-semver-parse-glitch

Conversation

@itowlson
Copy link
Contributor

Fixes #128.

The new regex is more complicated than wac likely needs - for example, it includes capture groups that the lexer discards. The reason for choosing it was that it is suggested by the official semver site (https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string). Of course it can be trimmed back if necessary but that introduces the possibility of me messing it up... grin

@itowlson
Copy link
Contributor Author

The tests are red because some test cases use a version of 1. Should that be allowed, or should I tweak the tests to use e.g. 1.0.0?

@rylev rylev requested a review from peterhuene June 24, 2024 09:24
@calvinrp
Copy link
Collaborator

The tests are red because some test cases use a version of 1. Should that be allowed, or should I tweak the tests to use e.g. 1.0.0?

I think we should fix the test cases to use a valid semver , like 1.0.0.

@itowlson
Copy link
Contributor Author

Ah, correction. Those tests are meant to be invalid semvers. The tests are for the error message, and it looks like the new regex has changed that from

  × `1` is not a valid semantic version
   ╭─[tests/parser/fail/invalid-path-semver.wac:3:23]
 2 │ 
 3 │ import i: foo:bar/baz@1;
   ·                       ┬
   ·                       ╰── invalid version
   ╰────

to

  × unexpected token was encountered
   ╭─[tests/parser/fail/invalid-path-semver.wac:3:11]
 2 │ 
 3 │ import i: foo:bar/baz@1;
   ·           ─────────────
   ╰────

which is definitely a step backward! So a custom regex might be needed after all.

@peterhuene
Copy link
Member

peterhuene commented Jun 25, 2024

Looking back, I think I left it the more generic regex and then did validation via the semver crate on what was matched for this reason, i.e. the fact that if it fails to match, we'll treat it as an unknown token.

Perhaps we simply just slap a [^\.] to the end of the existing expression?

@itowlson itowlson force-pushed the fix-semver-parse-glitch branch from 850d3db to 8491be7 Compare June 25, 2024 21:12
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
@itowlson itowlson force-pushed the fix-semver-parse-glitch branch from 8491be7 to 2ea2465 Compare June 25, 2024 21:29
@itowlson
Copy link
Contributor Author

@peterhuene I think I have something that works - forgiving enough to allow invalid things in version positions so that better errors can be given later, but strict enough to consume dots only when within the regex. I've added a test case for the issue and everything passes locally so 🤞

Copy link
Member

@peterhuene peterhuene left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for fixing this!

@peterhuene peterhuene merged commit 290c100 into bytecodealliance:main Jun 25, 2024
mergify bot referenced this pull request in andrzejressel/pulumi-gestalt Jul 9, 2024
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [wac-graph](https://togithub.com/bytecodealliance/wac) | workspace.dependencies | minor | `0.3.0` -> `0.4.0` |

---

### Release Notes

<details>
<summary>bytecodealliance/wac (wac-graph)</summary>

### [`v0.4.0`](https://togithub.com/bytecodealliance/wac/releases/tag/v0.4.0)

[Compare Source](https://togithub.com/bytecodealliance/wac/compare/v0.3.0...v0.4.0)

#### What's Changed

-   docs: more generic command listing prose by [@&#8203;vados-cosmonic](https://togithub.com/vados-cosmonic) in [https://github.com/bytecodealliance/wac/pull/125](https://togithub.com/bytecodealliance/wac/pull/125)
-   Bump h2 from 0.4.3 to 0.4.5 by [@&#8203;dependabot](https://togithub.com/dependabot) in [https://github.com/bytecodealliance/wac/pull/126](https://togithub.com/bytecodealliance/wac/pull/126)
-   Update semver regex to not swallow trailing `.` by [@&#8203;itowlson](https://togithub.com/itowlson) in [https://github.com/bytecodealliance/wac/pull/129](https://togithub.com/bytecodealliance/wac/pull/129)
-   Implicit import check during target validation by [@&#8203;rylev](https://togithub.com/rylev) in [https://github.com/bytecodealliance/wac/pull/130](https://togithub.com/bytecodealliance/wac/pull/130)

#### New Contributors

-   [@&#8203;vados-cosmonic](https://togithub.com/vados-cosmonic) made their first contribution in [https://github.com/bytecodealliance/wac/pull/125](https://togithub.com/bytecodealliance/wac/pull/125)
-   [@&#8203;dependabot](https://togithub.com/dependabot) made their first contribution in [https://github.com/bytecodealliance/wac/pull/126](https://togithub.com/bytecodealliance/wac/pull/126)
-   [@&#8203;itowlson](https://togithub.com/itowlson) made their first contribution in [https://github.com/bytecodealliance/wac/pull/129](https://togithub.com/bytecodealliance/wac/pull/129)

**Full Changelog**: bytecodealliance/wac@v0.3.0...v0.4.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/andrzejressel/pulumi-wasm).
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.

Syntax supported in WIT but not in WAC

3 participants