slevomat / coding-standard

Slevomat Coding Standard for PHP_CodeSniffer provides many useful sniffs
MIT License
1.39k stars 171 forks source link

[PHP7.4] ReferencedNameHelper::createAllReferencedNames produces warnings on php multiline comment in HEREDOC #1355

Closed enl closed 2 years ago

enl commented 2 years ago

Steps to reproduce:

  1. Create a example.php file with the following code:
<?php

<<<EXAMPLE
/**
 * I'm a multiline comment inside HEREDOC
 * Why do I even exist?
 * One can create a sniff that checks copyright correctness and want to provide an example :)
 */
EXAMPLE;
  1. Run phpcs --standard=SlevomatCodingStandard example.php

Expected Results:

No warnings.

Actual results:

PHP Warning:  Unterminated comment starting line 1 in /Users/aleksandrpanshin/work/modules/phpcs-rulesets/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/ReferencedNameHelper.php on line 470

Warning: Unterminated comment starting line 1 in /Users/aleksandrpanshin/work/modules/phpcs-rulesets/vendor/slevomat/coding-standard/SlevomatCodingStandard/Helpers/ReferencedNameHelper.php on line 470

Notes

According to my investigation, it happens because multiline heredoc creates several tokens and they are searched for references one by one in ReferencedNameHelper::createAllReferencedNames in this piece:

// Find referenced names inside double quotes string
if (self::isNeedParsedContent($tokens[$nameStartPointer]['code'])) {
$content = $tokens[$nameStartPointer]['content'];

if (self::isNeedParsedContent($tokens[$nameStartPointer - 1]['code'])) {
    $content = '"' . $content;
}
if (self::isNeedParsedContent($tokens[$nameStartPointer + 1]['code'])) {
    $content .= '"';
}
$names = self::getReferencedNamesFromString($content);
foreach ($names as $name) {
    $referencedNames[] = new ReferencedName($name, $nameStartPointer, $nameStartPointer, ReferencedName::TYPE_CLASS);
}
$beginSearchAtPointer = $nameStartPointer + 1;
continue;
}

Which in process calls self::getReferencedNamesFromString("/**");, and this function calls token_get_all("/**");, which produces the warning because comment is indeed not closed.

The issue started once I updated to 7.1.

kukulich commented 2 years ago

I'm not able to reproduce the bug. Can you please try to write failing test?

enl commented 2 years ago

Hello @kukulich!

Added as a PR. This is enough to start getting the warning:

1) SlevomatCodingStandard\Helpers\ReferencedNameHelperTest::testGetAllReferencedNames
This test printed output: 
Warning: Unterminated comment starting line 1 in /Users/aleksandrpanshin/work/slevomat-coding-standard/SlevomatCodingStandard/Helpers/ReferencedNameHelper.php on line 469

I should have added this piece of information in the very first comment, but...

% php --version
PHP 7.4.22 (cli) (built: Jul 29 2021 17:27:21) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Xdebug v3.1.0, Copyright (c) 2002-2021, by Derick Rethans
    with Zend OPcache v7.4.22, Copyright (c), by Zend Technologies

Is it possible that you run it on newer version of php that got it fixed in the tokeniser itself?

enl commented 2 years ago

Yup, checked build status:

The warning is there for 7.4 but neither for 8.0 nor 8.1.

kukulich commented 2 years ago

Ok, so that's probably bug in PHPCS - the tokens should be same on all PHP versions.

enl commented 2 years ago

Yeah, sounds logical. I'll create an issue on their repo.

Although, question is what is expected list of tokens: one we get on 7.4 or one we get on 8.+ :)

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.