php-fig / per-coding-style Goto Github PK

I believe the indentation requirement of nowdoc/heredoc is not obviously correct within a multi-line function parameter list, as the ending delimiter also needs to be indented compared to e.g. an anonymous function.

<?php

// Right?
var_dump(
    'foo',
    <<<'EOT'
        Line 1
        Line 2
        Line 3
        EOT,
    'bar',
);

// Wrong?
var_dump(
    'foo',
    <<<'EOT'
    Line 1
    Line 2
    Line 3
    EOT,
    'bar',
);

see https://3v4l.org/eGlGY
see #5

PHP supports trailing commas in arrays since 7.2. As of now the existing formatting standards of PHP-FIG do not specify whether or not (or when) trailing commas in Arrays are acceptable or required. More generally the formatting of multi-line arrays is left unspecified, whereas for other constructs it is clearly specified how they need to be split across multiple lines.

This is related to #3, since PHP 7.3+ introduces additional constructs where trailing commas are allowed and PHP 8.0 introduces named parameters which are syntactically very close to an associative array literal.

Need to check the following issues and discussions that came after PSR-12 was accepted:

• php-fig/fig-standards#1206
• https://groups.google.com/g/php-fig/c/LItcyM27ll0/m/0Iq00esqBgAJ
• https://groups.google.com/g/php-fig/c/4gZe07mWWvk/m/up8rgfe4AgAJ
• https://groups.google.com/g/php-fig/c/nbpm5wLTt2w/m/mlYP5lXFAwAJ
• https://groups.google.com/g/php-fig/c/J77aMA9LLaU/m/CF9Xq_uFAQAJ
• https://groups.google.com/g/php-fig/c/9RybRUHLWuU/m/TQbyJ-47BgAJ
• https://groups.google.com/g/php-fig/c/VWcw3NnNHnM/m/HIyWQk2EBwAJ
• https://groups.google.com/g/php-fig/c/F0IVXx1LL_Q/m/62QdL7_pDQAJ
• https://groups.google.com/g/php-fig/c/FloEjPcgIag/m/K3a-d-VxBQAJ

Currently I will write method chaining like this:

final class User
{
  public function scopeAuthor(Builder $query): Builder
  {
    return $query->whereIn("status", [
        User::STATUS_SUSPENDED, 
        User::STATUS_BANNED, 
      ])
      ->where("active", true) 
      ->whereHas("books");
  } 
} 

I know I was writing the methods chaining like this a while ago

final class User extends Model
{
  public function scopeAuthor(Builder $query): Builder
  {
    return $query->whereIn("status", [
      User::STATUS_SUSPENDED, 
      User::STATUS_BANNED, 
    ])
    ->where("active", true) 
    ->whereHas("books");
  } 
}

I've seen on Twitter people use a trick when the first method call is multiline

final class User
{
  public function scopeAuthor(Builder $query): Builder
  {
    return $query->new()
      ->whereIn("status", [
        User::STATUS_SUSPENDED, 
        User::STATUS_BANNED, 
      ])
      ->where("active", true) 
      ->whereHas("books");
  } 
} 

Maybe it's worth considering adding a word about it.

Has switching to tabs instead of spaces been considered for accessibility reasons? This tweet has been making the rounds recently: https://twitter.com/Rich_Harris/status/1541761871585464323

Kirby CMS recently made the switch too: getkirby/kirby#4454

The foreach is not too clear whether it is okay to break the expression into multiple lines. https://www.php-fig.org/psr/psr-12/#55-foreach . Other expressions like for and while are very clear on the issue. I am unsure why this restriction is on foreach if other loop expressions are "allowed" to do this.

I need to know for these two issues:
squizlabs/PHP_CodeSniffer#3673
prettier/plugin-php#2060

example:

foreach (
    $client->getContractsByCustomerId($customer["id"])
    as $contract
) {

or is only this within the standard:

foreach ($client->getContractsByCustomerId($customer["id"]) as $contract) {

на русском не планируете? Спасибо

Currently all of the PSR/PER code examples show using PHP short array syntax $thing = [];. PRs (ex: #18) also use short array syntax $arr = ['a' => 'A', 'b' => 'B', 'c' => 'C'];. There is no rule though requiring short array syntax.

The short array syntax was adopted for good reasons. According to the RFC:

• Good for framework development when dealing with long parameterlists
• Other web languages have similar syntax
• Readable

Scouring Github repositories, PHP short array syntax is very prevalent in popular PHP frameworks. Using the old syntax $thing = array(...); is relatively rare. Most occurrences are in old repositories or in conversion from old to short syntax tools.

Would creating a PR for adding a MUST use short array syntax section be welcomed?

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) #65 (comment)

Enum case declarations MUST use CamelCase capitalization.

Should be Enum case declarations MUST use PascalCase capitalization. since that is what is both demonstrated and consistent with constants in Enumerations.

Are there any requirements regarding spaces in type casting? There is the requirement:

Type casting operators MUST NOT have any space within the parentheses

But what about the space between the type and the expression? I.e.

$intValue = (int) $input;

or

$intValue = (int)$input;

To the best of my recollection in all of the code I've encountered, despite functioning correctly in either order, attributes are placed between any docblock and the target they apply to (class, method, etc). If you were to instead place attributes above the docblock, you'd encounter a nasty bug in any static analyser that relies on nikic's parser (eg phpstan): nikic/PHP-Parser#762. In my opinion, this PER should require a consistent order of docblock first, then attributes and finally the node.

PER Coding Style allows the use of grouped import:

use function Foo\Bar\{baz, qux};

But what about multi import statements?

use Foo\Bar, A\B\C;

use function array_key_exists, count, in_array;

In my opinion it should be allowed for root classes/functions/constants (because otherwise they could not be used in a grouped style). But it should be disallowed for namespaced elements.

I believe some clarification is needed about the spacing requirements around the | operator when used in the following two situations:

• Union types
• Multi-catch statements

At this moment, most coding standards I'm aware of use "no spaces around the | operator" as a rule for union types.

However, multi-catch statements predate union types and the example in section 5.6 had spaces around the | operator.
PHPCS has taken the literal interpretation based on the example and expects one space on either side of the | operator in multi-catch statements.
CS-Fixer treats the | in multi-catch as if it were a union type, expecting no spaces.

End-users are confused ;-/

Ref: squizlabs/PHP_CodeSniffer#3663

While the operator precedence in PHP is clearly defined so that && has higher precedence than || and this precedence matches the vast majority of programming languages in use, expressions mixing those two operators without explicit parentheses still require the reader to know the precedence off the top of their head, adding room for error.

The coding style should require (or recommend) explicit parentheses whenever && and || are mixed within an expression to make the intent clear:

if ($foo && $bar || $baz) { } // no
if (($foo && $bar) || $baz) { } // yes
if ($foo && ($bar || $baz)) { } // yes, but different semantics than the "no".

The "Disjunctive Normal Form Types" RFC that is currently under discussion explicitly requires parentheses for types that mix union and intersection, likely for the same reason: Removing any possible ambiguity.

There's an open pull request for PHP_CodeSniffer that performs and highlights expressions where these operators are mixed without parentheses:

squizlabs/PHP_CodeSniffer#3205

Anecdotally running the Sniff from the PR on our code base exposed a non-trivial number of bugs where $foo && ($bar || $baz) was intended, but $foo && $bar || $baz was used, showing that this is an issue that happens in the real world.

Look at the indentation in short closure multiline example.

The indentation bounces wildly, it feels not right.

It's because blocks go in form that is not consistent with all other constructs.

Better:

$func = fn(
        int $x,
        int $y,
    ): int
    => $x + $y;

I've read and re-read the specs, and had a peer search as well, and I am not finding clarity that there MUST be a blank line between class methods. All the examples show it, but it doesn't seem to be clear that it is required in the spec.

For example, this seems to be allowed:

class BlankLine
{
    public function firstMethod()
    {
        ...
    }
    public function nextMethod()
    {
        ...
    }
}

When it should be this instead:

class BlankLine
{
    public function firstMethod()
    {
        ...
    }

    public function nextMethod()
    {
        ...
    }
}

Can anyone prove me wrong in the spec? If not, I'll get a PR open to clarify this.

Leaving this on @Crell's request:

The PSR-12 documentation has this section: https://www.php-fig.org/psr/psr-12/#3-declare-statements-namespace-and-import-statements

Compound namespaces with a depth of more than two MUST NOT be used. Therefore the following is the maximum compounding depth allowed:

While it should be clear that this restriction of two namespaces is specific to the use of grouping, I was confused for a bit whether it was meant to limit all namespaces to a depth of 4 total, 2 beyond the package namespace. This was because "compounding" is not what I've ever heard that called (always heard it called grouping instead), and because all the examples in that section, even those that do not have grouping, are limited to a total depth of 4, except the example of what is not allowed.

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

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

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

• #19 Class property types: public int $foo;
• #17 Short functions: fn(int $foo) => $foo * 2
• Numeric literal separator: $int = 1_000_000;
• __serialize and __unserialize vs using Serializable
• 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
• Constructor property promotion: https://3v4l.org/2hirZ
• #45 Union types: Foo|Baz vs Foo | Baz
• #21 Match expressions: https://3v4l.org/2s8uK
• Throw as expression: $a = $b ?: throw Exception('b is falsy');
• #18 Trailing comma in parameter list: https://3v4l.org/RGf19
• FILTER_VALIDATE_BOOL over FILTER_VALIDATE_BOOLEAN when using filter_var

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

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

I wanted to clearly define what "major" "minor" and "patch" mean in the context of this PER and how we will handle versioning for v1.0.0 and after. This is an important thing to nail down because we only need core committee approval for major versions.

To make this easier to follow, let's split the available requirement keywords into two groups:

1. "MUST"s: MUST, REQUIRED or SHALL
2. "SHOULD"s: SHOULD, RECOMMEND, MAY, or OPTIONAL

• Major: Altered/added/removed MUSTs for already covered syntax and altered/removed SHOULDs
• Minor: Added SHOULDs and added MUSTs for new syntax
• Patch: Any small changes to text like typos, clarifications, and anything else that does not effect compatibility with any requirements

There are times when SHOULDs can be modified or even removed in backwards compatible ways and in those cases we may opt for a minor release rather than a major release, erring on the side of a major release (and therefore a core committee vote) whenever appropriate. This is all up for discussion so if folks have any disagreements please add comments here.

With all of that said version 1.0.0 will be effectively a "patch" change from PSR-12, all changes are done to align names and descriptions with this PER and no requirements have been altered/added/removed.

Unless you are specifically looking at the URL of the page, you don't know what PER version the specifications are. The version should be placed near the top of the document, above the fold.

This allows mere mortals to glance at the document to see the version. Without it, there is no way to ascertain that the page contains the same version as another page. The images contain two different pages that may or may not have the same information, but there is no way to tell which is the most current version.

We have a recommendation to use StudlyCaps (PascalCase) naming in the PER:

PER

The term "StudlyCaps" in PSR-1 MUST be interpreted as PascalCase where the first letter of each word is capitalized including the very first letter.
-- https://www.php-fig.org/per/coding-style/#2-general

Class names

Class names MUST be declared in StudlyCaps
-- https://www.php-fig.org/psr/psr-1/#3-namespace-and-class-names

Method names

Method names MUST be declared in camelCase().
-- https://www.php-fig.org/psr/psr-1/#43-methods

But how should we write abbreviations and acronyms?

I suggest following camel case, e.g., loadHttpUrl as recommended in the "Google TypeScript Style Guide".

Google TypeScript Style Guide:

5.1.3 Camel case
Treat abbreviations like acronyms in names as whole words, i.e. use loadHttpUrl, not loadHTTPURL, unless required by a platform name (e.g. XMLHttpRequest).
-- https://google.github.io/styleguide/tsguide.html#camel-case

See Steven Mcconnell "Code Complete", page 729, chapter 31 "Layout and Style".

Blocks (multiline statements) must be delimited with a blank line.

So instead of

<?php

class Talker
{
    use A;
    use B {
        A::smallTalk insteadof B;
    }
    use C {
        B::bigTalk insteadof C;
        C::mediumTalk as FooBar;
    }
}

you get

<?php

class Talker
{
    use A;

    use B {
        A::smallTalk insteadof B;
    }

    use C {
        B::bigTalk insteadof C;
        C::mediumTalk as FooBar;
    }
}

It's more readable and keeps consistent rythm throughout all text.