search

PHPCSStandards / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
979 stars 58 forks source link

Generic/EmptyPHPStatement: inconsistent behavior when fixing superfluous semicolon #728

Open rodrigoprimo opened 3 days ago

rodrigoprimo commented 3 days ago

Describe the bug

While working on https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/699, I noticed an inconsistency in how the Generic.CodeAnalysis.EmptyPHPStatement sniff fixes superfluous semicolons placed right after a PHP open tag when the PHP ini directive short_open_tag is enabled.

If a normal open tag is used, the superfluous semicolon is removed, and a space is left between the open tag and the rest of the code. If a shot open tag is used, no space is left between the open tag and the rest of the code. I'm inclined to think the sniff should behave the same for both types of open tags.

This is probably not a high-priority issue as the resulting code is not a parse error, and short open tags are not very popular.

Related to https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/593

Code sample

<?php ; my_function(); ?>

<? ; my_function(); ?>

To reproduce

Steps to reproduce the behavior:

  1. Ensure that the short_open_tag ini directive is enabled.
  2. Create a file called test.php with the code sample above.
  3. Run phpcbf --standard=Generic --sniffs=Generic.CodeAnalysis.EmptyPHPStatement test.php
  4. See that the resulting test.php file contains the following. Note there is no space between <? and my_function();. 
    
<?php my_function(); ?>

<?my_function(); ?>



## Expected behavior

I expect the sniff to preserve the space between `<?` and `my_function();`.

## Versions (please complete the following information)

|                                       |                                                                                                           |
|--------------------------|----------------------------------------------------------------------  |
| Operating System           | Ubuntu 24.04                                                       |
| PHP version                    | 8.3                                                                                     |
| PHP_CodeSniffer version | master                                                                            |
| Standard                         | Generic                                                      |
| Install type                      | git clone |

## Please confirm

- [x] I have searched the issue list and am not opening a duplicate issue.
- [x] I have read the [Contribution Guidelines](https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/.github/CONTRIBUTING.md) and this is not a [support question](https://github.com/PHPCSStandards/PHP_CodeSniffer/discussions).
- [x] I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
- [x] I have verified the issue still exists in the `master` branch of PHP_CodeSniffer.
jrfnl commented 3 days ago

Leaving a single space after a short open tag makes sense to me and a PR to add special handling for short open tags would be welcome. Though as you already said: low priority as it's not a parse error and AFAICS not a cause for fixer conflicts.