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

Contextual type inference for high order function arg #7417

Merged
merged 13 commits into from
Jan 20, 2022
Merged

Contextual type inference for high order function arg #7417

merged 13 commits into from
Jan 20, 2022

Conversation

klimick
Copy link
Contributor

@klimick klimick commented Jan 18, 2022

Since php doesn't have extension method functionality like C# then pipe function can be a good solution to the situation:

<?php

$greaterThan100 = pipe2(
    [1, 2, 3],
    map(function ($i) {
        return $i > 1 ? $i + 1 : $i + 2;
    }),
    filter(function ($i) {
        return $i > 100;
    })
);

Before we can write generic pipe function we should support inference for partially applied closures (fn($a) => fn($b) => $a + $b)

This PR tries to implement inference of this kind.
P.S. I tried to do it through the plugin system. It didn't work out.

@orklah
Copy link
Collaborator

orklah commented Jan 18, 2022

Can you add a snippet in test to show the easiest possible code that is solved by this PR? Also could you add some comments in the code describing what you're doing. It will help for the review and also makes easier to read this in the future

@klimick klimick changed the title Infer partially applied closure arg by previous function arg Contextual templates inference for high order func arg Jan 18, 2022
@klimick klimick changed the title Contextual templates inference for high order func arg Contextual type inference for high order function arg Jan 18, 2022
@klimick
Copy link
Contributor Author

klimick commented Jan 18, 2022

Done. If there is anything else that needs to be added, please let me know)

@orklah
Copy link
Collaborator

orklah commented Jan 19, 2022

Sorry, I should have warned before but could you target this on master? We'll start avoiding to land big new features on 4.x to prevent maintenance on old versions once Psalm 5 is out

@orklah orklah added the release:feature The PR will be included in 'Features' section of the release notes label Jan 19, 2022
@klimick klimick changed the base branch from 4.x to master January 20, 2022 00:42
@orklah orklah merged commit 6f1a5e8 into vimeo:master Jan 20, 2022
@orklah
Copy link
Collaborator

orklah commented Jan 20, 2022

Thanks! @drupol @veewee I think this could interest you

@drupol
Copy link
Contributor

drupol commented Jan 20, 2022

OMG !!! I don't have much time these days, but I'm planning to review that and the work of @veewee !

@drupol
Copy link
Contributor

drupol commented Jan 20, 2022

I think @Ocramius, @Crell and @AlexandruGG might be interested as well !

@Crell
Copy link

Crell commented Jan 20, 2022

Oh my.

I don't know the Psalm codebase well enough, but the generic pipe function already exists: https://github.com/Crell/fp

Includes a whole bunch of related utilities as well. It's currently very well-audited by PHPStan, but I haven't tried Psalm on it. (PRs welcome for that.)

@veewee
Copy link
Contributor

veewee commented Jan 21, 2022

Hello @klimick

Nice work on this PR! Will check out the details soon.

Some context from my side:

I am also trying to build a variadic pipe plugin as well and currently ended up with this:

It works quite well, but only works for callable arguments.

Now I am trying to use the NodeTypeProvider in order to get the best type information directly from psalm when calculating the pipe function's attributes type. This is currently not possible yet, since the types of the arguments are not known yet during FunctionReturnTypeProviderInterface.

See:

I guess you probably also need the type information from the provided variadic pipeline stages to build your plugin.
This might be related to your newly reported issue #7450 as well.

This way, you could do something like this inside the code from the pipe plugin mentioned above:

    private static function parseStages(StatementsSource $source, array $args): array
    {
        $stages = [];
        $nodeTypeProvider = $source->getNodeTypeProvider();

        foreach ($args as $arg) {
            $stage = $arg->value;


            $stageType = $nodeTypeProvider->getType($stage);
            $stageClosureTypes = $stageType ? $stageType->getClosureTypes() :  [];

And base the calculations based on the first $stageClosureTypes

Care to take a look at it? :)

klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 15, 2023
klimick added a commit to klimick/psalm that referenced this pull request Aug 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature The PR will be included in 'Features' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants