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

Fails to infer return type for method using static callable #6005

Closed
leflings opened this issue Jun 28, 2021 · 3 comments · Fixed by #6019
Closed

Fails to infer return type for method using static callable #6005

leflings opened this issue Jun 28, 2021 · 3 comments · Fixed by #6019

Comments

@leflings
Copy link

https://psalm.dev/r/2791d5c034

I would expect return type of combineAll1 to be inferable from the context.

Oddly enough, psalm has no issue with the "callable"-form used in combineAll2. (I wonder if this is because Psalm just "gives up" with this form or if it actually understands it?)

I prefer (and believe is php standard?) to supply the callable in the first form [class, method] as this is understood by PhpStorm when finding usages and navigating to implementation.

Possible duplicate of #5388 though from my (mostly inexperienced) perspective they seem to be two slightly different cases.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/2791d5c034
<?php

declare(strict_types=1);

final class DaysInterval
{
    private ?int $min;

    private ?int $max;

    public function __construct(?int $min, ?int $max)
    {
        $this->min = $min;
        $this->max = $max;
    }

    public function getMin(): ?int
    {
        return $this->min;
    }

    public function getMax(): ?int
    {
        return $this->max;
    }

    public static function combineAll1(self ...$intervals): self
    {
        return array_reduce($intervals, [self::class, 'combine'], new self(0, 0));
    }
    
    public static function combineAll2(self ...$intervals): self
    {
        return array_reduce($intervals, self::class . '::combine', new self(0, 0));
    }

    public static function combine(self $a, self $b): self
    {
        $newMin = max($a->getMin() ?? \PHP_INT_MAX, $b->getMin() ?? \PHP_INT_MAX);
        $newMax = max($a->getMax() ?? \PHP_INT_MAX, $b->getMax() ?? \PHP_INT_MAX);

        if ($newMin > $newMax) {
            $newMin = $newMax;
        }

        if ($newMin === \PHP_INT_MAX) {
            $newMin = null;
        }
        if ($newMax === \PHP_INT_MAX) {
            $newMax = null;
        }

        return new self($newMin, $newMax);
    }
}
Psalm output (using commit 8c13752):

INFO: MixedReturnStatement - 29:16 - Could not infer a return type

INFO: MixedInferredReturnType - 27:61 - Could not verify return type 'DaysInterval' for DaysInterval::combineAll1

@weirdan
Copy link
Collaborator

weirdan commented Jun 28, 2021

Simplified a bit: https://psalm.dev/r/4d5778d2aa

I wonder if this is because Psalm just "gives up" with this form or if it actually understands it?

Psalm understands that, see the trace produced by the linked snippet.

Possible duplicate of #5388 though from my (mostly inexperienced) perspective they seem to be two slightly different cases.

They are likely separate, because Psalm has custom code to infer array_reduce return type.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/4d5778d2aa
<?php

final class DaysInterval {
    public function __construct() {}

    public static function combineAll1(self ...$intervals): self {
        return array_reduce($intervals, [self::class, 'combine'], new self());
    }
    
    public static function combineAll2(self ...$intervals): self {
        $ret = array_reduce($intervals, self::class . '::combine', new self());
        /** @psalm-trace $ret */;
        return $ret;
    }

    public static function combine(self $_a, self $_b): self {
        return new self();
    }
}
Psalm output (using commit 9e9413d):

INFO: MixedReturnStatement - 7:16 - Could not infer a return type

INFO: MixedInferredReturnType - 6:61 - Could not verify return type 'DaysInterval' for DaysInterval::combineAll1

INFO: Trace - 12:33 - $ret: DaysInterval

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants