PHP_CodeSniffer
PHP_CodeSniffer copied to clipboard
PSR12.Operators.OperatorSpacing interprets pipe character in catch declaration as binary operator
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:
- Create a file called
test.php
with the code sample above... - Run
phpcs -s --standard=PSR12 test.php
- 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
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
- PHPCS is working correctly as per the PSR12 spec
- I'd like to reconsider this token in PHP 4, where backwards compatibility can be broken
Keen to hear people's thoughts on that.
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.