Skip to content

Add the years and months of a Period at once in LocalDate::plusPeriod() - #141

Merged
BenMorel merged 2 commits into
brick:mainfrom
gnutix:fix/local-date-plus-period-feb-29
Oct 10, 2026
Merged

BenMorel merged 2 commits into
brick:mainfrom
gnutix:fix/local-date-plus-period-feb-29

Conversation

@gnutix

@gnutix gnutix commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

Item 2 of #137.

Problem

LocalDate::plusPeriod() called plusYears(), then plusMonths(), so a Feb 29 was clamped by both steps:

$d = LocalDate::of(2020, 2, 29);
$d->plusPeriod(Period::of(1, 1, 0));                   // 2021-03-28, but P13M gives 2021-03-29
$d->plusPeriod($d->until(LocalDate::of(2021, 3, 29))); // 2021-03-28: until() and plusPeriod() don't round-trip
LocalDate::of(999995, 2, 26)->plusPeriod(Period::of(5, -29, 5)); // throws "Invalid year: 1000000", the result 999997-10-01 is valid

Change

The years and months are added as a single number of months, as java.time does (plusMonths(toTotalMonths())), then the days. LocalDateTime::plusPeriod() and ZonedDateTime::plusPeriod() delegate to it, and minusPeriod() goes through it.

New cases in the providerPeriod of LocalDateTest and LocalDateTimeTest, used by both plusPeriod and minusPeriod.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Fm9bvgKFasEGzpK48bQrxA

plusPeriod() added the years, then the months, so a Feb 29 was clamped by
both steps, and an intermediate year could be out of range even when the
result was valid:

    LocalDate::of(2020, 2, 29)->plusPeriod(Period::of(1, 1, 0));     // 2021-03-28 instead of 2021-03-29
    LocalDate::of(999995, 2, 26)->plusPeriod(Period::of(5, -29, 5)); // "Invalid year: 1000000"

It now adds the total number of months in one step, as java.time does.
LocalDateTime and ZonedDateTime::plusPeriod() go through it.

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

codecov Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.24%. Comparing base (44f8c01) to head (846b73e).

Additional details and impacted files
@@            Coverage Diff            @@
##               main     #141   +/-   ##
=========================================
  Coverage     99.24%   99.24%           
  Complexity     1141     1141           
=========================================
  Files            48       48           
  Lines          2501     2501           
=========================================
  Hits           2482     2482           
  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 ad12536 into brick:main Oct 10, 2026
10 checks passed
@BenMorel

Copy link
Copy Markdown
Member

Thank you, @gnutix!

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