Carbon: [PHPSTAN] Expects Illuminate\Support\Carbon, Carbon\CarbonInterface given

[mfn]

Hi!

I first filed a bug report with phpstan but was educated otherwise 😅 => see https://github.com/phpstan/phpstan/issues/2197 for reference

After upgrading to Laravel 5.8, phpstan "broke" for due to Carbon 2.

I encountered an issue with the following code (I omitted the namespaces):
https://phpstan.org/r/545a4abe-93b0-4074-b34a-465cc1f074c2

<?php declare(strict_types = 1);

// library
interface CarbonInterface {
    /**
     * @return static|CarbonInterface
     */
    public static function now($tz = null);
}

trait Date {
    /**
     * @return static|CarbonInterface
     */
    public static function now($tz = null)
    {
        return new static(null, $tz);
    }
}

class Carbon extends DateTime implements CarbonInterface
{
    use Date;
}

class IlluminateSupportCarbon extends Carbon {
}

// app code 

class Foo {
    public function bar(IlluminateSupportCarbon $date): void {
    }
}

$foo = new Foo();
$foo->bar(IlluminateSupportCarbon::now());

| Line | test.php                                                                                        |
+--------------------------------------------------------------------------------------------------------+
| 37   | Parameter #1 $date of method Foo::bar() expects IlluminateSupportCarbon, CarbonInterface given. |
+--------------------------------------------------------------------------------------------------------+

Carbon version: 2.19.1

PHP version: 7.2.17

I expected to get:

No error from phpstan.

But I actually get:

Parameter #1 $date of method …::foo() expects Illuminate\Support\Carbon, Carbon\CarbonInterface given.

See the phpstan developers response https://github.com/phpstan/phpstan/issues/2197#issuecomment-500153556

All types are compared to each other and find out if some are supertypes of others. Let's say you have mixed|int. It does not make sense to leave the int part in - the type means "this can be anything, or it can be int". The "int" part is already included in the "anything" part.

static|CarbonInterface in CarbonInterface is the same situation. It says "it will return the type which it's called on, or it will return CarbonInterface". If there's still a risk of returning "CarbonInterface", the static part does not add any information.

Thus the question: can we drop the CarbonInterface part on the typehint?

If not, what other solutions would be possible?

To be clear: this is not related to the type declaration for Illuminate\Support\Carbon => using just \Carbon\Carbon is the same.

Maybe I should switch to CarbonInterface all over in my code? But then why would there be a Illuminate\Support\Carbon class at all?

thanks,

• Markus

[kylekatarnls]

Hi @mfn, PHPDoc is here in priority for the IDE completion, and even if the static|CarbonInterface is not ideal in CarbonInterface itself (it's here because the CarbonInterface PHPDoc is auto-generated from traits). We could probably be more precise checking for each method if we're sure to get static only or not.

But currently, replacing all static|CarbonInterface with CarbonInterface would break some nice auto-completion from IDEs, and replacing all static|CarbonInterface with static would just be wrong, as for some of the methods documented this way, this is not guaranteed, all static method of Carbon does not return Carbon instances 100% of the time. Some settings allow you for some method to return instances of other classes that implements CarbonInterface but does not extend Carbon.

[kylekatarnls]

static|CarbonInterface will disappear from many methods in 2.20.

@mfn can you try to require "nesbot/carbon": "dev-master" and see if it's all fine for you now (phpstan and IDE auto-completion)?

[mfn]

Works like a charm, many thanks!

FTR, I had to use this workaround: "nesbot/carbon": "dev-master as 2.19.3"

Otherwise it wouldn't install because the Laravel dependency wouldn't allow it.

[pablorsk]

Today we have a working release:

"nesbot/carbon": "^2.25"

thnks @kylekatarnls and @mfn