Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Squiz.PHP.DisallowMultipleAssignments false positive for list assignments after embedded PHP without semicolon #537

Closed
4 tasks done
biinari opened this issue Jul 12, 2024 · 1 comment · Fixed by #538
Closed
4 tasks done

Comments

@biinari
Copy link
Contributor

biinari commented Jul 12, 2024

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:

  1. Create a file called test.php with the code sample above
  2. Create a file called phpcs.xml with the custom ruleset above
  3. Run phpcs test.php
  4. 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:

  1. A block (e.g. if, while) can be added before line 7 and the sniff will still issue an error.
  2. Surrounding the short echo tag with brackets <?= ($a * $b) ?> resolves this issue. Enabling Squiz.Formatting.OperatorBracket.MissingBrackets would enforce this usage.
  3. Adding a semicolon to the short echo tag <?= $a * $b; ?> resolves this issue. Enabling Squiz.PHP.EmbeddedPhp.ShortOpenEchoNoSemicolon would enforce this usage.
  4. 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.
@jrfnl
Copy link
Member

jrfnl commented Jul 13, 2024

@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.

@jrfnl jrfnl changed the title Squiz.PHP.DisallowMultipleAssignments false positive following short echo tag Squiz.PHP.DisallowMultipleAssignments false positive for list assignments after embedded PHP without semicolon Jul 13, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
2 participants