Squiz.PHP.DisallowMultipleAssignments false positive for list assignments after embedded PHP without semicolon
biinari opened this issue · 1 comments
Describe the bug
Following a short echo tag with an expression, a false positive is given by Squiz.PHP.DisallowMultipleAssignments
for destructured assignment using list()
Code sample
<?php
$a = 1;
$b = 2;
?>
<?= $a * $b ?>
<?php
list ($c, $d) = explode(',', '1,2');
Custom ruleset
<?xml version="1.0"?>
<ruleset name="My Custom Standard">
<description>Ruleset to demonstrate this issue</description>
<rule ref="Squiz">
<exclude name="Generic.PHP.DisallowShortOpenTag.EchoFound" />
<exclude name="Squiz.Commenting.FileComment" />
<exclude name="Squiz.Formatting.OperatorBracket.MissingBrackets" />
<exclude name="Squiz.PHP.EmbeddedPhp.ShortOpenEchoNoSemicolon" />
</rule>
</ruleset>
To reproduce
Steps to reproduce the behaviour:
- Create a file called
test.php
with the code sample above - Create a file called
phpcs.xml
with the custom ruleset above - Run
phpcs test.php
- See error message displayed
FILE: (path snipped)/test.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
7 | ERROR | Assignments must be the first block of code on a line
----------------------------------------------------------------------
Expected behavior
I expect no error to be found by Squiz.PHP.DisallowMultipleAssignments
here.
Versions
Operating System | [Ubuntu 22.04.4] |
PHP version | [7.4, 8.3] |
PHP_CodeSniffer version | [3.10.1, master] |
Standard | [Squiz, custom] |
Install type | [Composer (local)] |
Additional context
The syntax given on the PHP tags documentation demonstrates use without a semicolon like:
<?= 'print this string' ?>
This leads me to believe that such usage should be allowed.
I tripped into this issue when removing a line of code that sat between the short echo tag with the expression and the destructuring assignment.
Further interesting factors:
- A block (e.g.
if
,while
) can be added before line 7 and the sniff will still issue an error. - Surrounding the short echo tag with brackets
<?= ($a * $b) ?>
resolves this issue. EnablingSquiz.Formatting.OperatorBracket.MissingBrackets
would enforce this usage. - Adding a semicolon to the short echo tag
<?= $a * $b; ?>
resolves this issue. EnablingSquiz.PHP.EmbeddedPhp.ShortOpenEchoNoSemicolon
would enforce this usage. - A single-line statement added before line 7 will resolve this issue.
Please confirm
- I have searched the issue list and am not opening a duplicate issue.
- I have read the Contribution Guidelines and this is not a support question.
- I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
- I have verified the issue still exists in the
master
branch of PHP_CodeSniffer.
@biinari Thanks for opening this bug report. Nice find!
I've had a quick look and have confirmed the bug. This is unrelated to the short echo tag though, the same behaviour can be observed when using a full PHP open tag for the embedded PHP snippet.
Basically, the sniff walks back too far and doesn't take code going in and out of PHP into account sufficiently.
PR #538 should fix this. Testing appreciated.