-
Notifications
You must be signed in to change notification settings - Fork 26
7.1 Short closure improvements #65
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
Comments
Thanks for taking the time to weigh in on this, you make some good points so I want to address them each individually.
I disagree that there is an inconsistency here. It's true that other structures that do have closing syntax have that closing syntax placed on the next line, but this structure does not have closing syntax and we don't have an example anywhere else where we allow the semicolon to act as the closing syntax and live on its own line. There's probably some place for that but we don't have that today.
The example in your set that matches our situation the closest is Kotlin, and your example is missing most of the function: Int ->
print("acc = $acc, i = $i, ")
val result = acc + i
println("result = $result")
// The last expression in a lambda is considered the return value:
result This is indeed different from what we have proposed, but I'd argue that given the fact that even you struggled to identify the parts of this lambda that are relevant is evidence for statements being alone on their own lines with no block context surrounding them being confusing to read. That said I do agree that if PHP allowed for multiple statements in a short function that our recommendation would likely need to change and that's probably not great.
It's good to keep in mind what we're certain to have to undo in the future but I'd argue that given this functionality was added explicitly as a single statement and we already have other better constructs for multiline anonymous functions that it's highly unlikely that we ever see mutliline short functions added to the language. That all said, I'm not 100% sure where I sit on this yet. |
While I would like to see multiline auto-capture closures added in the future, that's about the only potential change I could see that would be relevant here. That would result in, potentially: $foo = fn(int $a, int $b): int
=> $a + $b;
$foo = fn(int $a, int $b): int => {
return $a + $b;
}; Which... I'm OK with. One is a genuinely multi-line function, the other is an over-engineered expression. 😄 |
Thanks for the answer! I understand that this is more of a fictional argument than a real one, but in most cases in the standard, the operator/statement remains on the previous line, and then comes the body to which it is applied, like: // 1.
implements
ExpectedInterface
// 2.
extends
ExpectedClass
// 3.
foo(
$argument,
// body (another argument)
// 4.
use Some\Any\{
ClassZ,
// body (another class)
declare(strict_types=1) {
// code
// 5.
use Trait {
// body
// etc... Curly braces wrapping in classes and methods is also consistent with this, as expressions/body go on a new line. I don't remember a single case in the PSR-2/12 when the statment was mixed with the expression (or body of thist statement). P.S. As for the rest of the remarks, I have nothing to add, you understood everything correctly and your argument is clear to me. |
On the other hand, I just looked at some of my code and in some places used arrow wrapping along with an expression, but only in "match" expressions, lol. This is handy for using trailing commas which are used in PER, like: match ($format) {
// ...
DXGIFormat::DXGI_FORMAT_BC6H_TYPELESS,
DXGIFormat::DXGI_FORMAT_BC6H_UF16,
DXGIFormat::DXGI_FORMAT_BC6H_SF16, // << trailing comma
=> PixelFormat::R8G8B8, // << arrow stmt + expr In this case, this issue #65 inconsistent with other "arrow expressions" 🤷 |
I think it makes sense to leave this as is and revisit in the future if we have a good reason to. Thanks for the thorough write-up here! |
There are several problems of short closures:
Problem#1: Semantical conflict with PSR-12
The existing PSR-12 standard defines that the body must contains on the new line in anonymous functions:
But the new PER standard suggests that there should be a body and arrow prefix (part of lambda stmt) on a new line:
Problem#2: Conflict with other languages (with lambdas)
The PER standard violates the existing specifications of other languages:
2.1) Kotlin: https://kotlinlang.org/docs/lambdas.html#function-types
2.2) C#: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/lambda-expressions
2.3) JS (Google CS): https://google.github.io/styleguide/jsguide.html#features-functions-top-level-functions
2.4) Ruby: rubocop/ruby-style-guide#479
2.5) etc (like a TS, CoffeeScript, Haskell)
Problem#3: Conflict with possible future language improvements
In future versions of the PHP, the implementation of anonymous functions is quite possible to combine several expressions (like this: https://wiki.php.net/rfc/auto-capture-closure) within the one block, for example:
The PER standard suggests that such expressions (if they appear) need to be written as follows:
Suggest
Allow the arrow (
=>
) symbol on the same line. That is set the function definition on a separate line:Contradictions
match
expressions) 7.1 Short closure improvements #65 (comment)The text was updated successfully, but these errors were encountered: