Skip to content

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

Closed
SerafimArts opened this issue Mar 1, 2023 · 5 comments
Closed

7.1 Short closure improvements #65

SerafimArts opened this issue Mar 1, 2023 · 5 comments

Comments

@SerafimArts
Copy link

SerafimArts commented Mar 1, 2023

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:

$foo = function (mixed $foo): void { // << definition
  // << body
}; // << terminal stmts

But the new PER standard suggests that there should be a body and arrow prefix (part of lambda stmt) on a new line:

$foo = fn(mixed $foo): void // << definition
  => 42; // << definition + body + terminal stmt

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

// ...some code...
acc: Int, i: Int -> 
    print("acc = $acc, i = $i, ") 
// ...some code...

2.2) C#: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/lambda-expressions

Action<string> greet = name =>
{
    string greeting = $"Hello {name}!";
    Console.WriteLine(greeting);
};

2.3) JS (Google CS): https://google.github.io/styleguide/jsguide.html#features-functions-top-level-functions

exports.processString = (str) => {
  // Process the string.
};

2.4) Ruby: rubocop/ruby-style-guide#479

l = ->(a, b) {
  tmp = a * 7
  tmp * b / 50
}

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:

$foo = fn(int $a, int $b) => {
    $c = $a + $b;
    return $c - 0xDEAD_BEEF;
};

The PER standard suggests that such expressions (if they appear) need to be written as follows:

$foo = fn(int $a, int $b) 
    => {
        $c = $a + $b;
        return $c - 0xDEAD_BEEF;
};

Suggest

Allow the arrow (=>) symbol on the same line. That is set the function definition on a separate line:

// Example 1

$foo = fn(int $a, int $b): int =>
    $a + $b;

// Example 2 (also valid style)

$foo = fn(int $a, int $b): int => 
    $a + $b
;

// Example 3 (possible future lang changes)

$foo = fn(int $a, int $b): int => {
    return $a + $b;
};

Contradictions

  1. See (trailing commas in match expressions) 7.1 Short closure improvements #65 (comment)
@KorvinSzanto
Copy link
Contributor

KorvinSzanto commented Mar 2, 2023

Thanks for taking the time to weigh in on this, you make some good points so I want to address them each individually.

Semantical conflict with PSR-12

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.

Conflict with other languages (with lambdas)

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.

Conflict with possible future language improvements

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's proven by the fact that the linked RFC failed and the fact that there are no current efforts to pick up where that RFC left off.


That all said, I'm not 100% sure where I sit on this yet.

@Crell
Copy link
Collaborator

Crell commented Mar 2, 2023

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. 😄

@SerafimArts
Copy link
Author

SerafimArts commented Mar 2, 2023

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.

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.

@SerafimArts
Copy link
Author

SerafimArts commented Mar 2, 2023

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" 🤷

@KorvinSzanto
Copy link
Contributor

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!

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

No branches or pull requests

3 participants