Skip to content

Fix LocalDateTime::plusNanos() and minusNanos() within the same second - #136

Merged
BenMorel merged 1 commit into
brick:mainfrom
gnutix:fix/local-date-time-plus-nanos
Oct 9, 2026
Merged

BenMorel merged 1 commit into
brick:mainfrom
gnutix:fix/local-date-time-plus-nanos

Conversation

@gnutix

@gnutix gnutix commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Problem

LocalDateTime::plusNanos() and minusNanos() return the original value when the result stays within the same second:

LocalDateTime::parse('2000-01-01T12:34:56.5')->plusNanos(1);  // 2000-01-01T12:34:56.5, expected 2000-01-01T12:34:56.500000001
LocalDateTime::parse('2000-01-01T12:34:56')->plusNanos(1);    // 2000-01-01T12:34:56,   expected 2000-01-01T12:34:56.000000001
LocalDateTime::parse('2000-01-01T12:34:56.5')->minusNanos(1); // 2000-01-01T12:34:56.5, expected 2000-01-01T12:34:56.499999999

plusWithOverflow() reuses the current LocalTime whenever the second-of-day is unchanged, which drops the newly computed nano-of-second. Java's original compares the nano-of-day instead. The existing test cases all cross a second boundary, so they did not catch it.

The other callers (plusHours(), plusMinutes(), plusSeconds() and their minus counterparts) pass zero nanos, so the shortcut was correct for them and they are unaffected.

Change

The shortcut now also requires the nano-of-second to be unchanged. New test cases cover adding and subtracting nanos within the same second, including a whole-day jump where the second-of-day comes back equal.

🤖 Generated with Claude Code, Opus 5.5

https://claude.ai/code/session_01Fm9bvgKFasEGzpK48bQrxA

plusWithOverflow() reused the current time whenever the second-of-day was
unchanged, discarding the new nano-of-second. Adding or subtracting nanos
that did not cross a second boundary therefore returned the original value:

    LocalDateTime::parse('2000-01-01T12:34:56.5')->plusNanos(1);
    // 2000-01-01T12:34:56.5 instead of 2000-01-01T12:34:56.500000001

The shortcut now also requires the nano-of-second to be unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fm9bvgKFasEGzpK48bQrxA
@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.23%. Comparing base (2cafd91) to head (6447ef4).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #136   +/-   ##
=========================================
  Coverage     99.23%   99.23%           
- Complexity     1139     1140    +1     
=========================================
  Files            48       48           
  Lines          2489     2491    +2     
=========================================
+ Hits           2470     2472    +2     
  Misses           19       19           

☔ 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.

@BenMorel
BenMorel merged commit 88f85b5 into brick:main Oct 9, 2026
8 of 9 checks passed
@BenMorel

BenMorel commented Oct 9, 2026

Copy link
Copy Markdown
Member

Thank you, @gnutix!

@BenMorel

BenMorel commented Oct 9, 2026

Copy link
Copy Markdown
Member

Released as 0.10.4.

@gnutix

gnutix commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor Author

Just so you know, it's totally okay for me if you don't release each bug fix "right away" :) If I ever have a bug fix that's urgent, I'll let you know (though I'll probably just fix it in my project with a composer patch first anyway 😄 ).

Of course, your reactivity is much appreciated, and it makes it that much nicer to contribute back to this library. 🙏

@BenMorel

Copy link
Copy Markdown
Member

I usually prefer to not keep things unreleased, but I may group some of your upcoming fixes under the same release! 😉

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.

2 participants