vimeo / psalm

A static analysis tool for finding errors in PHP applications
https://psalm.dev
MIT License
5.55k stars 662 forks source link

MoreSpecificReturnType and LessSpecificReturnStatement Error #10841

Open TheDevick opened 6 months ago

TheDevick commented 6 months ago

I encountered an issue with PSALM's, related to the return types of methods in my classes. PSALM is flagging errors regarding the specificity of return types, despite the code being logically correct.

<?php

class Decimal implements \Stringable
{
    /**
     * @param numeric-string $value
     */
    final private function __construct(
        private string $value
    ) {
    }

    public static function create(string $value): static
    {
        if (!is_numeric($value)) {
            throw new \Exception(sprintf('The decimal value "%s" is non-numeric', $value));
        }

        return new static($value);
    }

    public function add(Decimal $value, ?int $scale = null): static
    {
        return static::create(bcadd($this, $value, $scale));
    }

    public function subtract(Decimal $value, ?int $scale = null): static
    {
        return static::create(bcsub($this, $value, $scale));
    }

    public function multiply(Decimal $value, ?int $scale = null): static
    {
        return static::create(bcmul($this, $value, $scale));
    }

    public function divide(Decimal $value, ?int $scale = null): static
    {
        return static::create(bcdiv($this, $value, $scale));
    }

    /**
     * @return numeric-string
     */
    public function __toString(): string
    {
        return $this->value;
    }
}

class Kilowatt extends Decimal
{
    public static function createFromWatt(Watt $watt, ?int $scale = null): self
    {
        $thousand = Decimal::create('1000');

        return self::create($watt->divide($thousand, $scale));
    }
}

class Watt extends Decimal
{
    public static function createFromKilowatt(Kilowatt $kilowatt, ?int $scale = null): self
    {
        $thousand = Decimal::create('1000');

        return self::create($kilowatt->multiply($thousand, $scale));
    }
}

Errors:

ERROR: MoreSpecificReturnType - src/Shared/Power/Domain/ValueObject/Kilowatt.php:9:76 - The declared return type 'Shared\Power\Domain\ValueObject\Kilowatt' for Shared\Power\Domain\ValueObject\Kilowatt::createFromWatt is more specific than the inferred return type 'Shared\Number\Domain\ValueObject\Decimal&static' (see https://psalm.dev/070)
    public static function createFromWatt(Watt $watt, ?int $scale = null): self

ERROR: LessSpecificReturnStatement - src/Shared/Power/Domain/ValueObject/Kilowatt.php:13:16 - The type 'Shared\Number\Domain\ValueObject\Decimal&static' is more general than the declared return type 'Shared\Power\Domain\ValueObject\Kilowatt' for Shared\Power\Domain\ValueObject\Kilowatt::createFromWatt (see https://psalm.dev/129)
        return self::create($watt->divide($thousand, $scale));
psalm-github-bot[bot] commented 6 months ago

Hey @TheDevick, can you reproduce the issue on https://psalm.dev? These will be used as phpunit tests when implementing the feature or fixing this bug.

TheDevick commented 6 months ago

Hey @TheDevick, can you reproduce the issue on https://psalm.dev? These will be used as phpunit tests when implementing the feature or fixing this bug.

https://psalm.dev/r/1486877b8b

psalm-github-bot[bot] commented 6 months ago

I found these snippets:

https://psalm.dev/r/1486877b8b ```php value; } } class Kilowatt extends Decimal { public static function createFromWatt(Watt $watt, ?int $scale = null): self { $thousand = Decimal::create('1000'); return self::create($watt->divide($thousand, $scale)); } } class Watt extends Decimal { public static function createFromKilowatt(Kilowatt $kilowatt, ?int $scale = null): self { $thousand = Decimal::create('1000'); return self::create($kilowatt->multiply($thousand, $scale)); } } ``` ``` Psalm output (using commit ef3b018): ERROR: ImplicitToStringCast - 24:37 - Argument 1 of bcadd expects numeric-string, but Decimal&static provided with a __toString method ERROR: ImplicitToStringCast - 24:44 - Argument 2 of bcadd expects numeric-string, but Decimal provided with a __toString method ERROR: ImplicitToStringCast - 29:37 - Argument 1 of bcsub expects numeric-string, but Decimal&static provided with a __toString method ERROR: ImplicitToStringCast - 29:44 - Argument 2 of bcsub expects numeric-string, but Decimal provided with a __toString method ERROR: ImplicitToStringCast - 34:37 - Argument 1 of bcmul expects numeric-string, but Decimal&static provided with a __toString method ERROR: ImplicitToStringCast - 34:44 - Argument 2 of bcmul expects numeric-string, but Decimal provided with a __toString method ERROR: ImplicitToStringCast - 39:37 - Argument 1 of bcdiv expects numeric-string, but Decimal&static provided with a __toString method ERROR: ImplicitToStringCast - 39:44 - Argument 2 of bcdiv expects numeric-string, but Decimal provided with a __toString method ERROR: ImplicitToStringCast - 57:29 - Argument 1 of Kilowatt::create expects string, but Decimal&Watt provided with a __toString method INFO: LessSpecificReturnStatement - 57:16 - The type 'Decimal&static' is more general than the declared return type 'Kilowatt' for Kilowatt::createFromWatt INFO: MoreSpecificReturnType - 53:76 - The declared return type 'Kilowatt' for Kilowatt::createFromWatt is more specific than the inferred return type 'Decimal&static' ERROR: ImplicitToStringCast - 67:29 - Argument 1 of Watt::create expects string, but Decimal&Kilowatt provided with a __toString method INFO: LessSpecificReturnStatement - 67:16 - The type 'Decimal&static' is more general than the declared return type 'Watt' for Watt::createFromKilowatt INFO: MoreSpecificReturnType - 63:88 - The declared return type 'Watt' for Watt::createFromKilowatt is more specific than the inferred return type 'Decimal&static' ```
TheDevick commented 6 months ago

I don't know why it's giving so much errors. Maybe because of the bc extension. But in my project, it just shows the errors I mentioned

TheDevick commented 6 months ago

Hey! Just wanted to metion that I did some tests and concluded that:

$watt = Watt::create('1.0') // This IS returning an Watt object, instead of the referred psalm return type Decimal&static;
TheDevick commented 6 months ago

Founded a solution. Add the following @return annotation to the Decimal::create() static method:


    /**
     * @return static
     */
    public static function create(string $value): static
    {
        if (!is_numeric($value)) {
            throw new InvalidDecimalValueException(sprintf('The decimal value "%s" is non-numeric', $value));
        }

        return new static($value);
    }

Is it a bug? Because I'm already telling the PHP itself that the method returns static!

chriskapp commented 3 months ago

@TheDevick thanks for the workaround, adding @return static fixes also my problem, it looks indeed like the phpdoc should be not needed since we have already the static return type.