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

Pipe operator, New AST node version #7214

Closed
wants to merge 9 commits into from
Closed

Conversation

Crell
Copy link
Contributor

@Crell Crell commented Jul 3, 2021

An improved version of the pipe operator, this time using a new AST node in order to ensure left to right evaluation.

Replaces #5425.

RFC: https://wiki.php.net/rfc/pipe-operator-v2

@nikic nikic added the RFC label Jul 5, 2021
@Crell Crell force-pushed the pipe-operator-ast branch from c9ea8a4 to 5515ddf Compare July 6, 2021 16:59
@emtudo
Copy link

emtudo commented Jul 9, 2021

The best thing about Elixir is the pipe operator, I honestly don't understand why people aren't approving this RFC.

@jdreesen
Copy link
Contributor

jdreesen commented Jul 9, 2021

Especially given how small the change in this PR is.

@kolardavid
Copy link

If I might express my opinion, I get why this is not accepted and I am kind of happy it is not. There is imho big flaw in implementation. PHP is getting much better in recent versions by more strong control and type safety and this seems like the opposite. The main issue for me is, that pipe operator sends argument to callable, which should be imho eliminated as most as possible in whole PHP and replaced with Closure. Also this makes static analysis for IDEs more complicated. $x |> 'strlen' simply does not hint function call. I find it ugly and prone to some ugly code, which breaks static analysis like this $x |> $y . $z.

However, since First-class callable syntax was accepted, I think, this RFC would make much more sense if the author adapted it to this and pipe operator would require callable as argument. The above example then should look like $x |> strlen(...) which imho better implies it is function call and restricts some ugly usage. Even more sense would be probably this in conjunction with Partial Function Application, since it would be possible to add other arguments into the chain, but this RFC was declined.

@Crell
Copy link
Contributor Author

Crell commented Jul 13, 2021

@kolardavid I'm afraid your post doesn't make any sense to me.

I know Nikita hates callable and wants to make everyone use formal Closures, but that's not PHP today. callable will be around for at least the next decade if not longer. You and he are the only people I know that feel strongly on that front, and I don't entirely grok why.

In any case, by design, this RFC is impartial on that front. It works with anything that PHP can treat as "a thing you can pass an argument to." That PHP 8.0's options there are crap is no fault of this RFC; it's a separate issue. The PFA RFC was intended specifically (in my mind) to solve that exact issue. It sadly failed. The FCC RFC partially solves it (although as you note, not as well as PFA would have). So the problem you describe is 1) Not a flaw in this RFC and 2) Is already getting better; in fact, the RFC text itself in its current form was written on the assumption that FCC would pass; it even says exactly that in the RFC text.

So... I really don't know what it is you're objecting to. Admittedly, you're not alone in that. A lot of people have objected to this RFC on the grounds that PHP's callable syntax is terrible, yet the main effort to fix that problem was rejected. Don't blame pipes for that. 😄

@kolardavid
Copy link

kolardavid commented Jul 13, 2021

@Crell Well, I am completely OK with the rest of the RFC, except the callable part. I would be all in if pipe operator would require Closure, which can be created by FCC or possibly moderned future PFA. Because once you implement this, you will not be able to restrict it later without BC. I just don't feel it is good to give new purpose to callable. I know PHP will not leave callables soon or never, but I would definitely vote against using it in new features, especially new operator. Not sure what issue has Nikita with callables, but I find them ugly and nightmare for safe static analysis.

@Crell
Copy link
Contributor Author

Crell commented Jul 14, 2021

Absent a concerted long-term plan to actually work to deprecate callable entirely, I think it's premature and inappropriate to start doing so unilaterally in this RFC, or any similar RFC.

And any such long-term plan needs to be discussed on the list, and possibly voted on as a roadmap. As this RFC is definitely not passing this round anyway, I'd frankly only be up for modifying it in a v3 to only take closures iff there was such a plan explicitly in place. It would be inappropriate for this RFC (or any other) to stealth-begin-deprecating callables.

If such a roadmap plan were actually adopted, then yes, it would make sense to implement that restriction for pipes now that FCC takes care of at least a large chunk of the use cases.

@HallofFamer
Copy link

HallofFamer commented Jul 16, 2021

Tbh if 90% of the use cases for Pipe Operator is for str_ and array_ functions, then this feature is completely unnecessary. It feels like a lesser solution to a problem that can be better fixed with the introduction of Scalar Objects in future. The object -> syntax is also better and cleaner than any syntax choice you can come up with for pipe operator.

Of course I dont mind if it is accepted into PHP, I just cant see myself using it, nor do I see more than 1% of developers finding this useful. Note pipe operator usually exists in pure FP languages that dont have objects and cannot use the dot(or arrow) notation to chain methods, PHP doesnt have this problem.

@buismaarten
Copy link

You could use the boostphp/pipe-operator package instead.

@Crell
Copy link
Contributor Author

Crell commented Jul 18, 2021

@HallofFamer Certainly many of the use cases are for those, but the RFC opens with an example that is neither.

Scalar methods work if and only if the method you want to use is one that was pre-blessed as a method. If not, you're SOL. Pipes allow any type-compatible function at all to be used, anywhere. There's simply no comparison in terms of the flexibility it allows.

@krakjoe
Copy link
Member

krakjoe commented Jul 20, 2021

Closing, declined.

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

Successfully merging this pull request may close these issues.

10 participants