More precise types after Option::filter()

[devnix]

I've been digging into trying to refine the type inside of Option<T> as I made a heavy use of it. Right now I'm writing a lot of code that looks like this:

$data
    ->filter(non_empty_string()->matches(...))
    ->map(non_empty_string()->assert(...))

that I think could be easily avoided.

As I've been able to write a handful of Rector rules, I decided to try to make an implementation, but I have to confess that I ✨vibed✨ it a little bit. I'm trying to fully understand each step, but some details are escaping my current knowledge.

Right now it seems to work, except for cases like \Psl\Type\non_empty_list()->matches(...). I think that there should be a generic way of handling those and not hardcoding the TypeInterface, but I'm not getting it to work.

Do you think it could be a nice-to-have feature, and how it's done? I will happily receive any kind of feedback so I can become less dependent on LLMs to write PHPStan rules. I also would love to implement this feature on VectorInterface, MapInterface, etc.

[ondrejmirtes]

Hello, the first step you should do is to run the build on your local machine before you create a PR. vendor/bin/phpstan and vendor/bin/phpunit and vendor/bin/phpcs give you todos that you should solve first.

[ondrejmirtes]

Of course I'm here to help you get your contribution over the finish line but please respect my time and try to learn the substance behind the code. You can start here https://phpstan.org/developing-extensions/core-concepts

[azjezz]

I don't exactly understand what this new extension does..

Option in psl is a class, so you can't determine if its a Some or None in the type system, and the callable passed to filter is supposed to return a bool, when false, it will turn a Some into a None.

Even if phpstan knows for a fact that false or true literal is being returned, there is 0 added value, as the type would still be Option ( as mentioned, there is no sub-types Some/None ).

@devnix could you please explain what the goal is here exactly?

[azjezz]

Is the goal for example to turn Option<string> into Option<non-empty-string> when the callable passed to filter returns true only for non-empty-string?

[devnix]

Sorry, @ondrejmirtes, I just wanted to gauge interest in finishing a feature like this. Maybe I should have created a draft instead of a PR, or as you stated, invest some more time into cleaning all the CI. Until I figure out how to make those non_empty_string()->matches(...) work, that's the only tests that are going to fail. Sorry if I wasted anyone's time! 🙏🏼

Hi @azjezz!

Is the goal for example to turn Option<string> into Option<non-empty-string> when the callable passed to filter returns true only for non-empty-string?

Yes, that's the whole purpose, I guess it can also be applied to other filtrable monads 😊

[azjezz]

I would suggest then marking this PR as a draft, looking how something like this is implemented already for array_filter https://phpstan.org/r/6358ce01-177c-45c5-b381-7f76cfb03ad6, and try to replicate that.

[ondrejmirtes]

You don't need to copy and paste the array_filter implementation from phpstan-src, you can create "fake AST" that will just call the logic, like:

$scope->getType(new FuncCall('array_filter', ...)...

[devnix]

Now I see that there's a ArrayFilterFunctionReturnTypeHelper, I will try something this weekend!

[ondrejmirtes]

That's not part of BC promise.

[devnix]

Thank you for your comments! It took me a while to figure out how to make it work, and I learned a lot through the process, discovering TypeExpr has been my biggest blocker.

It's indeed a much simpler and smarter approach, I'm really really grateful.

I think CI should be all green now, except for the tests. I've still left two cases to ask you for some last feedback: it looks like it's not going to infer a type to $option->filter(\Psl\Type\literal_string('potato')->matches(...)) and $option->filter(\Psl\Type\non_empty_list()->matches(...)).

Maybe it would be related to #11 and #16? Should I remove those failing tests for now?

@azjezz, if the implementation is correct, I'm probably extracting it to a helper and applying it to more implementations for other classes that have similar methods. What are your thoughts about azjezz/psl#535? That would greatly simplify the extension, if it makes sense for the general use cases of the library 🙂

[devnix]

Comment edited, after taking a third closer look, I found that I was using a couple of non-existent methods that probably were hallucinated at my first attempt, sorry for the noise there, tests should be green now.

[devnix]

Fixed (I hope) workflows for lower versions of PHP and PSL

[devnix]

Sorry, the support for older PHP and PSL versions is biting me. Additionally, now that I'm aware of the widespread support the plugin has, introducing an interface would not simplify anything, as implementing a filter extension for each class would be necessary anyway.

I'm going to close this pull request and enable Github actions on my branch, and I will create a pull request when I've got everything green. Sorry for the extra noise.