Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

NativeConstantInvocationFixer - better constant detection #3823

Merged
merged 1 commit into from Jul 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 9 additions & 13 deletions src/Fixer/ConstantNotation/NativeConstantInvocationFixer.php
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
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
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
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