PSR12.Operators.OperatorSpacing interprets pipe character in catch declaration as binary operator

[pschultz]

Describe the bug

PSR12.Operators.OperatorSpacing produces false positives on type alternatives in catch clause.

Code sample

<?php

try {
} catch (A|B $exception) {
}

To reproduce
Steps to reproduce the behavior:

1. Create a file called test.php with the code sample above...
2. Run phpcs -s --standard=PSR12 test.php
3. See error message displayed

FILE: /tmp/tmp.0QHXuAapol/foo.php
---------------------------------------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 1 LINE
---------------------------------------------------------------------------------------------------------------
 4 | ERROR | [x] Expected at least 1 space before "|"; 0 found
   |       |     (PSR12.Operators.OperatorSpacing.NoSpaceBefore)
 4 | ERROR | [x] Expected at least 1 space after "|"; 0 found
   |       |     (PSR12.Operators.OperatorSpacing.NoSpaceAfter)
---------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------------------------------------------

Expected behavior

No error, since the pipe character in the code is not an operator. This already works as expected for function declarations, i.e. this is fine:

<?php

function f(A|B $x)
{
}

Versions (please complete the following information):

• OS: Fedora 53
• PHP: 8.0
• PHPCS: 3.7.1
• Standard: PSR12

[gsherwood]

This one is tricky to fix due to PSR12 and PHP itself.

The try/catch/finally example in PSR12 is this (https://www.php-fig.org/psr/psr-12/#56-try-catch-finally):

<?php

try {
    // try body
} catch (FirstThrowableType $e) {
    // catch body
} catch (OtherThrowableType | AnotherThrowableType $e) {
    // catch body
} finally {
    // finally body
}

Note the spaces around the pipe character.

This obviously pre-dates PHP8 union types, so the pipe character here has been treated as a bitwise OR in PHPCS because a union types concept didn't exist. PSR12 wants spaces around it, so PHPCS is functioning correctly as per PSR12.

With union types making it into PHP8, it would probably be more accurate to re-tokenize the pipe character in a catch to T_TYPE_UNION, but this breaks existing sniffs, including the PSR12 sniff that checks for whitespace around the pipe character.

So where I've landed on this is

1. PHPCS is working correctly as per the PSR12 spec
2. I'd like to reconsider this token in PHP 4, where backwards compatibility can be broken

Keen to hear people's thoughts on that.

[pschultz]

That's my bad, I guess. We encountered this issue after updating FriendsOfPHP/PHP-CS-Fixer, which removed the spaces, but I haven't double checked the standard.

As far as I'm concerned this issue can be closed.

[florentdestremau]

Hi! What's the workaround for this, having both php_cs and php-cs-fixer on the same projet? Ignore php_cs locally ?

[jrfnl]

First thing to ask if you are using both: why are you using both ?

[florentdestremau]

because they have complementary rulesets and especially complementary fixers. But I think it's beside the point here. The rules here are not compliant with the standard, that's the only thing to ask here: how can we make this work?

I agree with @gsherwood 's statement: if this is a BC break it should wait for v4. In the meantime I'm ignoring phpcs for the conflicting lines.

[jrfnl]

I believe this needs clarification from FIG / PSR PER about what the spacing rules are for the | operator in a multi-catch statement.

[jrfnl]

I've opened an issue with PER to ask for clarification: php-fig/per-coding-style#67