add Conditional types for psalm

[vv12131415]

Some functions can't be analyzed by psalm normally because they return union types

examples are 'preg_replace', 'preg_replace_callback',

what you can do add annotations like this

/**
 * previous info
 *
 * @template TSubject as string|string[]
 * @psalm-param TSubject $subject
 * @psalm-return (TSubject is string ? string : string[])
 */
function preg_replace($pattern, $replacement, $subject, int $limit = -1, int &$count = null)
{
    error_clear_last();
    $result = \preg_replace($pattern, $replacement, $subject, $limit, $count);
    if (preg_last_error() !== PREG_NO_ERROR || $result === null) {
        throw PcreException::createFromPhpError();
    }
    return $result;
}

I'm not sure if it works, but the idea is borrowed from https://psalm.dev/docs/annotating_code/type_syntax/conditional_types/

[Kharhamel]

Yes we already thoughtabout something like this in #185 but I didn't manage to do it. The information is just too much spread in too many places. I may look again in cases it has changed.

[orklah]

What could be the next step for anyone wanting to work on Psalm's integration?

If there's grunt work to be done, maybe I or anyone else could help?

[Kharhamel]

Well that is the problem, I don't see any grunt work. What we need for that feature would be functions that take a method name as a parameter and return the correct psalm annotation as a result. Something like getPsalmUnionType($functionName) for example.

The problem is, last time I checked, such thing doesn't exist in psalm source. Either the info comes from separate places, or it ask a complicated object as a parameter than I don't know how to construct.

I supposed you could browse psalm code and see if you can find something that I could easily integrate into safe.

[vv12131415]

@muglug can you please suggest a way to do it? Maybe this lib can refer to original function, but without union false type.

[orklah]

Well actually, I have a much better grasp of Psalm's internal now, but I have a lot less time than before 😄

This is probably not the absolute best way to do it, but it's the best I could find:

• We require psalm and make it run on an empty file
• It will go through its callmap and stubs and load every function
• We can create a psalm plugin on the AfterFunctionLikeAnalysisEvent hook that will receive the result of every function loaded
• In this plugin, we receive a FunctionStorage (with $event->getClasslikeStorage()) that contains a property return_type who is a Union.
• A Union is a group of one or more Atomic. An Atomic can be as simple as TString to describe a string or as complex as TConditional to describe a conditional return
• The Union could be disassembled into it's parts, stripped of false and then put back together to generate the annotation as a string (though Union::getKey())

If someone want to start on that, I could provide some guidance. Although maybe @muglug has a better idea than going through a plugin...

[orklah]

It should be even simpler than what I said above. If we plug in the AfterAnalysisInterface hook, we get a $codebase param on which we can do: $codebase->functions->getAllStubbedFunctions()[$function_id]

with $function_id being the FQN of the function. It returns the FunctionStorage on which we can get the return_type

EDIT: the above will actually retrieve all stubbed functions. Note that there is also the content of callMap files retrievable like that: InternalCallMapHandler::getCallablesFromCallMap($function_id)

[Jean85]

I've found this: https://github.com/hectorj/safe-php-psalm-plugin

Maybe an external package with the right stub is the way to go?

[vv12131415]

the way that @orklah described is better, because it will be always up to date. If we will have stubs, you will need to update all the places when psalm updates it's internal types

[Jean85]

Wouldn't be better to at least provide docblocks whenever a function allows it? Sorry, but I'm missing why we need something dynamic to have correct types with those functions... Or is this something related to changes in types between different PHP versions?

[vv12131415]

Or is this something related to changes in types between different PHP versions? no, only related to changes in psalm base types.

Basically, what @orklah is suggesting, is to get types from psalm it self, BUT exclude the |false part (since this is the purpose of the library).

For example, let's analyze base64_decode function. There are 2 ways to do this

1. external package way
2. @orklah way

The external package just copy/pastes code from psalm stubs dir (it's even said about this https://github.com/hectorj/safe-php-psalm-plugin/blob/b60ed45a06d5246e2efd193ce9c8940b244d5c7d/src/stubs/CoreGenericFunctions.phpstub#L4). Because this is copy, it will be only eventually consistent with real psalm stubs (one day you will update psalm, and it will describe internal functions in more precise way, BUT the external plugin will not update it's stubs). So as for our example base64_decode signature can be updated in psalm it's self, but it's not guaranteed that it will be updated in external package

If we go @orklah way we will have guarantees that we have updated signature when psalm is updated.

The algorithm is simple,

1. every time when psalm analyses function call, it does trigger AfterFunctionLikeAnalysisEvent.
2. We subscribe to this event
3. If we see that a function was from Safe namespace go further, no then exit
4. get psalm original function signature by our function (this package guarantees that we have only php native functions)
5. get the return type on the original function (in our example with base64_decode it's string|false) cc @Jean85
6. strip the |false part and leave only the first one string
7. return function information for Safe\base64_decode as it was base64_decode but without falsey return.

Hope it's better explains what I mean

[Jean85]

Ok I understand but it seems enormously complicated for no proper advantage: the behavior of those functions is fixed (apart from new language versions), so the expected types are known. The Psalm signature shouldn't change a lot, but could be very complicated, some of them have nested conditional types.

IMHO having this check with static tests in the CI here would be simpler and more than enough.

[vv12131415]

it does have changes at least every month https://github.com/vimeo/psalm/commits/master/stubs/CoreGenericFunctions.phpstub