squizlabs / PHP_CodeSniffer

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

PEAR.Functions.FunctionCallSignature incorrect indent fix when checking mixed HTML/PHP files #3666

Closed giorgosbotis closed 2 years ago

giorgosbotis commented 2 years ago

running phpcbf on a file containing the following code:

<?php include get_theme_file_path(
        '/theme_extra/test_block.php'
      ); ?>   

I get the following error: Fatal error: Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: Undefined array key -1 in /app/myproject/vendor/squizlabs/php_codesniffer/src/Fixer.php on line 510 in /app/myproject/vendor/squizlabs/php_codesniffer/src/Runner.php:604

gsherwood commented 2 years ago

I can't replicate this issue. What coding standard are you using and with what PHPCS version?

giorgosbotis commented 2 years ago

Sorry for not adding info: phpcs 3.7.1 php 8.0.19 phpcbf --standard=PEAR ./test.php

the exact test.php files has some tabs in front of the actual code like below:

      <?php include get_theme_file_path(
        '/theme_extra/test_block.php'
      ); ?>         
gsherwood commented 2 years ago

Thanks, have managed to replicate. Doesn't appear to be tab related, but the indent is important.

gsherwood commented 2 years ago

So the issue here is the indent before the open PHP tag. If it's divisible by the indent size (normally 4 characters) then you wont get an error message and nothing will try to be fixed.

If it's not (I believe the example code has 6 spaces on indent) then it will try and bring the indent back in line with a tab stop. When it did this with inline HTML (anything outside the PHP tags) it did the fix incorrectly and removed content. If the content was the first thing in the file, you get the error message in this bug report.

The fix I'm committing will still report the same error message, but the auto-fixer in PHPCBF will now treat inline HTML differently so it can apply the fix properly.

gsherwood commented 2 years ago

I've committed a fix for this issue, which will be in the 3.7.2 release. Thanks a lot for reporting it.

jrfnl commented 2 years ago

Looks like the OP also reported this (with a different description) in https://github.com/WordPress/WordPress-Coding-Standards/issues/2083 for which I opened PR #3667....

giorgosbotis commented 2 years ago

It was reported with a different description, as there were two different issues I encountered.

The one reported here was getting an exception running cbf with PEAR standard only, while the other one was Worpdress-Core standard producing wrong code (no exception there).

I am sorry but I had no clue they were connected or the same.