Skip to content

Report invalid DateInterval constructor arguments at analysis time#5587

Open
phpstan-bot wants to merge 4 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-v8z1qut
Open

Report invalid DateInterval constructor arguments at analysis time#5587
phpstan-bot wants to merge 4 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-v8z1qut

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

@phpstan-bot phpstan-bot commented May 3, 2026

Summary

PHPStan did not report errors when DateInterval was constructed with an invalid duration string (e.g. new DateInterval('1M') instead of new DateInterval('P1M')). While there was already a DateIntervalConstructorThrowTypeExtension that refined the throw type, there was no rule to actually report the error to the user.

Fixes phpstan/phpstan#14442

Comment thread src/Rules/DateTimeZoneInstantiationRule.php Outdated
@VincentLanglet VincentLanglet changed the title Report invalid DateInterval and DateTimeZone constructor arguments at analysis time Report invalid DateInterval constructor arguments at analysis time May 3, 2026
return new DateIntervalInstantiationRule();
}

public function test(): void
Copy link
Copy Markdown
Contributor

@staabm staabm May 3, 2026

Choose a reason for hiding this comment

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

this tests expectations only work for PHP 8.1+.
make it run only on PHP 8.1+.

add another test referecing the same __DIR__ . '/data/date-interval-instantiation.php' path but with different expectations for PHP versions 8.0/7.4

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. The review feedback has been addressed:

  • Added #[RequiresPhp('>= 8.1.0')] to the existing test() method since its error message expectations match PHP 8.1+ format
  • Added a new testLegacyPhp() method with #[RequiresPhp('< 8.1.0')] that references the same test data file but expects the DateInterval::__construct(): prefix in error messages (the format used on PHP 7.4/8.0)
  • All tests pass and make phpstan reports no errors

staabm and others added 3 commits May 3, 2026 13:16
…s at analysis time

- Add DateIntervalInstantiationRule that validates constant duration strings
  passed to the DateInterval constructor and reports errors for invalid ones
- Add DateTimeZoneInstantiationRule that validates constant timezone strings
  passed to the DateTimeZone constructor and reports errors for invalid ones
- Both rules follow the same pattern as the existing DateTimeInstantiationRule:
  extract constant strings, attempt construction, report exceptions as errors
- Both registered at level 5, matching DateTimeInstantiationRule
The DateTimeZone validation rule is less valuable than the DateInterval
case since DateInterval durations are more commonly known at analysis time.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On PHP < 8.1, DateInterval::__construct() prefixes the exception
message with "DateInterval::__construct(): ", while PHP 8.1+ uses
a shorter format. Gate the existing test with RequiresPhp(>= 8.1.0)
and add a separate testLegacyPhp method for older PHP versions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@staabm staabm force-pushed the create-pull-request/patch-v8z1qut branch from 070b036 to c0929f9 Compare May 3, 2026 11:16
/**
* @implements Rule<Node\Expr\New_>
*/
#[RegisteredRule(level: 5)]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should it be bleeding edge only?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. The DateIntervalInstantiationRule is now bleeding edge only:

  • Removed #[RegisteredRule(level: 5)] from the rule class
  • Added checkDateIntervalConstructor feature toggle (default false in config.neon, true in bleedingEdge.neon)
  • Added the toggle to the parameters schema
  • Registered the rule as a service in config.level5.neon with a conditionalTags entry gated on the feature toggle

All tests pass and make phpstan reports no errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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