per-coding-style icon indicating copy to clipboard operation
per-coding-style copied to clipboard

New PHP syntax worth considering

Open KorvinSzanto opened this issue 3 years ago • 16 comments

I've gone through the PHP versions since 7.3 which was the most recently released when PSR-12 was accepted and gathered a list of syntax that we may want to cover in this PER. This list is likely incomplete so please add a comment if there's more I should add.

PHP 7.3: https://www.php.net/manual/en/migration73.php

  • [x] #5 Heredoc / Nowdoc indention: https://3v4l.org/lTXHQ
  • [x] ~~Array destructuring: https://3v4l.org/85FjD~~
  • [x] #18 Trailing commas in function / method calls: foo(1, 2, 3,)

PHP 7.4: https://www.php.net/manual/en/migration74.php

  • [x] #19 Class property types: public int $foo;
  • [x] #17 Short functions: fn(int $foo) => $foo * 2
  • [x] ~~Numeric literal separator: $int = 1_000_000;~~
  • [x] ~~__serialize and __unserialize vs using Serializable~~
  • [x] ~~Nested ternaries require parenthesis in some situations post 7.4, we should consider using SHOULD NOT for nested ternaries.~~

PHP 8.0: https://www.php.net/manual/en/migration80.php

  • [ ] #42 Named Arguments: foo(1, c: 2) https://3v4l.org/tglTA
  • [ ] #26 Attributes: https://3v4l.org/veQIX
  • [x] ~~Constructor property promotion: https://3v4l.org/2hirZ~~
  • [ ] #45 Union types: Foo|Baz vs Foo | Baz
  • [x] #21 Match expressions: https://3v4l.org/2s8uK
  • [x] ~~Throw as expression: $a = $b ?: throw Exception('b is falsy');~~
  • [x] #18 Trailing comma in parameter list: https://3v4l.org/RGf19
  • [x] ~~FILTER_VALIDATE_BOOL over FILTER_VALIDATE_BOOLEAN when using filter_var~~

PHP 8.1: https://www.php.net/manual/en/migration81.php

  • [x] ~~Array unpacking with string keys: $array = [...$otherData, 'b' => 1];~~
  • [x] ~~Named arguments used after argument unpacking: $result = foo(...$args, namedArg: 123) https://3v4l.org/PN7kg~~
  • [x] #7 Enums: https://3v4l.org/8Dljv
  • [x] First class callables: foo(...) vs foo( ... )
  • [ ] #45 Intersection types: Foo&Bar vs Foo & Bar
  • [x] ~~new keyword in parameter initializers: https://3v4l.org/vcqT5~~
  • [x] #19 Readonly properties

PHP 8.2: https://github.com/php/php-src/milestone/4

  • [ ] #41 Readonly classes

Extra potential things to cover:

  • [ ] Visibility keyword is not required when specifying readonly: public function __construct(readonly $foo)

KorvinSzanto avatar Jan 15 '22 19:01 KorvinSzanto

__serialize and __unserialize vs using Serializable

Serializable is deprecated (see https://wiki.php.net/rfc/phase_out_serializable) and is meant as a compatibility layer for old implementations.

FILTER_VALIDATE_BOOL over FILTER_VALIDATE_BOOLEAN

FILTER_VALIDATE_BOOL was added as it is the canonical type name, therefore this one should be recommended IMHO

For composite types, no spaces IMHO, especially when mixing union and intersection types (soon inc.) having loads of spaces is going to be detrimental to readability IMHO.

Girgias avatar Jan 15 '22 19:01 Girgias

Serializable is deprecated (see https://wiki.php.net/rfc/phase_out_serializable) and is meant as a compatibility layer for old implementations.

I wanted to include anything not removed just in case

FILTER_VALIDATE_BOOL was added as it is the canonical type name, therefore this one should be recommended IMHO

:+1: We already require folks use bool over boolean so this makes sense.

For composite types, no spaces IMHO, especially when mixing union and intersection types (soon inc.) having loads of spaces is going to be detrimental to readability IMHO.

I'm not so sure on this one, PSR-12 currently prefers spaces around other binary operators and other languages that have similar syntax also tend to prefer the spaces. ^1 ^2 ^3 I agree these declarations will get harder to read as they get longer especially since we don't have a great way to make larger union types portable like type FooBarBazType = Foo | Bar | Baz;. That said, I think we're going to have that issue whether we require spaces or not so we will probably need think about multiline declarations with something like:

function foo(): 
    null |
    SomeReallyLongClassName |
    SomeReallyLongOtherClassNameInterface |
    SomeReallyLongOtherOtherClassNameInterface
{
    return 1;
}

KorvinSzanto avatar Jan 15 '22 22:01 KorvinSzanto

In regard to multiline declaration, IMHO we should suggest to put the operators at the start of the line, so it makes immediately clear that there's a continued declaration (or operation, in other contexts like with || and &&):

function foo(): 
    null
    | SomeReallyLongClassName
    | SomeReallyLongOtherClassNameInterface
    | SomeReallyLongOtherOtherClassNameInterface
{
    return 1;
}

Jean85 avatar Jan 18 '22 16:01 Jean85

From what I've seen, union types are generally written without spaces right now. At least that's what TYPO3 has been doing as we add types for v12.

IF going multiline, I agree the | should be at the start, as is typical for other similar constructs.

Crell avatar Apr 29 '22 16:04 Crell

Looking at Typescript, union and intersection type declarations include spaces around operators (see https://github.com/microsoft/TypeScript/blob/main/src/lib/es5.d.ts#L174 for example).

ricardoboss avatar Jun 01 '22 21:06 ricardoboss

PR for short closures: https://github.com/php-fig/per-coding-style/pull/17

Crell avatar Jun 01 '22 22:06 Crell

PR for trailing commas: https://github.com/php-fig/per-coding-style/pull/18

Crell avatar Jun 01 '22 22:06 Crell

PR for typed properties and readonly: https://github.com/php-fig/per-coding-style/pull/19

Crell avatar Jun 01 '22 22:06 Crell

PR for attributes: https://github.com/php-fig/per-coding-style/pull/26

Crell avatar Jul 08 '22 21:07 Crell

Things I think we can be silent on:

  • Numeric literal separator: Is there a guideline here to use?
  • __serialize vs Serializable: The latter is deprecated anyway, so I don't know that we have much to add beyond the language itself.
  • Throw as an expression: I really don't know what we'd even say here.
  • new in parameter defaults: Again, I'm not sure there's anything to standardize here that isn't already covered by other statements. (Eg, space on either side of the =.)

Crell avatar Jul 08 '22 21:07 Crell

PR for FCC: https://github.com/php-fig/per-coding-style/pull/27

Crell avatar Jul 08 '22 21:07 Crell

Numeric literal separator: Is there a guideline here to use?

I don't think there is a generally applicable guideline other than “yes, please make use of it”. The “correct” grouping depends on a context:

const ONE_EURO_IN_CENTS = 1_00;
const THOUSAND_EUROS_IN_CENTS = 1_000_00;
const HUNDRED_THOUSAND = 100_000;
const ONE_MILLION = 1_000_000;

TimWolla avatar Jul 08 '22 22:07 TimWolla

Handles readonly classes: https://github.com/php-fig/per-coding-style/pull/41

Crell avatar Sep 12 '22 19:09 Crell

I'm going to argue that CPP is already adequately covered by existing rules, so nothing new needs to be said.

Named arguments are here: #42

Crell avatar Sep 12 '22 19:09 Crell

Nested ternaries: https://github.com/php-fig/per-coding-style/pull/43

Crell avatar Sep 12 '22 19:09 Crell

OK, I have gone through and ~~struck out~~ any changes that I believe require no action, either because they're not worth it or existing rules are already sufficient. Korvin, if you disagree with any of those go ahead and un-strike them.

Everything else now has an active PR. Discussion on those PRs. :smile:

Crell avatar Sep 12 '22 20:09 Crell

I'll call this done at this point.

Crell avatar Jan 13 '23 21:01 Crell