Skip to content

Commit

Permalink
bug #3823 NativeConstantInvocationFixer - better constant detection (…
Browse files Browse the repository at this point in the history 
…gharlan, SpacePossum, keradus)

This PR was squashed before being merged into the 2.12 branch (closes #3823).

Discussion
----------

NativeConstantInvocationFixer - better constant detection

Some previous failing test cases:

```php
M_PI::foo();

function foo(M_PI $a) {}

function foo(): M_PI {}

$foo instanceof M_PI;

class x implements FOO, BAR, BAZ {}

use X\Y\{FOO, BAR as BAR2, BAZ};

class Foo { use Bar, M_PI { Bar::baz insteadof M_PI; } }

M_PI: goto M_PI; // ;)
```

Commits
-------

a8a4ef2 NativeConstantInvocationFixer - better constant detection
  • Loading branch information
@SpacePossum
SpacePossum committed Jul 5, 2018
2 parents b6c27d5 + a8a4ef2 commit ceec561
Show file tree
Hide file tree
Showing 4 changed files with 316 additions and 13 deletions.
22 changes: 9 additions & 13 deletions src/Fixer/ConstantNotation/NativeConstantInvocationFixer.php
View file
Expand Up @@ -19,9 +19,9 @@
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\Tokenizer\Analyzer\NamespaceUsesAnalyzer;
use PhpCsFixer\Tokenizer\CT;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;
use PhpCsFixer\Tokenizer\TokensAnalyzer;
use Symfony\Component\OptionsResolver\Exception\InvalidOptionsException;

/**
Expand Down Expand Up @@ -155,35 +155,31 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
}
}

$tokenAnalyzer = new TokensAnalyzer($tokens);

$indexes = [];
foreach ($tokens as $index => $token) {
$tokenContent = $token->getContent();

// test if we are at a constant call
if (!$token->isGivenKind(T_STRING)) {
continue;
}

$prevIndex = $tokens->getPrevMeaningfulToken($index);
if ($tokens[$prevIndex]->isGivenKind(T_AS)) {
continue;
}
$tokenContent = $token->getContent();

$nextIndex = $tokens->getNextMeaningfulToken($index);
if ($tokens[$nextIndex]->equals('(')) {
if (!isset($this->constantsToEscape[$tokenContent]) && !isset($this->caseInsensitiveConstantsToEscape[strtolower($tokenContent)])) {
continue;
}

$constantNamePrefix = $tokens->getPrevMeaningfulToken($index);
if ($tokens[$constantNamePrefix]->isGivenKind([CT::T_USE_TRAIT, T_CLASS, T_CONST, T_DOUBLE_COLON, T_EXTENDS, T_FUNCTION, T_IMPLEMENTS, T_INTERFACE, T_NAMESPACE, T_NEW, T_NS_SEPARATOR, T_OBJECT_OPERATOR, T_TRAIT, T_USE])) {
if (isset($useConstantDeclarations[$tokenContent])) {
continue;
}

if (!isset($this->constantsToEscape[$tokenContent]) && !isset($this->caseInsensitiveConstantsToEscape[strtolower($tokenContent)])) {
$prevIndex = $tokens->getPrevMeaningfulToken($index);
if ($tokens[$prevIndex]->isGivenKind(T_NS_SEPARATOR)) {
continue;
}

if (isset($useConstantDeclarations[$tokenContent])) {
if (!$tokenAnalyzer->isConstantInvocation($index)) {
continue;
}

Expand Down
67 changes: 67 additions & 0 deletions src/Tokenizer/TokensAnalyzer.php
View file
Expand Up @@ -296,6 +296,73 @@ public function isLambda($index)
return $startParenthesisToken->equals('(');
}

/**
* Check if the T_STRING under given index is a constant invocation.
*
* @param int $index
*
* @return bool
*/
public function isConstantInvocation($index)
{
if (!$this->tokens[$index]->isGivenKind(T_STRING)) {
throw new \LogicException(sprintf('No T_STRING at given index %d, got %s.', $index, $this->tokens[$index]->getName()));
}

$nextIndex = $this->tokens->getNextMeaningfulToken($index);

if (
$this->tokens[$nextIndex]->equalsAny(['(', '{']) ||
$this->tokens[$nextIndex]->isGivenKind([T_AS, T_DOUBLE_COLON, T_ELLIPSIS, T_NS_SEPARATOR, CT::T_RETURN_REF, CT::T_TYPE_ALTERNATION, T_VARIABLE])
) {
return false;
}

$prevIndex = $this->tokens->getPrevMeaningfulToken($index);

if ($this->tokens[$prevIndex]->isGivenKind([T_AS, T_CLASS, T_CONST, T_DOUBLE_COLON, T_FUNCTION, T_GOTO, CT::T_GROUP_IMPORT_BRACE_OPEN, T_INTERFACE, T_OBJECT_OPERATOR, T_TRAIT])) {
return false;
}

while ($this->tokens[$prevIndex]->isGivenKind([CT::T_NAMESPACE_OPERATOR, T_NS_SEPARATOR, T_STRING])) {
$prevIndex = $this->tokens->getPrevMeaningfulToken($prevIndex);
}

if ($this->tokens[$prevIndex]->isGivenKind([CT::T_CONST_IMPORT, T_EXTENDS, CT::T_FUNCTION_IMPORT, T_IMPLEMENTS, T_INSTANCEOF, T_INSTEADOF, T_NAMESPACE, T_NEW, T_USE, CT::T_USE_TRAIT])) {
return false;
}

// `FOO & $bar` could be:
// - function reference parameter: function baz(Foo & $bar) {}
// - bit operator: $x = FOO & $bar;
if ($this->tokens[$nextIndex]->equals('&') && $this->tokens[$this->tokens->getNextMeaningfulToken($nextIndex)]->isGivenKind(T_VARIABLE)) {
$checkIndex = $this->tokens->getPrevTokenOfKind($prevIndex, [';', '{', '}', [T_FUNCTION], [T_OPEN_TAG], [T_OPEN_TAG_WITH_ECHO]]);

if ($this->tokens[$checkIndex]->isGivenKind(T_FUNCTION)) {
return false;
}
}

// check for `implements`/`use` list
if ($this->tokens[$prevIndex]->equals(',')) {
$checkIndex = $prevIndex;
while ($this->tokens[$checkIndex]->equalsAny([',', [T_AS], [CT::T_NAMESPACE_OPERATOR], [T_NS_SEPARATOR], [T_STRING]])) {
$checkIndex = $this->tokens->getPrevMeaningfulToken($checkIndex);
}

if ($this->tokens[$checkIndex]->isGivenKind([CT::T_GROUP_IMPORT_BRACE_OPEN, T_IMPLEMENTS, CT::T_USE_TRAIT])) {
return false;
}
}

// check for goto label
if ($this->tokens[$nextIndex]->equals(':') && $this->tokens[$prevIndex]->equalsAny([';', '}', [T_OPEN_TAG], [T_OPEN_TAG_WITH_ECHO]])) {
return false;
}

return true;
}

/**
* Checks if there is an unary successor operator under given index.
*
Expand Down
30 changes: 30 additions & 0 deletions tests/Fixer/ConstantNotation/NativeConstantInvocationFixerTest.php
View file
Expand Up @@ -152,6 +152,13 @@ public function provideFixWithDefaultConfigurationCases()
['<?php class Foo { function bar() { $this->M_PI(self::M_PI); } }'],
['<?php namespace Foo; use M_PI;'],
['<?php namespace Foo; use Bar as M_PI;'],
['<?php echo Foo\\M_PI\\Bar;'],
['<?php M_PI::foo();'],
['<?php function x(M_PI $foo, M_PI &$bar, M_PI ...$baz) {}'],
['<?php $foo instanceof M_PI;'],
['<?php class x implements FOO, M_PI, BAZ {}'],
['<?php class Foo { use Bar, M_PI { Bar::baz insteadof M_PI; } }'],
['<?php M_PI: goto M_PI;'],
[
'<?php echo \\M_PI;',
'<?php echo M_PI;',
Expand Down Expand Up @@ -187,6 +194,29 @@ public function provideFixWithDefaultConfigurationCases()
];
}

/**
* @dataProvider provideFix70WithDefaultConfigurationCases
*
* @param string $expected
* @param null|string $input
* @requires PHP 7.0
*/
public function testFix70WithDefaultConfiguration($expected, $input = null)
{
$this->doTest($expected, $input);
}

/**
* @return array
*/
public function provideFix70WithDefaultConfigurationCases()
{
return [
['<?php function foo(): M_PI {}'],
['<?php use X\Y\{FOO, BAR as BAR2, M_PI};'],
];
}

/**
* @dataProvider provideFixWithConfiguredCustomIncludeCases
*
Expand Down
210 changes: 210 additions & 0 deletions tests/Tokenizer/TokensAnalyzerTest.php
View file
Expand Up @@ -503,6 +503,216 @@ function foo (): ?int {
];
}

/**
* @param string $source
*
* @dataProvider provideIsConstantInvocationCases
*/
public function testIsConstantInvocation($source, array $expected)
{
$tokensAnalyzer = new TokensAnalyzer(Tokens::fromCode($source));

foreach ($expected as $index => $isLambda) {
$this->assertSame($isLambda, $tokensAnalyzer->isConstantInvocation($index), 'Token at index '.$index.' should match the expected value.');
}
}

public function provideIsConstantInvocationCases()
{
return [
[
'<?php echo FOO;',
[3 => true],
],
[
'<?php echo \FOO;',
[4 => true],
],
[
'<?php echo Foo\Bar\BAR;',
[3 => false, 5 => false, 7 => true],
],
[
'<?php echo FOO ? BAR : BAZ;',
[3 => true, 7 => true, 11 => true],
],
[
'<?php echo FOO & BAR | BAZ;',
[3 => true, 7 => true, 11 => true],
],
[
'<?php echo FOO & $bar;',
[3 => true],
],
[
'<?php echo FOO[BAR];',
[3 => true, 5 => true],
],
[
'<?php func(FOO, Bar\BAZ);',
[3 => true, 8 => true],
],
[
'<?php if (FOO && BAR) {}',
[4 => true, 8 => true],
],
[
'<?php return FOO * X\Y\BAR;',
[3 => true, 11 => true],
],
[
'<?php function x() { yield FOO; yield FOO => BAR; }',
[11 => true, 16 => true, 20 => true],
],
[
'<?php switch ($a) { case FOO: break; }',
[11 => true],
],
[
'<?php namespace FOO;',
[3 => false],
],
[
'<?php use FOO;',
[3 => false],
],
[
'<?php use function FOO\BAR\BAZ;',
[5 => false, 7 => false, 9 => false],
],
[
'<?php namespace X; const FOO = 1;',
[8 => false],
],
[
'<?php class FOO {}',
[3 => false],
],
[
'<?php interface FOO {}',
[3 => false],
],
[
'<?php trait FOO {}',
[3 => false],
],
[
'<?php class x extends FOO {}',
[7 => false],
],
[
'<?php class x implements FOO {}',
[7 => false],
],
[
'<?php class x implements FOO, BAR, BAZ {}',
[7 => false, 10 => false, 13 => false],
],
[
'<?php class x { const FOO = 1; }',
[9 => false],
],
[
'<?php class x { use FOO; }',
[9 => false],
],
[
'<?php class x { use FOO, BAR { FOO::BAZ insteadof BAR; } }',
[9 => false, 12 => false, 16 => false, 18 => false, 22 => false],
],
[
'<?php function x (FOO $foo, BAR &$bar, BAZ ...$baz) {}',
[6 => false, 11 => false, 17 => false],
],
[
'<?php FOO();',
[1 => false],
],
[
'<?php FOO::x();',
[1 => false],
],
[
'<?php x::FOO();',
[3 => false],
],
[
'<?php $foo instanceof FOO;',
[5 => false],
],
[
'<?php try {} catch (FOO $e) {}',
[9 => false],
],
[
'<?php FOO: goto FOO;',
[1 => false, 6 => false],
],
];
}

/**
* @param string $source
*
* @dataProvider provideIsConstantInvocation70Cases
* @requires PHP 7.0
*/
public function testIsConstantInvocation70($source, array $expected)
{
$tokensAnalyzer = new TokensAnalyzer(Tokens::fromCode($source));

foreach ($expected as $index => $expectedValue) {
$this->assertSame($expectedValue, $tokensAnalyzer->isConstantInvocation($index), 'Token at index '.$index.' should match the expected value.');
}
}

public function provideIsConstantInvocation70Cases()
{
return [
[
'<?php function x(): FOO {}',
[8 => false],
],
[
'<?php use X\Y\{FOO, BAR as BAR2, BAZ};',
[8 => false, 11 => false, 15 => false, 18 => false],
],
];
}

/**
* @param string $source
*
* @dataProvider provideIsConstantInvocation71Cases
* @requires PHP 7.1
*/
public function testIsConstantInvocation71($source, array $expected)
{
$tokensAnalyzer = new TokensAnalyzer(Tokens::fromCode($source));

foreach ($expected as $index => $expectedValue) {
$this->assertSame($expectedValue, $tokensAnalyzer->isConstantInvocation($index), 'Token at index '.$index.' should match the expected value.');
}
}

public function provideIsConstantInvocation71Cases()
{
return [
[
'<?php function x(?FOO $foo) {}',
[6 => false],
],
[
'<?php function x(): ?FOO {}',
[9 => false],
],
[
'<?php try {} catch (FOO|BAR|BAZ $e) {}',
[9 => false, 11 => false, 13 => false],
],
];
}

/**
* @param string $source
*
Expand Down

0 comments on commit ceec561

Please sign in to comment.