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

Generic's sub-type is lost when passed through a callable #7281

Closed
gnutix opened this issue May 21, 2022 · 7 comments
Closed

Generic's sub-type is lost when passed through a callable #7281

gnutix opened this issue May 21, 2022 · 7 comments
Labels

Comments

@gnutix
Copy link

gnutix commented May 21, 2022

Bug report

Hello there. While adding some types and types' tests for a bunch of Functional-related functions in our codebase, I noticed a loss of precision in the return type of operations like map, reduce and collect, when a Generic with a sub-type is passed through a callable.

I've collected a few playgrounds over the last few weeks, sorry if some of them overlap.

(I don't currently have the time to work on an implementation, but maybe someone will.)

Code snippet that reproduces the problem

Expected output

The expected types should be the ones returned by PHPStan.

Did PHPStan help you today? Did it make you happy in any way?

(I have a bunch of small issues to create, sorry if I don't come up with something new for each of them 🙏 )

@rvanvelzen
Copy link
Contributor

Your callables are only defined to return Timeline, that's why they don't get more specific.

array_map() has some special handling which makes it understand what happens in the callable slightly better.

@gnutix
Copy link
Author

gnutix commented May 30, 2022

If by "special handling", you mean a custom DynamicReturnTypeExtension, then I'm confused, because I've created one too for our map() wrapper which extends PHPStan's (cf. code below), which I could not include in the playground. So I should have exactly the same results as array_map, hypothetically. Or is it somewhere else, hardcoded in PHPStan's internal code ? 🤔

<?php

declare(strict_types=1);

namespace Gammadia\Collections\PhpStan\Functional\Extensions;

use PhpParser\Node\Expr\FuncCall;
use PHPStan\Analyser\Scope;
use PHPStan\Reflection\FunctionReflection;
use PHPStan\Type\Php\ArrayMapFunctionReturnTypeExtension;
use PHPStan\Type\Type;
use function Gammadia\Collections\Functional\reverse;

final class FunctionalMapDynamicReturnTypeExtension extends ArrayMapFunctionReturnTypeExtension
{
    public function isFunctionSupported(FunctionReflection $functionReflection): bool
    {
        return 'gammadia\collections\functional\map' === strtolower($functionReflection->getName());
    }

    /**
     * Our implementation as reversed arguments (array first, callable second) compared to array_map
     */
    public function getTypeFromFunctionCall(FunctionReflection $functionReflection, FuncCall $functionCall, Scope $scope): Type
    {
        $reversedFunctionCall = new FuncCall($functionCall->name, reverse($functionCall->getArgs()));

        return parent::getTypeFromFunctionCall($functionReflection, $reversedFunctionCall, $scope);
    }
}

@rvanvelzen
Copy link
Contributor

Unfortunately I'm not talking about an extension, but internal handling: https://github.com/phpstan/phpstan-src/blob/75c5574a402e858458bf0f83a5942a22e6cfb737/src/Analyser/MutatingScope.php#L1235

@gnutix
Copy link
Author

gnutix commented Aug 7, 2022

@rvanvelzen would it be feasible to make that "internal handling" pluggable ? Like being able to define a list of functions (and/or methods?) acting like array_map ?

@rvanvelzen
Copy link
Contributor

@gnutix fairly unlikely, but this case will be solved when generalization of template types goes away (see phpstan/phpstan-src#1206 for some info on that)

@gnutix
Copy link
Author

gnutix commented Sep 28, 2023

@rvanvelzen Out of curiosity, is this "generalization of template types going away" thing still a topic ? (just seen the PR hasn't been touched in a year)

@phpstan-bot
Copy link
Contributor

@gnutix After the latest push in 1.11.x, PHPStan now reports different result with your code snippet:

@@ @@
-PHP 7.4 – 8.1 (4 errors)
+PHP 7.4 – 8.1 (2 errors)
 ==========
 
 34: Expected type array<int, Timeline<Percentage>>, actual: array<int, Timeline>
-40: Expected type array<int, Timeline<Percentage>>, actual: array<int, mixed>
 47: Expected type array<int, Timeline<Percentage>>, actual: array<int, Timeline>
-51: Expected type array<int, Timeline<Percentage>>, actual: array<int, mixed>
 
 PHP 7.1 – 7.3 (8 errors)
 ==========
Full report

PHP 7.4 – 8.1 (2 errors)

Line Error
34 Expected type array<int, Timeline<Percentage>>, actual: array<int, Timeline>
47 Expected type array<int, Timeline<Percentage>>, actual: array<int, Timeline>

PHP 7.1 – 7.3 (8 errors)

Line Error
49 Syntax error, unexpected T_STRING, expecting T_PAAMAYIM_NEKUDOTAYIM on line 49
49 Syntax error, unexpected T_VARIABLE, expecting ')' on line 49
53 Syntax error, unexpected T_DOUBLE_ARROW on line 53
53 Syntax error, unexpected T_STRING, expecting T_PAAMAYIM_NEKUDOTAYIM on line 53
74 Syntax error, unexpected T_STRING, expecting T_PAAMAYIM_NEKUDOTAYIM on line 74
74 Syntax error, unexpected T_VARIABLE, expecting ')' on line 74
78 Syntax error, unexpected T_DOUBLE_ARROW on line 78
78 Syntax error, unexpected T_STRING, expecting T_PAAMAYIM_NEKUDOTAYIM on line 78

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

No branches or pull requests

4 participants