Skip to content

fix: align float parsing with strconv and harden helper functions#229

Merged
ReneWerner87 merged 3 commits into
masterfrom
fix/parse-float-and-improvements
Jul 7, 2026
Merged

fix: align float parsing with strconv and harden helper functions#229
ReneWerner87 merged 3 commits into
masterfrom
fix/parse-float-and-improvements

Conversation

@ReneWerner87

@ReneWerner87 ReneWerner87 commented Jul 5, 2026

Copy link
Copy Markdown
Member

Summary

Fixes two real divergences between ParseFloat64/ParseFloat32 and strconv.ParseFloat, plus several small hardening improvements across the package, and refreshes the README benchmarks.

Float parsing (parse.go)

  • Digit-less inputs like ".", "e5" and "+." now return ErrSyntax instead of 0, nil
  • Integer parts larger than uint64 (e.g. "18446744073709551616") now parse correctly instead of returning ErrRange; strconv parses these fine
  • More than 16 fractional digits no longer return ErrRange; excess digits are ignored, matching strconv
  • The parser now collects digits into a single mantissa with a decimal exponent instead of separate int/frac parts; still 0 allocs and ~1.7x faster than strconv (13.9 ns/op vs 24.2 ns/op)

Improvements

  • ConvertToBytes: returns 0 for negative sizes (previously "-5.0k" returned -5000 while "-5k" returned 0); supports binary units (42KiB = 43008, previously silently parsed as decimal 42000); doc comment now explains the decimal vs binary relation to ByteSize
  • GetMIME: extension lookup is now case-insensitive via the internal ToLower (no alloc for lower-case input), so upper-case extensions like "PNG" hit the internal table instead of the platform-dependent mime fallback
  • StartTimeStampUpdater/StopTimeStampUpdater: now safe for concurrent use; previously two parallel stops could panic on double-close and the sync.Once reset raced with a concurrent start
  • ToString: direct error case before fmt.Stringer (matching fmt precedence), avoiding the reflection fallback
  • Trim/TrimLeft/TrimRight: corrected doc comments; cutset is a single byte, not a character set as in the stdlib counterparts

Docs

  • README benchmark numbers re-measured on the same environment (Apple M2 Pro, darwin/arm64, go1.25)

Testing

  • Full suite with -race: 483 tests passing
  • Three ParseFloat test expectations asserted the old divergent behavior and were updated to the strconv-conforming values; new cases cover digit-less inputs, KiB units, negative sizes, upper-case extensions and error in ToString
  • golangci-lint (CI version v2.12.2) reports no issues in the changed files

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Improved number parsing to handle a wider range of float inputs more reliably.
    • Added support for binary size units like KiB and MiB when converting byte sizes.
    • Timestamp updating can now be started and stopped safely multiple times.
  • Bug Fixes

    • MIME type detection now works with uppercase file extensions.
    • ToString now formats error values using their message text.
    • Negative size inputs are now rejected more consistently.
  • Documentation

    • Refreshed benchmark results in the README and clarified byte-trimming behavior.

ReneWerner87 and others added 2 commits July 5, 2026 22:16
ParseFloat64/ParseFloat32 diverged from strconv.ParseFloat in three ways:
digit-less inputs (".", "e5", "+.") returned 0 without an error, integer
parts larger than uint64 returned ErrRange although float64 can represent
them, and more than 16 fractional digits returned ErrRange. The parser now
collects digits into a single mantissa with a decimal exponent, fixing all
three cases while staying allocation-free.

Further improvements:
- ConvertToBytes: return 0 for negative sizes, support binary units
  (42KiB = 43008), document decimal vs binary semantics
- GetMIME: case-insensitive extension lookup via internal ToLower,
  avoiding the platform-dependent mime fallback for upper-case input
- Start/StopTimeStampUpdater: guard with a mutex to prevent double-close
  panics and races on concurrent use
- ToString: handle error before fmt.Stringer, matching fmt precedence
- Trim/TrimLeft/TrimRight: correct doc comments (single byte cutset)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Re-measured on the same environment (Apple M2 Pro, darwin/arm64).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 5, 2026 20:17
@ReneWerner87 ReneWerner87 requested a review from a team as a code owner July 5, 2026 20:17
@ReneWerner87 ReneWerner87 requested review from efectn, gaby and sixcolors and removed request for a team July 5, 2026 20:17
@ReneWerner87 ReneWerner87 added the ☢️ Bug Something isn't working label Jul 5, 2026
@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.02985% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.94%. Comparing base (0725808) to head (1406ead).

Files with missing lines Patch % Lines
time.go 84.00% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #229      +/-   ##
==========================================
- Coverage   84.02%   83.94%   -0.09%     
==========================================
  Files          14       14              
  Lines        1158     1177      +19     
==========================================
+ Hits          973      988      +15     
- Misses        154      156       +2     
- Partials       31       33       +2     
Flag Coverage Δ
unittests 83.94% <94.02%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR brings several utility helpers in this repository closer to Go stdlib behavior (notably float parsing vs strconv.ParseFloat) and hardens a few frequently-used helpers, while updating published benchmark numbers.

Changes:

  • Reworked parseFloat to handle digit-less inputs as syntax errors and to better match strconv behavior for large integer parts and long fractional parts.
  • Improved helper robustness: case-insensitive MIME lookups, binary-unit support + negative handling in ConvertToBytes, safer timestamp updater start/stop, and ToString precedence adjustments.
  • Updated README benchmark outputs to reflect newly re-measured results.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
time.go Replaces sync.Once with a mutex/chan lifecycle to make timestamp updater start/stop safe for concurrent callers.
README.md Refreshes benchmark output numbers.
parse.go Updates float parsing to be more strconv-compatible (syntax handling, large integer parts, long fractional parts).
parse_test.go Adjusts/extends float parsing expectations to match the new parseFloat behavior.
http.go Makes extension lookup case-insensitive by normalizing keys before map lookup.
http_test.go Adds coverage for upper-case extension inputs.
convert.go Ensures error is handled before fmt.Stringer in ToString.
convert_test.go Adds tests for ToString(error) behavior.
common.go Enhances ConvertToBytes to reject negatives and support binary KiB-style units.
common_test.go Adds tests for binary units, negative sizes, and strengthens UUIDv4 format validation.
byteseq.go Corrects Trim* doc comments to clarify cutset is a single byte.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread parse.go
@coderabbitai

coderabbitai Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@ReneWerner87, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 49 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f1f714a5-4643-4e88-a9ca-df35de5d8178

📥 Commits

Reviewing files that changed from the base of the PR and between ae095a8 and 1406ead.

📒 Files selected for processing (2)
  • parse.go
  • parse_test.go
📝 Walkthrough

Walkthrough

This PR updates ConvertToBytes to support binary ('i' infix) unit suffixes and reject negative sizes, adds an explicit error case to ToString, makes GetMIME case-insensitive, rewrites parseFloat's mantissa/exponent accumulation, converts the timestamp updater to a restartable mutex-based design, clarifies trim doc comments, and refreshes README benchmark figures.

Changes

Utility fixes and benchmark refresh

Layer / File(s) Summary
ConvertToBytes binary unit support
common.go, common_test.go
Adds parsing for binary ('i' infix) unit suffixes with 1024-based multipliers, rejects negative parsed sizes, and updates doc comments and tests.
ToString error handling
convert.go, convert_test.go
Adds explicit error case to ToString returning v.Error(), with new test coverage and errors import.
Case-insensitive MIME lookup
http.go, http_test.go
Lowercases extension key before mimeExtensions map lookup, with tests for uppercase extensions.
parseFloat mantissa/exponent rewrite
parse.go, parse_test.go
Replaces integer/fraction parsing with mantissa+exp10 accumulation, removes maxFracDigits, and updates float64/float32 test expectations.
Timestamp updater mutex-based restart
time.go
Replaces sync.Once with a mutex and channel-state checks so the updater can be stopped and restarted.
Trim doc comment clarifications
byteseq.go
Updates TrimLeft, Trim, TrimRight doc comments to clarify single-byte cutset behavior.
Test_UUIDv4 regex assertion
common_test.go
Adds regex-based UUIDv4 format validation.
README benchmark refresh
README.md
Replaces benchmark result figures across all sections with updated metric values.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant StartTimeStampUpdater
  participant StopTimeStampUpdater
  participant TickerGoroutine
  Caller->>StartTimeStampUpdater: call
  StartTimeStampUpdater->>StartTimeStampUpdater: lock updaterMu, check stopUpdater
  StartTimeStampUpdater->>TickerGoroutine: start goroutine
  Caller->>StopTimeStampUpdater: call
  StopTimeStampUpdater->>StopTimeStampUpdater: lock updaterMu, close stopUpdater
  TickerGoroutine->>StopTimeStampUpdater: close updaterDone
  StopTimeStampUpdater->>StopTimeStampUpdater: clear channels to nil
Loading

Possibly related PRs

  • gofiber/utils#91: Overlaps in byteseq.go's TrimLeft, Trim, TrimRight doc comments/functions.
  • gofiber/utils#126: Refactors the same StartTimeStampUpdater/StopTimeStampUpdater lifecycle synchronization logic in time.go.
  • gofiber/utils#134: Reworks the same parseFloat logic in parse.go with corresponding test updates.

Suggested labels: 🧹 Updates

Suggested reviewers: sixcolors, efectn, gaby

Poem

A rabbit hops through bytes and bits,
Ki and Mi now fit like knits,
Errors speak, and MIME don't care
If letters big or small are there,
Floats align with mantissa's grace,
Clocks restart without a chase. 🐇⏱️

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main float parsing fix and the broader helper hardening in the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/parse-float-and-improvements

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Clamping the parsed exponent to the 309/-325 sentinels before combining
it with the mantissa exponent (exp10) corrupted results for long digit
strings: "999...9e-390" (400 nines, true value ~1e10) came back as 1e75
because the parsed -390 was clamped to -325 and then shifted by +381.
The parse loop now only saturates the exponent against int64 overflow
and the clamp is applied to the combined exponent, which also keeps the
int conversion for math.Pow10 safe on 32-bit platforms.

Also skip the Pow10 scaling for a zero mantissa: 0 * Pow10(309) is NaN,
which turned inputs like "0e400" into a spurious range error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Comment thread common.go
@ReneWerner87 ReneWerner87 merged commit c4eb8e8 into master Jul 7, 2026
18 of 19 checks passed
@ReneWerner87 ReneWerner87 deleted the fix/parse-float-and-improvements branch July 7, 2026 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants