Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added better stubs for DateTimeImmutable, highlighting how the constructor is **NOT** immutable #8350

Merged
merged 22 commits into from Aug 7, 2022

Commits on Jul 31, 2022

  1. Added better stubs for DateTimeImmutable, highlighting how the cons…

    …tructor is **NOT** immutable
    
    `DateTimeImmutable` is **almost** immutable: `DateTimeImmutable::__construct()` is in fact not a pure
    method, since `new DateTimeImmutable('now')` produces a different value at each instantiation (by design).
    
    This change makes sure that `DateTimeImmutable` loses its `@psalm-immutable` class-level marker,
    preventing downstream misuse of the constructor inside otherwise referentially transparent code.
    
    Note: only pure methods are stubbed here: all other methods declared by `DateTimeImmutable` (parent interface)
    are NOT present here, and are either inferred from runtime reflection, or `CallMap*.php` definitions.
    
    Methods are sorted in the order defined by reflection on PHP 8.1.8, at the time of writing this ( https://3v4l.org/3TGg8 ).
    
    Following simplistic snippet was used to infer the current signature:
    
    ```php
    <?php
    
    $c = new \ReflectionClass(\DateTimeImmutable::class);
    
    $methods = array_map(function ($m) {
        return $m->getName()
            . '(' . implode(',', array_map(function ($p) {
                return $p->getType()
                    . ' $' . $p->getName()
                    . ($p->isOptional() ? ' = ' . var_export($p->getDefaultValue(), true) : '');
            }, $m->getParameters())) . ')' . ($m->getReturnType() ? (': ' . $m->getReturnType()) : '');
    }, $c->getMethods());
    
    $properties = array_map(function ($m) {
        return $m->getName();
    }, $c->getProperties());
    
    var_dump($methods, $properties);
    ```
    Ocramius committed Jul 31, 2022
    Configuration menu
    Copy the full SHA
    b4b2bc6 View commit details
    Browse the repository at this point in the history
  2. Removed @psalm-immutable marked from MyDate extending `DateTimeIm…

    …mutable`
    
    `DateTimeImmutable` is not really immutable, therefore this marker was wrong upfront
    Ocramius committed Jul 31, 2022
    Configuration menu
    Copy the full SHA
    dcaf610 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    c205d65 View commit details
    Browse the repository at this point in the history

Commits on Aug 1, 2022

  1. s/psalm-pure/psalm-mutation-free, since psalm-mutation-free is safer …

    …to use
    
    Ref: https://github.com/vimeo/psalm/pull/8350/files/c205d652d1e9afd9510db59e72c3fd0a4a093b3d#r934032422
    
    The idea is that `@psalm-pure` disallows `$this` usage in child classes,
    which is not wanted, while `@psalm-mutation-free` allows it.
    
    By using `@psalm-mutation-free`, we don't completely destroy inheritance
    use-cases based on internal (immutable) state.
    Ocramius committed Aug 1, 2022
    Configuration menu
    Copy the full SHA
    68978b9 View commit details
    Browse the repository at this point in the history

Commits on Aug 5, 2022

  1. Configuration menu
    Copy the full SHA
    dc7d26a View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    267d760 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    58ca4e0 View commit details
    Browse the repository at this point in the history
  4. Removed DateTimeImmutable::format() from the CallMap: fully covered…

    … by stub
    
    Note: some conditional return type magic was required here.
    
    See: vimeo#8350 (comment)
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    7ee12c7 View commit details
    Browse the repository at this point in the history
  5. Removed DateTimeImmutable::getTimezone() from the CallMap: fully co…

    …vered by stub
    
    Note: also verified that a `DateTimeImmutable#getTimezone()` always returns
    a default timezone (initialized internally), and therefore restricted the
    type a bit.
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    2b6fddf View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    1be04e0 View commit details
    Browse the repository at this point in the history
  7. Removed DateTimeImmutable::getTimestamp() from the CallMap: fully c…

    …overed by stub
    
    This also simplifies the return type from `int|false` to always `int`,
    since a timestamp can always be produced.
    
    Ref: https://github.com/php/php-src/blob/eff9aed1592f59cddb12d36a55dec0ccc3bbbfd6/ext/date/php_date.stub.php#L496-L500
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    002585b View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    18557b8 View commit details
    Browse the repository at this point in the history
  9. Removed DateTimeImmutable::modify() from the CallMap: fully covered…

    … by stub
    
    Also expanded the return type from `static` to `static|false`, since the
    operation can fail (with a warning too), such as in following example:
    
    https://3v4l.org/Xrjlc
    
    ```php
    <?php
    
    var_dump(
        (new DateTimeImmutable())
            ->modify('potato')
    );
    ```
    
    Produces
    
    ```
    Warning: DateTimeImmutable::modify(): Failed to parse time string (potato) at position 0 (p): The timezone could not be found in the database in /in/Xrjlc on line 6
    bool(false)
    ```
    
    Ref: https://github.com/php/php-src/blob/534127d3b22b193ffb9511c4447584f0d2bd4e24/ext/date/php_date.stub.php#L508-L509
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    7cd3d49 View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    cb9939c View commit details
    Browse the repository at this point in the history
  11. Removed DateTimeImmutable::setTimezone() from the CallMap: fully co…

    …vered by stub
    
    Also simplified the return type from `static|false` to `static`, since
    the method throws at all times, on failure.
    
    On PHP 7.x, it could only fail if an invalid type was passed in, which is
    not really valid anyway, from a type perspective.
    
    Ref (PHP 8.2.x): https://github.com/php/php-src/blob/534127d3b22b193ffb9511c4447584f0d2bd4e24/ext/date/php_date.c#L3291-L3307
    Ref (PHP 8.2.x): https://github.com/php/php-src/blob/534127d3b22b193ffb9511c4447584f0d2bd4e24/ext/date/php_date.stub.php#L517-L518
    Ref (PHP 7.0.33): https://github.com/php/php-src/blob/bf574c2b67a1f786e36cf679f41b758b973a82c4/ext/date/php_date.c#L3363-L3379
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    4fe554d View commit details
    Browse the repository at this point in the history
  12. Removed DateTimeImmutable::setTime() from the CallMap: fully covere…

    …d by stub
    
    Also simplified the return type from `static|false` to `static`, since
    the method throws at all times, on failure.
    
    On PHP 7.x, it could only fail if an invalid type was passed in, which is
    not really valid anyway, from a type perspective.
    
    Ref (PHP 8.1.x): https://github.com/php/php-src/blob/32d55f74229e7913db0d59ef874a401744479b6a/ext/date/php_date.c#L3212-L3228
    Ref (PHP 7.0.33): https://github.com/php/php-src/blob/bf574c2b67a1f786e36cf679f41b758b973a82c4/ext/date/php_date.c#L3447-L3463
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    e61c593 View commit details
    Browse the repository at this point in the history
  13. Removed DateTimeImmutable::setDate() from the CallMap: fully covere…

    …d by stub
    
    Also simplified the return type from `static|false` to `static`, since
    the method throws at all times, on failure.
    
    On PHP 7.x, it could only fail if an invalid type was passed in, which is
    not really valid anyway, from a type perspective.
    
    Ref (PHP 8.1.x): https://github.com/php/php-src/blob/32d55f74229e7913db0d59ef874a401744479b6a/ext/date/php_date.c#L3258-L3274
    Ref (PHP 7.0.33): https://github.com/php/php-src/blob/bf574c2b67a1f786e36cf679f41b758b973a82c4/ext/date/php_date.c#L3496-L3512
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    0a6c9d0 View commit details
    Browse the repository at this point in the history
  14. Removed DateTimeImmutable::setISODate() from the CallMap: fully cov…

    …ered by stub
    
    Also simplified the return type from `static|false` to `static`, since
    the method throws at all times, on failure.
    
    On PHP 7.x, it could only fail if an invalid type was passed in, which is
    not really valid anyway, from a type perspective.
    
    Ref (PHP 8.1.x): https://github.com/php/php-src/blob/32d55f74229e7913db0d59ef874a401744479b6a/ext/date/php_date.c#L3308-L3324
    Ref (PHP 7.0.33): https://github.com/php/php-src/blob/bf574c2b67a1f786e36cf679f41b758b973a82c4/ext/date/php_date.c#L3549-L3565
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    964f64a View commit details
    Browse the repository at this point in the history
  15. Removed DateTimeImmutable::setTimestamp() from the CallMap: fully c…

    …overed by stub
    
    Also simplified the return type from `static|false` to `static`, since
    the method throws at all times, on failure.
    
    On PHP 7.x, it could only fail if an invalid type was passed in, which is
    not really valid anyway, from a type perspective.
    
    Ref (PHP 8.1.x): https://github.com/php/php-src/blob/32d55f74229e7913db0d59ef874a401744479b6a/ext/date/php_date.c#L3353-L3369
    Ref (PHP 7.0.33): https://github.com/php/php-src/blob/bf574c2b67a1f786e36cf679f41b758b973a82c4/ext/date/php_date.c#L3596-L3612
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    aaac9cc View commit details
    Browse the repository at this point in the history
  16. Configuration menu
    Copy the full SHA
    a1ed84f View commit details
    Browse the repository at this point in the history
  17. Simplified DateTimeImmutable::format(): always returns a string

    Also:
    
     * a non-empty format string will always lead to `non-empty-string`
     * it seems that you can throw **everything** at `DateTimeInterface#format()`, even null bytes,
       and it will always produce a `string`
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    68ffae0 View commit details
    Browse the repository at this point in the history
  18. Removed DateTimeImmutable::createFromInterface() from stubs

    While there is value in declaring `DateTimeImmutable::createFromInterface()` as mutation-free in
    a stub, this method was introduced in PHP 8.0, so we cannot really declare it in a stub.
    
    For now, we drop it, as the value of its stub declaration is much lower than the problems it
    introduces through its conditional existence.
    Ocramius committed Aug 5, 2022
    Configuration menu
    Copy the full SHA
    1382877 View commit details
    Browse the repository at this point in the history