fix(core): fix semver handling #7197
Conversation
|
WalkthroughFixes a logic bug in NodeJsPackage::analyze by correcting the deprecated-license conditional. Removes the node_semver parser wrapper and its public convenience re-exports. Refactors Range into a public enum (Comparators / Alternatives), introduces ComparatorOperator and Display impls, replaces Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (7)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
🧰 Additional context used📓 Path-based instructions (2)**/*.{rs,toml}📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
crates/biome_*/**/*📄 CodeRabbit Inference Engine (CLAUDE.md)
Files:
🧬 Code Graph Analysis (1)crates/biome_package/src/node_semver/range.rs (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
🔇 Additional comments (12)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
crates/biome_package/src/node_semver/version.rs (1)
214-226: Overflow handling ininc_minorcould be clearer.The overflow cascade from minor to major is a sensible design choice, but it might surprise users. Consider documenting this behaviour more prominently or providing a non-cascading alternative.
crates/biome_package/src/node_semver/range.rs (1)
306-408: Complex parsing logic could benefit from more documentation.The
parse_setmethod handles multiple cases with nested pattern matching. Consider adding inline comments to explain the wildcard handling logic, especially for theExactWithWildcardscase.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
crates/biome_package/src/node_js_package/mod.rs(1 hunks)crates/biome_package/src/node_semver/mod.rs(2 hunks)crates/biome_package/src/node_semver/parser.rs(0 hunks)crates/biome_package/src/node_semver/range.rs(11 hunks)crates/biome_package/src/node_semver/range.tests.rs(1 hunks)crates/biome_package/src/node_semver/version.rs(13 hunks)
💤 Files with no reviewable changes (1)
- crates/biome_package/src/node_semver/parser.rs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Format code (Rust + TOML) using
just formatFormat Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_package/src/node_semver/range.tests.rscrates/biome_package/src/node_js_package/mod.rscrates/biome_package/src/node_semver/version.rscrates/biome_package/src/node_semver/mod.rscrates/biome_package/src/node_semver/range.rs
crates/biome_*/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Core crates must be located in
/crates/biome_*/
Files:
crates/biome_package/src/node_semver/range.tests.rscrates/biome_package/src/node_js_package/mod.rscrates/biome_package/src/node_semver/version.rscrates/biome_package/src/node_semver/mod.rscrates/biome_package/src/node_semver/range.rs
🧠 Learnings (7)
📚 Learning: 2025-08-11T11:48:27.756Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.756Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:48:27.756Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.756Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:53:15.289Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.289Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:53:15.289Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.289Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:48:27.756Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.756Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/language.rs : Create tests/language.rs defining HtmlTestFormatLanguage and implement TestFormatLanguage for it
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:43:46.589Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:43:46.589Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Quick test lives in biome_js_analyze/tests/quick_test.rs; unignore it and set RuleFilter appropriately when using
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:51:26.988Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_parser/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:51:26.988Z
Learning: Implement a parser struct that implements the Parser trait and holds context, token source, and optional options
Applied to files:
crates/biome_package/src/node_semver/mod.rs
🧬 Code Graph Analysis (2)
crates/biome_package/src/node_semver/range.tests.rs (2)
crates/biome_package/src/node_semver/range.rs (5)
includes(234-245)intersects(264-283)intersects(484-537)input(550-554)from_str(543-559)crates/biome_package/src/node_semver/version.rs (17)
parse(414-414)parse(424-424)parse(434-434)parse(444-444)parse(454-454)parse(464-464)parse(474-474)parse(475-475)parse(476-476)parse(484-484)parse(485-485)parse(486-486)parse(487-487)parse(496-496)parse(497-497)parse(498-498)from_str(285-328)
crates/biome_package/src/node_semver/range.rs (2)
crates/biome_package/src/node_semver/version.rs (6)
fmt(54-61)fmt(332-344)new(83-91)inc_major(214-216)inc_minor(221-227)inc_patch(232-238)crates/biome_package/benches/semver.rs (1)
version(61-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Documentation
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: autofix
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Check Dependencies
🔇 Additional comments (9)
crates/biome_package/src/node_js_package/mod.rs (1)
75-75: Nice fix!The logic correctly checks for deprecated licenses now. Previously, the condition was inverted, which would have incorrectly skipped the deprecation diagnostic.
crates/biome_package/src/node_semver/version.rs (3)
301-313: LGTM! Sensible handling of missing components.The logic correctly treats missing minor and patch components as 0, which aligns with common semver parsing behaviour.
391-391: Nice optimisation for prerelease comparison.Using
u16for numeric prerelease parts aligns with the version component types and should handle most practical cases.
29-37: Reevaluate use of u16 for version componentsOur grep didn’t find any
Version::newcalls with literals ≥ 65 536, but cutting fromu64down tou16is still a breaking change. The SemVer spec places no hard cap on major/minor/patch values, so consumers could hit the 65 535 ceiling.– Document the 0–65 535 limit on
major,minorandpatch.
– Add tests or validation to catch any overflow or out-of-range inputs.
– If you anticipate larger version numbers, revert back tou64(or choose a wider type).crates/biome_package/src/node_semver/range.tests.rs (1)
1-383: Excellent test coverage!The test suite is comprehensive and well-structured. It covers edge cases, includes tests from node-semver for compatibility, and uses clear assertion messages. The macro for range parsing tests is particularly elegant.
crates/biome_package/src/node_semver/mod.rs (1)
1-64: Clean API simplification!The removal of the parser module wrapper and the shift to
includes()makes the API more intuitive and aligns better with Rust conventions.crates/biome_package/src/node_semver/range.rs (3)
29-39: Good enum design for Range representation.The enum-based approach with
ComparatorsandAlternativesvariants cleanly models the OR semantics of version ranges.
569-613: Clever deduplication logic inparse_comparator_set.The logic for merging comparators with the same operator (lines 590-606) is well-thought-out, ensuring the most restrictive bounds are kept.
771-777: Efficient dot counting implementation.Good use of byte-level operations for performance. The early termination on
-and+correctly handles prerelease and build metadata.
| pub fn dec_minor(&self) -> Self { | ||
| if let Some(decremented_minor) = self.minor.checked_sub(1) { | ||
| Self::new(self.major, decremented_minor, 0) | ||
| } else { | ||
| self.dec_major() | ||
| } | ||
| } |
There was a problem hiding this comment.
Questionable semantics for dec_minor.
Decrementing the minor version when underflow occurs seems counterintuitive. When minor is 0, decrementing to the previous major version with minor = 0 might not represent a meaningful version transition.
Consider either:
- Saturating at 0 (like
dec_majordoes) - Returning an error/Option to indicate underflow
- Documenting this behaviour very clearly as it's unexpected
pub fn dec_minor(&self) -> Self {
if let Some(decremented_minor) = self.minor.checked_sub(1) {
Self::new(self.major, decremented_minor, 0)
} else {
- self.dec_major()
+ // Saturate at current version when minor is already 0
+ self.clone()
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn dec_minor(&self) -> Self { | |
| if let Some(decremented_minor) = self.minor.checked_sub(1) { | |
| Self::new(self.major, decremented_minor, 0) | |
| } else { | |
| self.dec_major() | |
| } | |
| } | |
| pub fn dec_minor(&self) -> Self { | |
| if let Some(decremented_minor) = self.minor.checked_sub(1) { | |
| Self::new(self.major, decremented_minor, 0) | |
| } else { | |
| // Saturate at current version when minor is already 0 | |
| self.clone() | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/biome_package/src/node_semver/version.rs around lines 250–256,
dec_minor currently wraps underflow into dec_major which is confusing; change
semantics to saturate at 0 instead: when minor > 0 return a version with minor-1
and patch 0, otherwise return the same major with minor = 0 and patch = 0 (i.e.,
do not decrement major). Update the implementation and add a brief doc comment
on dec_minor stating it saturates at 0 rather than rolling back the major.
| pub fn dec_patch(&self) -> Self { | ||
| if let Some(decremented_patch) = self.patch.checked_sub(1) { | ||
| Self::new(self.major, self.minor, decremented_patch) | ||
| } else { | ||
| self.dec_minor() | ||
| } | ||
| } |
There was a problem hiding this comment.
Same concern for dec_patch underflow behaviour.
The cascade to dec_minor when patch underflows is unexpected and could lead to surprising version transitions.
pub fn dec_patch(&self) -> Self {
if let Some(decremented_patch) = self.patch.checked_sub(1) {
Self::new(self.major, self.minor, decremented_patch)
} else {
- self.dec_minor()
+ // Saturate at current version when patch is already 0
+ self.clone()
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn dec_patch(&self) -> Self { | |
| if let Some(decremented_patch) = self.patch.checked_sub(1) { | |
| Self::new(self.major, self.minor, decremented_patch) | |
| } else { | |
| self.dec_minor() | |
| } | |
| } | |
| pub fn dec_patch(&self) -> Self { | |
| if let Some(decremented_patch) = self.patch.checked_sub(1) { | |
| Self::new(self.major, self.minor, decremented_patch) | |
| } else { | |
| // Saturate at current version when patch is already 0 | |
| self.clone() | |
| } | |
| } |
🤖 Prompt for AI Agents
In crates/biome_package/src/node_semver/version.rs around lines 261 to 267,
dec_patch currently cascades to dec_minor on underflow which is surprising;
change the behavior so that when patch is 0 the method does not decrement minor
but instead returns the current version (no-op) or otherwise explicitly handle
underflow (e.g., return a Result/Option) to make the behavior predictable;
update the implementation to check for patch == 0 and return self (or convert
the signature to return Result/Option and return an error/None), and adjust/add
unit tests and documentation to reflect the new, non-cascading semantics.
CodSpeed Performance ReportMerging #7197 will improve performances by ×2.7Comparing Summary
Benchmarks breakdown
|
|
You should acknowledge the benchmarks before merging |
|
Fun fact about the benchmarks: I changed some of the But the benchmarks here certainly proved me wrong: |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
crates/biome_package/src/node_semver/version.rs (2)
250-256: Questionable semantics fordec_minorpersist.Decrementing the minor version when underflow occurs seems counterintuitive. When
minoris 0, cascading to the previous major version might not represent a meaningful version transition.
261-267: Same concern fordec_patchunderflow behaviour.The cascade to
dec_minorwhen patch underflows is unexpected and could lead to surprising version transitions.
🧹 Nitpick comments (2)
crates/biome_package/src/node_semver/version.rs (1)
29-33: Consider documenting the reduced numeric range for version components.The fields have been reduced from
u64tou16, limiting version numbers to 0-65535. While this should be sufficient for most use cases, it's worth documenting this constraint in the struct's documentation to avoid surprises./// The major version number. Incremented for incompatible API changes. +/// Valid range: 0-65535. pub major: u16, /// The minor version number. Incremented for backwards-compatible functionality additions. +/// Valid range: 0-65535. pub minor: u16, /// The patch version number. Incremented for backwards-compatible bug fixes. +/// Valid range: 0-65535. pub patch: u16,crates/biome_package/src/node_semver/range.tests.rs (1)
379-384: Consider documenting why these test cases are unsupported.The comment indicates these were supported by
node_semverbut not by Biome. It would be helpful to document the reasoning behind not supporting these patterns, especially if users might expect compatibility.// FIXME: The following test cases were supported by node_semver, but are // not supported by Biome. They look invalid anyway, but we may want - // to reconsider if bugs are reported for these. + // to reconsider if bugs are reported for these. + // These patterns involve leading zeros in version numbers which violate + // strict semver specification, and the "beta" suffix without separation + // is non-standard syntax. // loose1 => [">01.02.03", ">1.2.3"], // loose2 => ["~1.2.3beta", ">=1.2.3-beta <1.3.0-0"],
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
crates/biome_package/benches/semver.rs(1 hunks)crates/biome_package/src/node_js_package/mod.rs(1 hunks)crates/biome_package/src/node_semver/mod.rs(3 hunks)crates/biome_package/src/node_semver/parser.rs(0 hunks)crates/biome_package/src/node_semver/range.rs(9 hunks)crates/biome_package/src/node_semver/range.tests.rs(1 hunks)crates/biome_package/src/node_semver/version.rs(11 hunks)
💤 Files with no reviewable changes (1)
- crates/biome_package/src/node_semver/parser.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/biome_package/src/node_js_package/mod.rs
- crates/biome_package/benches/semver.rs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{rs,toml}
📄 CodeRabbit Inference Engine (CLAUDE.md)
Format code (Rust + TOML) using
just formatFormat Rust and TOML files before committing (use
just f/just format).
Files:
crates/biome_package/src/node_semver/range.tests.rscrates/biome_package/src/node_semver/mod.rscrates/biome_package/src/node_semver/range.rscrates/biome_package/src/node_semver/version.rs
crates/biome_*/**/*
📄 CodeRabbit Inference Engine (CLAUDE.md)
Core crates must be located in
/crates/biome_*/
Files:
crates/biome_package/src/node_semver/range.tests.rscrates/biome_package/src/node_semver/mod.rscrates/biome_package/src/node_semver/range.rscrates/biome_package/src/node_semver/version.rs
🧠 Learnings (6)
📚 Learning: 2025-08-11T11:48:27.756Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.756Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_test.rs : Create tests/spec_test.rs implementing the run(spec_input_file, _expected_file, test_directory, _file_type) function as shown and include!("language.rs")
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:48:27.756Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.756Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/spec_tests.rs : Create tests/spec_tests.rs in the biome_html_formatter crate that generates tests via tests_macros::gen_tests! for all HTML files at tests/specs/html/**/*.html
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:53:15.289Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.289Z
Learning: Applies to crates/biome_service/../biome_lsp/src/server.tests.rs : Keep end-to-end LSP tests in biome_lsp’s server.tests.rs
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:53:15.289Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_service/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:53:15.289Z
Learning: Applies to crates/biome_service/src/workspace/watcher.tests.rs : Place watcher tests for workspace methods in src/workspace/watcher.tests.rs
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:43:46.589Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_analyze/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:43:46.589Z
Learning: Applies to crates/biome_analyze/crates/biome_js_analyze/tests/quick_test.rs : Quick test lives in biome_js_analyze/tests/quick_test.rs; unignore it and set RuleFilter appropriately when using
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
📚 Learning: 2025-08-11T11:48:27.756Z
Learnt from: CR
PR: biomejs/biome#0
File: crates/biome_formatter/CONTRIBUTING.md:0-0
Timestamp: 2025-08-11T11:48:27.756Z
Learning: Applies to crates/biome_formatter/biome_html_formatter/tests/language.rs : Create tests/language.rs defining HtmlTestFormatLanguage and implement TestFormatLanguage for it
Applied to files:
crates/biome_package/src/node_semver/range.tests.rs
🧬 Code Graph Analysis (2)
crates/biome_package/src/node_semver/range.tests.rs (2)
crates/biome_package/src/node_semver/range.rs (5)
includes(223-234)intersects(253-272)intersects(364-417)input(430-433)from_str(423-442)crates/biome_package/src/node_semver/version.rs (17)
parse(414-414)parse(424-424)parse(434-434)parse(444-444)parse(454-454)parse(464-464)parse(474-474)parse(475-475)parse(476-476)parse(484-484)parse(485-485)parse(486-486)parse(487-487)parse(496-496)parse(497-497)parse(498-498)from_str(285-328)
crates/biome_package/src/node_semver/range.rs (1)
crates/biome_package/src/node_semver/version.rs (6)
fmt(54-61)fmt(332-344)new(83-91)inc_major(214-216)inc_minor(221-227)inc_patch(232-238)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (23)
- GitHub Check: Lint project (depot-windows-2022)
- GitHub Check: Bench (biome_module_graph)
- GitHub Check: Bench (biome_json_analyze)
- GitHub Check: Bench (biome_html_formatter)
- GitHub Check: Test (depot-ubuntu-24.04-arm-16)
- GitHub Check: Bench (biome_js_analyze)
- GitHub Check: Documentation
- GitHub Check: Bench (biome_graphql_parser)
- GitHub Check: Bench (biome_package)
- GitHub Check: Bench (biome_configuration)
- GitHub Check: Lint project (depot-ubuntu-24.04-arm-16)
- GitHub Check: Check Dependencies
- GitHub Check: Bench (biome_graphql_formatter)
- GitHub Check: Bench (biome_json_parser)
- GitHub Check: Bench (biome_json_formatter)
- GitHub Check: Bench (biome_css_formatter)
- GitHub Check: Bench (biome_html_parser)
- GitHub Check: Test (depot-windows-2022-16)
- GitHub Check: Bench (biome_js_formatter)
- GitHub Check: Bench (biome_css_analyze)
- GitHub Check: Bench (biome_js_parser)
- GitHub Check: Bench (biome_css_parser)
- GitHub Check: autofix
🔇 Additional comments (12)
crates/biome_package/src/node_semver/version.rs (2)
124-131: LGTM! Clean addition of the minimum prerelease helper.The
with_minimum_prereleasemethod is a useful addition that clearly communicates its purpose. The implementation is straightforward and well-documented.
300-313: Excellent relaxation of version parsing constraints.The changes to allow missing minor and patch components (defaulting to 0) align well with common semver usage patterns and improve compatibility with real-world version strings.
crates/biome_package/src/node_semver/range.tests.rs (3)
1-9: Well-structured test for exact range matching.Good coverage of the basic exact match scenario with clear assertions.
34-51: Excellent coverage of caret range edge cases.The tests thoroughly cover the special behaviour of caret ranges when major or minor versions are zero, which is crucial for correct semver handling.
287-300: Clever use of macros for range parsing tests.The macro approach significantly reduces boilerplate and makes the test cases more readable. The structure clearly shows input-output pairs which aids in understanding the expected behaviour.
crates/biome_package/src/node_semver/mod.rs (2)
63-64: Clean public API simplification.The removal of the parser module and consolidation of exports through the range and version modules makes the API surface much cleaner and easier to understand.
19-40: Examples are clear and demonstrate the new API well.The updated documentation examples effectively showcase the
includesmethod and theFromStrtrait usage, making the API changes immediately apparent to users.crates/biome_package/src/node_semver/range.rs (5)
29-43: Excellent refactor to enum-based Range representation.The change from a struct to an enum with
ComparatorsandAlternativesvariants makes the OR logic explicit and easier to reason about. This is a much cleaner design than implicitly handling alternatives.
365-416: Comprehensive intersection logic between comparators.The exhaustive handling of all operator combinations is thorough. The logic correctly handles the nuances of version comparisons, including the use of
inc_patch()for boundary calculations.
383-384: Check the intersection logic forGreaterThanwithLessThan.The condition uses
self.version.inc_patch() < other.version. This assumes there's always a version betweenself.versionandother.version, which might not be true for consecutive patch versions.Consider whether this should be
self.version < other.versionto properly check if the ranges>Xand<Yintersect (they do when X < Y).
594-610: Clever deduplication logic inpush_unique_comparator.The function elegantly handles merging duplicate operators by taking the most restrictive bound, which is exactly what's needed for AND operations within a comparator set.
721-758: Well-designed wildcard handling with clear enum variants.The
ComponentOrWildcardResultenum and its usage insplit_component_or_wildcardprovide excellent clarity in handling wildcards versus numeric components. The recursive pattern matching approach is clean and maintainable.
| pub patch: u64, | ||
| pub patch: u16, | ||
| /// Pre-release version identifiers (e.g., ("alpha", "1") for "1.2.3-alpha.1"). | ||
| pub prerelease: Vec<String>, |
There was a problem hiding this comment.
Because prerelease and build are rarely used, I wonder if this could be beneficial of adding a single field prerelease_build: Box<str>. Prerelease and build are already semi-parsed data. This could make them unparsed.
Summary
Fixes most regressions in semver handling from #7061. There were regressions in at least the following areas:
~) and caret ranges (^) and probably others.Additionally I removed a bunch of helpers that were nothing but dead code. I also optimised the
VersionandRangerepresentations slightly.Test Plan
Several tests from
node_semverhave been added. Tests forVersionhave been relaxed to allow parsing versions without minor and/or patch.