Add public Goccia.semver API namespace#192
Merged
Merged
Conversation
- Expose SemVer, Comparator, and Range constructors on `Goccia.semver` - Wire semver helper functions through the global `Goccia` object - Add docs and end-to-end coverage for the new public surface
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 16 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request introduces a full SemVer 2.0.0 API to GocciaScript through a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Suite Timing
Measured on ubuntu-latest x64. |
Contributor
Benchmark Results274 benchmarks Interpreted: 🟢 66 improved · 🔴 94 regressed · 114 unchanged · avg -0.8% arraybuffer.js — Interp: 🟢 10, 4 unch. · avg +1.3% · Bytecode: 🔴 14 · avg -9.5%
arrays.js — Interp: 🟢 9, 🔴 2, 8 unch. · avg +0.6% · Bytecode: 🔴 19 · avg -9.5%
async-await.js — Interp: 🟢 6 · avg +2.6% · Bytecode: 🔴 6 · avg -9.7%
classes.js — Interp: 🟢 10, 🔴 4, 17 unch. · avg +0.2% · Bytecode: 🔴 19, 12 unch. · avg -7.4%
closures.js — Interp: 🟢 4, 🔴 2, 5 unch. · avg +0.3% · Bytecode: 🔴 11 · avg -10.1%
collections.js — Interp: 🟢 2, 🔴 10 · avg -6.5% · Bytecode: 🔴 12 · avg -13.4%
destructuring.js — Interp: 🟢 2, 🔴 10, 10 unch. · avg -1.3% · Bytecode: 🔴 22 · avg -7.9%
fibonacci.js — Interp: 🔴 6, 2 unch. · avg -2.4% · Bytecode: 🔴 8 · avg -10.4%
for-of.js — Interp: 🔴 6, 1 unch. · avg -4.1% · Bytecode: 🔴 7 · avg -6.3%
helpers/bench-module.js — Interp: 0 · Bytecode: 0
iterators.js — Interp: 🟢 3, 🔴 7, 10 unch. · avg -0.5% · Bytecode: 🔴 20 · avg -9.6%
json.js — Interp: 🟢 1, 🔴 10, 9 unch. · avg -1.8% · Bytecode: 🔴 20 · avg -8.2%
jsx.jsx — Interp: 🟢 3, 🔴 13, 5 unch. · avg -1.3% · Bytecode: 🔴 21 · avg -5.0%
modules.js — Interp: 🟢 2, 🔴 2, 5 unch. · avg -0.7% · Bytecode: 🔴 8, 1 unch. · avg -10.1%
numbers.js — Interp: 🟢 3, 🔴 3, 5 unch. · avg +0.7% · Bytecode: 🔴 11 · avg -10.1%
objects.js — Interp: 🟢 2, 🔴 2, 3 unch. · avg -0.7% · Bytecode: 🟢 1, 🔴 4, 2 unch. · avg -3.3%
promises.js — Interp: 🟢 2, 🔴 3, 7 unch. · avg -0.7% · Bytecode: 🔴 12 · avg -5.2%
regexp.js — Interp: 🟢 1, 🔴 5, 5 unch. · avg -2.2% · Bytecode: 🟢 1, 🔴 9, 1 unch. · avg -4.8%
strings.js — Interp: 🟢 1, 10 unch. · avg +0.3% · Bytecode: 🔴 10, 1 unch. · avg -6.0%
typed-arrays.js — Interp: 🟢 5, 🔴 9, 8 unch. · avg -0.8% · Bytecode: 🔴 21, 1 unch. · avg -11.0%
Measured on ubuntu-latest x64. Benchmark ranges compare cached main-branch min/max ops/sec with the PR run; overlapping ranges are treated as unchanged noise. Percentage deltas are secondary context. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
tests/built-ins/global-properties/goccia.js (1)
14-16: Consider locking down representative members in alias groups.You assert
functions/ranges/classesobjects exist; adding a few key-level checks (for documented entries) would better guard API-surface regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/built-ins/global-properties/goccia.js` around lines 14 - 16, The tests currently only assert the existence of Goccia.semver.functions, .ranges, and .classes; update the test to lock down representative members by adding assertions that specific documented entries exist and have expected types (e.g., check that known function keys exist on Goccia.semver.functions, known range helpers exist on Goccia.semver.ranges, and documented constructors/classes exist on Goccia.semver.classes) using property presence checks (hasOwnProperty or 'in') and simple typeof checks to guard the API surface.tests/built-ins/Goccia/semver/toComparators.js (1)
6-11: Add edge/error assertions fortoComparatorscontract.This currently validates only one happy-path expansion. Please add at least one edge case and one error-path assertion in this same file to match method-level test coverage expectations.
As per coding guidelines: "Tests should cover happy paths, edge cases, and error cases — all in the same file for each method."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/built-ins/Goccia/semver/toComparators.js` around lines 6 - 11, The test only covers the happy path for Goccia.semver.toComparators; add one edge-case assertion (for example a loose/wildcard input like "1.2" or "*" and assert the expanded comparator array contains the expected lower/upper bounds) and one error-path assertion (call Goccia.semver.toComparators with an invalid input such as "invalid" or null and assert it throws using expect(() => ...).toThrow()/toThrowError()). Update the existing test file tests/built-ins/Goccia/semver/toComparators.js by extending the same test block or adding another test in the file that references Goccia.semver.toComparators and includes both the edge-case expectations and the thrown-error expectation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/built-ins/Goccia/semver/Comparator.js`:
- Around line 6-13: Add negative and edge-case assertions for
Goccia.semver.Comparator: add one test that constructing with a malformed
comparator string (e.g., ">>1.2.3" or ">=foo") throws (use expect(() => new
Goccia.semver.Comparator("...")).toThrow()) and another test for an edge input
(empty string or null) that either throws or produces the documented behavior
(use expect(...) toThrow or toEqual per spec). Place these assertions in the
same Comparator.js test file alongside the existing happy-path test and
reference Goccia.semver.Comparator, the constructor, and the instance.test
method to verify error-path coverage.
In `@tests/built-ins/Goccia/semver/inc.js`:
- Around line 6-10: Add bad-input and edge-case assertions for
Goccia.semver.inc: include at least one test expecting an error (or specific
return) when passing a malformed version like "1.2" or "not-a-version" and when
passing an unknown release type like "foobar", and add an edge-case test such as
incrementing a prerelease on a version without prerelease metadata (e.g.,
"1.2.3" with "prerelease") or max-component values; update the existing test
block in tests/built-ins/Goccia/semver/inc.js around the Goccia.semver.inc calls
to include these assertions using the same test() structure and expect(...)
style so happy paths, edge cases, and error cases are all covered.
In `@tests/built-ins/Goccia/semver/satisfies.js`:
- Around line 6-13: Add an error-case assertion for Goccia.semver.satisfies:
within the same test file (either the existing "Goccia.semver.satisfies respects
range and prerelease matching" test or a new nearby test), add an expect that
calling Goccia.semver.satisfies with a malformed version or range (e.g.,
"not-a-version" or an empty/null input) throws an error—use Jest's
toThrow/toThrowError matcher to assert the method raises on invalid input so the
method has happy, edge, and error coverage.
In `@tests/built-ins/Goccia/semver/SemVer.js`:
- Around line 6-17: Add two more assertions to
tests/built-ins/Goccia/semver/SemVer.js: one error-case that constructs
Goccia.semver.SemVer with a malformed version string (e.g., "1..2" or "invalid")
and asserts that it throws (use the test framework's expect(() => new
Goccia.semver.SemVer(...)).toThrow()), and one edge-case that constructs with a
minimal valid semver (e.g., "0.0.0" or "1.0.0+meta" / "1.0.0-0") and asserts the
parsed fields (version, major, minor, patch, prerelease/build) match expected
values; target the existing test block for new Goccia.semver.SemVer to keep
happy, edge and error cases together and use the class name Goccia.semver.SemVer
to locate where to add these assertions.
In `@tests/built-ins/Goccia/semver/validRange.js`:
- Around line 6-9: Add assertions to tests/built-ins/Goccia/semver/validRange.js
covering an invalid input that should return null and an edge-case input (e.g.,
wildcard or empty/partial version) to assert the expected normalized string or
null; specifically, update the test that calls Goccia.semver.validRange to
include one expect(Goccia.semver.validRange("<invalid-input>")).toBeNull() and
one
expect(Goccia.semver.validRange("<edge-input>")).toBe("<expected-normalized-range>")
so the file covers happy, edge, and error paths for Goccia.semver.validRange.
In `@units/Goccia.Builtins.Semver.pas`:
- Around line 523-538: SemVerCompareMainMethod currently considers prerelease
when it should only compare main version components: modify
TGocciaSemverNamespaceHost.SemVerCompareMainMethod (and use ReadSemver/Compare)
to ignore prerelease data (e.g., clear or exclude prerelease from ThisSemver and
OtherSemver) before calling Compare so it returns only major/minor/patch
ordering; change TGocciaSemverNamespaceHost.SemVerComparePreMethod to first call
the main-version comparison (ignoring prereleases) and only if that returns 0
perform a full Compare (including prerelease) to determine prerelease
precedence, returning the appropriate IntegerValue.
- Around line 498-540: Multiple exported methods (e.g., SemVerFormat,
SemVerToStringMethod, SemVerCompareMethod, SemVerCompareMainMethod,
SemVerComparePreMethod and other callbacks in the 576-640, 642-963 ranges) call
semver routines directly and will leak raw EGocciaSemver* exceptions to JS; wrap
the body of each exported callback in the same exception-to-JS-error translation
used by the constructors (reuse the existing HandleSemverException pattern) so
any EGocciaSemver* is caught and converted to the appropriate
ThrowTypeError/ThrowRangeError/ThrowError/etc helper before re-raising, and
ensure ReadSemver, MustParseSemver, Compare, RangeIntersects, MinVersion,
ToComparators and similar calls are executed inside that try/except translation.
- Around line 184-185: GSemverHosts is declared as a raw TObjectList<TObject>
which violates the FPC 3.2.2/AArch64 guideline; define a named type alias (e.g.,
TSemverHostList = class(TObjectList<TObject>) or type TSemverHostList =
TObjectList<TObject>) and replace the raw specialization with that alias in the
GSemverHosts declaration and wherever it is initialized (the initialization code
around the current GSemverHosts creation at/near the block starting at line
~1141). Update any Free/OwnsObjects usage to reference the alias so all
references use the named type instead of TObjectList<TObject>.
In `@units/Goccia.Semver.pas`:
- Around line 1790-1801: The current nested loops in the intersection check
prematurely return False on the first non-intersecting comparator pair, which
aborts checking other OR-disjunct pairs; change the control flow in the block
using LeftRange.SetOfComparators, RightRange.SetOfComparators,
ComparatorIntersects and IsSatisfiableComparatorSet so that a non-intersecting
comparator only cancels the current Left[i]/Right[j] combination (e.g., set a
local flag or break out to the next J) instead of calling Exit(False), and only
when all comparators within a given Left[i]/Right[j] fully intersect should you
set Result := True and Exit; keep existing satisfiability checks via
IsSatisfiableComparatorSet.
- Around line 2137-2142: The branch handling GreaterThanComparator has incorrect
operator precedence; update the condition in the loop over DomainComparator so
the empty-value check applies to both '>' and '>=' operators—i.e., change the if
to check (GreaterThanComparator.Value <> '') and ((DomainComparator.Operator =
'>') or (DomainComparator.Operator = '>=')) before calling
HigherGreaterThanComparator(GreaterThanComparator, DomainComparator, AOptions)
and checking its Value; keep the rest of the logic unchanged.
---
Nitpick comments:
In `@tests/built-ins/global-properties/goccia.js`:
- Around line 14-16: The tests currently only assert the existence of
Goccia.semver.functions, .ranges, and .classes; update the test to lock down
representative members by adding assertions that specific documented entries
exist and have expected types (e.g., check that known function keys exist on
Goccia.semver.functions, known range helpers exist on Goccia.semver.ranges, and
documented constructors/classes exist on Goccia.semver.classes) using property
presence checks (hasOwnProperty or 'in') and simple typeof checks to guard the
API surface.
In `@tests/built-ins/Goccia/semver/toComparators.js`:
- Around line 6-11: The test only covers the happy path for
Goccia.semver.toComparators; add one edge-case assertion (for example a
loose/wildcard input like "1.2" or "*" and assert the expanded comparator array
contains the expected lower/upper bounds) and one error-path assertion (call
Goccia.semver.toComparators with an invalid input such as "invalid" or null and
assert it throws using expect(() => ...).toThrow()/toThrowError()). Update the
existing test file tests/built-ins/Goccia/semver/toComparators.js by extending
the same test block or adding another test in the file that references
Goccia.semver.toComparators and includes both the edge-case expectations and the
thrown-error expectation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86cb62f7-7d3d-4fc8-9fae-dc19da4b4a8a
📒 Files selected for processing (19)
docs/built-ins.mdtests/built-ins/Goccia/semver/Comparator.jstests/built-ins/Goccia/semver/Range.jstests/built-ins/Goccia/semver/SemVer.jstests/built-ins/Goccia/semver/clean.jstests/built-ins/Goccia/semver/coerce.jstests/built-ins/Goccia/semver/compare.jstests/built-ins/Goccia/semver/inc.jstests/built-ins/Goccia/semver/satisfies.jstests/built-ins/Goccia/semver/toComparators.jstests/built-ins/Goccia/semver/valid.jstests/built-ins/Goccia/semver/validRange.jstests/built-ins/global-properties/goccia.jsunits/Goccia.Builtins.Semver.pasunits/Goccia.Constants.ConstructorNames.pasunits/Goccia.Constants.PropertyNames.pasunits/Goccia.Engine.pasunits/Goccia.Runtime.Bootstrap.pasunits/Goccia.Semver.pas
- Move semver property name to a shared constant - Add public API coverage for `Goccia.semver` - Update semver built-in tests to run only when the namespace exists
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
units/Goccia.Builtins.Semver.pas (2)
1374-1382: Consider using a loop to populate ReleaseTypesArray.The manual element-by-element copy with hardcoded indices is fragile if
RELEASE_TYPESlength changes. A loop would be more maintainable:♻️ Suggested refactor
SetLength(ReleaseTypesArray, Length(RELEASE_TYPES)); - ReleaseTypesArray[0] := RELEASE_TYPES[0]; - ReleaseTypesArray[1] := RELEASE_TYPES[1]; - ReleaseTypesArray[2] := RELEASE_TYPES[2]; - ReleaseTypesArray[3] := RELEASE_TYPES[3]; - ReleaseTypesArray[4] := RELEASE_TYPES[4]; - ReleaseTypesArray[5] := RELEASE_TYPES[5]; - ReleaseTypesArray[6] := RELEASE_TYPES[6]; + for I := 0 to High(RELEASE_TYPES) do + ReleaseTypesArray[I] := RELEASE_TYPES[I]; NamespaceObject.AssignProperty('RELEASE_TYPES', Host.StringArrayValue(ReleaseTypesArray));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Builtins.Semver.pas` around lines 1374 - 1382, The manual element-by-element copy from RELEASE_TYPES into ReleaseTypesArray is fragile; replace the hardcoded assignments with a loop that iterates over the range of RELEASE_TYPES (or ReleaseTypesArray after SetLength) and assigns each element dynamically, then call NamespaceObject.AssignProperty('RELEASE_TYPES', Host.StringArrayValue(ReleaseTypesArray)) as before; update the code around ReleaseTypesArray, RELEASE_TYPES and the SetLength call to use a for-loop so it adapts automatically if RELEASE_TYPES changes.
1091-1141: Consider using a more efficient sorting algorithm.The current bubble sort implementation is O(n²). For typical semver array sizes this is likely acceptable, but if larger arrays are expected, consider using a more efficient algorithm like quicksort or leveraging Pascal's built-in sorting utilities.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Builtins.Semver.pas` around lines 1091 - 1141, Replace the O(n²) manual bubble-sort loops in NamespaceSort and NamespaceRSort with a single call to a faster built-in sort (e.g., TArray.Sort or creating a TList<string> and calling its Sort) using a comparer that calls CompareBuild(Left, Right, Options); for NamespaceRSort invert the comparer result (or swap arguments) to get descending order. Update TGocciaSemverNamespaceHost.NamespaceSort and NamespaceRSort to build the comparer capturing Options, call the library sort on Values, and then return StringArrayValue(Values); keep the existing exception handling.units/Goccia.Semver.pas (2)
1082-1085: Misleading indentation on the fallthrough raise.The
raisestatement on line 1084 is indented at the same level asExit(...)on line 1083, making it appear to be part of theif AOperator = '<='block. However, withoutbegin/end, only theExitis conditional—theraiseexecutes unconditionally after theifwhen all prior conditions fail.The code is functionally correct (the raise properly handles invalid operators), but the indentation is confusing.
♻️ Suggested fix to clarify control flow
if AOperator = '<=' then Exit(Compare(ALeft, ARight, AOptions) <= 0); - raise EGocciaSemverTypeError.Create('Invalid operator: ' + AOperator); + raise EGocciaSemverTypeError.Create('Invalid operator: ' + AOperator); end;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Semver.pas` around lines 1082 - 1085, The raise is visually nested under the if for AOperator = '<=' but actually executes unconditionally; update the control flow clarity by either outdenting the raise statement so it aligns with the surrounding if/Exit lines or wrap the conditional block with begin/end if you intend the raise to be part of the if/else structure—adjust the lines that use AOperator = '<=', the Exit(Compare(ALeft, ARight, AOptions) <= 0) call, and the EGocciaSemverTypeError.Create('Invalid operator: ' + AOperator) raise to make the intended flow explicit.
1987-1991: Remove dead code: unnecessary while loop.This
while True do ... Breakconstruct is equivalent to a single assignmentResult := ''. The loop executes exactly once, setsResultto empty, and immediately breaks. SinceResultis reassigned later (lines 2024-2047), this block serves no purpose.🧹 Suggested cleanup
for I := Low(AVersions) to High(AVersions) do SortedVersions[I - Low(AVersions)] := AVersions[I]; - while True do - begin - Result := ''; - Break; - end; FirstValue := '';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@units/Goccia.Semver.pas` around lines 1987 - 1991, Remove the redundant infinite loop block "while True do begin Result := ''; Break; end;" — it only sets Result once and immediately breaks; either delete the entire loop or replace it with the single assignment "Result := ''" if an initial value is needed, and ensure the surrounding function (the routine that returns Result) continues to use the later reassignment logic unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/built-ins/Goccia/semver/SemVer.comparePre.js`:
- Around line 1-15: The test file currently testing the prototype method
SemVer.comparePre should be moved into the prototype/ tests folder and its
import/run references updated accordingly: relocate the file to the
tests/built-ins/Goccia/semver/prototype/ directory (keeping the same filename),
ensure the test continues to reference Goccia.semver.SemVer and
describe.runIf(hasGoccia) as before, and update any test harness or index that
collects tests so prototype tests are discovered from the new prototype/ path.
- Around line 9-14: Add edge and error-path tests for SemVer.comparePre: include
assertions that comparePre returns 0 for identical prerelease precedence (e.g.,
two SemVer instances with the same prerelease identifiers), tests for differing
prerelease lengths/order (e.g., "1.2.3-alpha" vs "1.2.3-alpha.1" and
"1.2.3-alpha.2" vs "1.2.3-alpha.10") asserting the expected -1/1 results, and
explicit error-path assertions that malformed version strings or invalid inputs
to new Goccia.semver.SemVer(...) or comparePre(...) produce the API-defined
failure (use expect(...).toThrow() or the project's defined error behavior).
Ensure all tests live in this same file alongside the existing comparePre tests
and reference Goccia.semver.SemVer and its comparePre method.
---
Nitpick comments:
In `@units/Goccia.Builtins.Semver.pas`:
- Around line 1374-1382: The manual element-by-element copy from RELEASE_TYPES
into ReleaseTypesArray is fragile; replace the hardcoded assignments with a loop
that iterates over the range of RELEASE_TYPES (or ReleaseTypesArray after
SetLength) and assigns each element dynamically, then call
NamespaceObject.AssignProperty('RELEASE_TYPES',
Host.StringArrayValue(ReleaseTypesArray)) as before; update the code around
ReleaseTypesArray, RELEASE_TYPES and the SetLength call to use a for-loop so it
adapts automatically if RELEASE_TYPES changes.
- Around line 1091-1141: Replace the O(n²) manual bubble-sort loops in
NamespaceSort and NamespaceRSort with a single call to a faster built-in sort
(e.g., TArray.Sort or creating a TList<string> and calling its Sort) using a
comparer that calls CompareBuild(Left, Right, Options); for NamespaceRSort
invert the comparer result (or swap arguments) to get descending order. Update
TGocciaSemverNamespaceHost.NamespaceSort and NamespaceRSort to build the
comparer capturing Options, call the library sort on Values, and then return
StringArrayValue(Values); keep the existing exception handling.
In `@units/Goccia.Semver.pas`:
- Around line 1082-1085: The raise is visually nested under the if for AOperator
= '<=' but actually executes unconditionally; update the control flow clarity by
either outdenting the raise statement so it aligns with the surrounding if/Exit
lines or wrap the conditional block with begin/end if you intend the raise to be
part of the if/else structure—adjust the lines that use AOperator = '<=', the
Exit(Compare(ALeft, ARight, AOptions) <= 0) call, and the
EGocciaSemverTypeError.Create('Invalid operator: ' + AOperator) raise to make
the intended flow explicit.
- Around line 1987-1991: Remove the redundant infinite loop block "while True do
begin Result := ''; Break; end;" — it only sets Result once and immediately
breaks; either delete the entire loop or replace it with the single assignment
"Result := ''" if an initial value is needed, and ensure the surrounding
function (the routine that returns Result) continues to use the later
reassignment logic unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1aa5074f-0ff2-4e47-a907-18b68c3961bc
📒 Files selected for processing (19)
tests/built-ins/Goccia/semver/Comparator.jstests/built-ins/Goccia/semver/Range.jstests/built-ins/Goccia/semver/SemVer.compareMain.jstests/built-ins/Goccia/semver/SemVer.comparePre.jstests/built-ins/Goccia/semver/SemVer.jstests/built-ins/Goccia/semver/clean.jstests/built-ins/Goccia/semver/coerce.jstests/built-ins/Goccia/semver/compare.jstests/built-ins/Goccia/semver/inc.jstests/built-ins/Goccia/semver/intersects.jstests/built-ins/Goccia/semver/satisfies.jstests/built-ins/Goccia/semver/toComparators.jstests/built-ins/Goccia/semver/valid.jstests/built-ins/Goccia/semver/validRange.jstests/built-ins/global-properties/goccia.jsunits/Goccia.Builtins.Semver.pasunits/Goccia.Engine.pasunits/Goccia.Runtime.Bootstrap.pasunits/Goccia.Semver.pas
✅ Files skipped from review due to trivial changes (10)
- tests/built-ins/Goccia/semver/valid.js
- tests/built-ins/Goccia/semver/inc.js
- tests/built-ins/Goccia/semver/clean.js
- tests/built-ins/global-properties/goccia.js
- tests/built-ins/Goccia/semver/coerce.js
- tests/built-ins/Goccia/semver/SemVer.compareMain.js
- tests/built-ins/Goccia/semver/Range.js
- tests/built-ins/Goccia/semver/intersects.js
- tests/built-ins/Goccia/semver/Comparator.js
- tests/built-ins/Goccia/semver/satisfies.js
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/built-ins/Goccia/semver/SemVer.js
- tests/built-ins/Goccia/semver/compare.js
- tests/built-ins/Goccia/semver/toComparators.js
- units/Goccia.Runtime.Bootstrap.pas
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/built-ins/Goccia/semver/prototype/compareMain.js (1)
8-13: Missing error cases and edge case coverage.Per coding guidelines, tests should cover happy paths, edge cases, and error cases in the same file. This file only has one test covering two assertions. Based on the implementation context (
SemVerCompareMainMethodcallsReadSemverwhich throws for invalid inputs), error handling should be tested.Consider adding:
📝 Suggested additional test coverage
describe.runIf(hasGoccia)("Goccia.semver.SemVer.prototype.compareMain", () => { test("compareMain ignores prerelease metadata", () => { const version = new Goccia.semver.SemVer("1.2.3-alpha.1"); expect(version.compareMain("1.2.3")).toBe(0); expect(version.compareMain("1.2.4-alpha.1")).toBe(-1); }); + + test("compareMain compares major, minor, and patch components", () => { + const version = new Goccia.semver.SemVer("2.3.4"); + expect(version.compareMain("1.3.4")).toBe(1); + expect(version.compareMain("2.2.4")).toBe(1); + expect(version.compareMain("2.3.3")).toBe(1); + expect(version.compareMain("3.0.0")).toBe(-1); + }); + + test("compareMain throws for invalid versions", () => { + expect(() => new Goccia.semver.SemVer("invalid")).toThrow(TypeError); + expect(() => new Goccia.semver.SemVer("1.2.3").compareMain("bad")).toThrow(TypeError); + }); });As per coding guidelines: "Tests should cover happy paths, edge cases, and error cases — all in the same file for each method."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/built-ins/Goccia/semver/prototype/compareMain.js` around lines 8 - 13, Add tests to cover edge and error cases for Goccia.semver.SemVer.prototype.compareMain: add cases that call compareMain with invalid inputs that trigger ReadSemver to throw (e.g., non-string, malformed version), assert that the method throws the expected error, and include boundary/edge comparisons (equal mains with different prerelease, larger major/minor differences, and inputs with build metadata). Reference the SemVer constructor, compareMain, and ReadSemver when adding tests and use the same describe.runIf(hasGoccia) / test structure to keep style consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/built-ins/Goccia/semver/prototype/compareMain.js`:
- Around line 8-13: Add tests to cover edge and error cases for
Goccia.semver.SemVer.prototype.compareMain: add cases that call compareMain with
invalid inputs that trigger ReadSemver to throw (e.g., non-string, malformed
version), assert that the method throws the expected error, and include
boundary/edge comparisons (equal mains with different prerelease, larger
major/minor differences, and inputs with build metadata). Reference the SemVer
constructor, compareMain, and ReadSemver when adding tests and use the same
describe.runIf(hasGoccia) / test structure to keep style consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0685a62-641b-4f83-b0a5-592617b908bf
📒 Files selected for processing (2)
tests/built-ins/Goccia/semver/prototype/compareMain.jstests/built-ins/Goccia/semver/prototype/comparePre.js
This was referenced Apr 12, 2026
This was referenced May 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Goccia.semvernamespace on the globalGocciaobject with SemVer 2.0.0 constants, constructors, helper groups, and top-level functions.SemVer,Comparator, andRangeobjects with node-semver-like fields and methods.valid,clean,compare,inc,satisfies, andtoComparators.Testing
Goccia.semvernamespace exposure and key API behavior undertests/built-ins/Goccia/semver/Summary by CodeRabbit
Release Notes
New Features
Goccia.semvernamespace providing semantic versioning (SemVer 2.0.0) functionality, including parsing, comparison, validation, range handling, and version incrementation.Documentation
Tests