Skip to content

[Rector Rule] Replace date, strtotime, and time calls with Carbon equivalents#6749

Merged
samsonasik merged 12 commits intorectorphp:mainfrom
gollumeo:feat/replace-strtotime-with-carbon
Mar 4, 2025
Merged

[Rector Rule] Replace date, strtotime, and time calls with Carbon equivalents#6749
samsonasik merged 12 commits intorectorphp:mainfrom
gollumeo:feat/replace-strtotime-with-carbon

Conversation

@gollumeo
Copy link
Contributor

Related issue

Closes #8695

Description

This PR adds a Rector rule to replace certain native PHP date/time function calls with Carbon equivalents. The conversions are based on the examples provided in the original issue

Implemented changes

  • date('format', strotime(...))Carbon::parse(...)->format('format')
  • strtotime($date)Carbon::parse($date)->timestamp
  • time() - (60 * 60 * 24 * 14)Carbon::now()->subWeeks(2)->timestamp

Code examples

Before

$dateFormatted = date('d.m.Y', strtotime($invoice->getDate()));
$timestamp = strtotime('2025-02-20');
$twoWeeksAgo = time() - (60 * 60 * 24 * 14);

After

$dateFormatted = \Carbon\Carbon::parse($invoice->getDate())->format('d.m.Y');
$timestamp = \Carbon\Carbon::parse('2025-02-20')->timestamp;
$twoWeeksAgo = \Carbon\Carbon::now()->subWeeks(2)->timestamp;

Tests & validation

  • Added unit tests for each transformation, using these fixtures:
    • strtotime.php.inc
    • date_with_strtotime.php.inc
    • time_calculation.php.inc
    • simple_date.php.inc
  • Validated conversions using PHPUnit
    image

Notes

  • The rule detects nested calls (e.g., date('format', strtotime(...)) and applies the correct Carbon replacement
  • Supports time calculations that are multiples of common time units (seconds, minutes, hours, days, weeks)
  • The rule ensures backward compatibility by strictly replacing only the targeted patterns
Looking forward to any suggestions or improvements! Let me know if there's anything else I should adjust 😁

- Add DateFuncCallToCarbonRector to replace date(), strtotime(), and time() - patterns
- Handle parent Minus node for time() - (N) or time() - (60 * 60 * 24 * N)
- Fix detectTimeUnit() to support both LNumber and Mul
- Reverse TIME_UNITS order to detect subWeeks() before subSeconds()
- Update fixtures to cover seconds, minutes, hours, days, and weeks
@TomasVotruba TomasVotruba requested a review from samsonasik March 3, 2025 19:05
@samsonasik
Copy link
Member

/cc @melbings @kylekatarnls could you verify this change? Thank you.

@samsonasik
Copy link
Member

Thank you @kylekatarnls @gollumeo , let's give it a try 👍

@samsonasik samsonasik merged commit 61d31e1 into rectorphp:main Mar 4, 2025
44 checks passed
@johnbacon
Copy link

We're seeing an issue at times with this –

Before

date('Y', $pricing->timestamp) . ')

After

Carbon::now()->format('Y')

$pricing->timestamp is a model property that's returned as a Carbon object. We saw the same thing happen on another model property that's returned as a Carbon object.

For this reason, we don't feel comfortable enabling this going forward, as it would break functionality. But if this can be addressed, it's a great feature!

@kylekatarnls
Copy link
Contributor

I don't get the case: if date function has 2 arguments, it will not use Carbon::now(), can you create a minimalist file to reproduce the issue and show the code you actually get when running the rule on this file?

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2025

This pull request has been automatically locked because it has been closed for 150 days. Please open a new PR if you want to continue the work.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor time, date and strtotime function usage to Carbon

4 participants