Skip to content

Commit

Permalink
bug #4022 NoUnsetOnPropertyFixer - refactor and bugfixes (kubawerlos)
Browse files Browse the repository at this point in the history 
This PR was squashed before being merged into the 2.12 branch (closes #4022).

Discussion
----------

NoUnsetOnPropertyFixer - refactor and bugfixes

Replaces #4020
Fix #3843

Commits
-------

e43137a NoUnsetOnPropertyFixer - refactor and bugfixes
  • Loading branch information
@keradus
keradus committed Dec 30, 2018
2 parents e5f472f + e43137a commit e147eab
Show file tree
Hide file tree
Showing 2 changed files with 152 additions and 104 deletions.
219 changes: 117 additions & 102 deletions src/Fixer/LanguageConstruct/NoUnsetOnPropertyFixer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use PhpCsFixer\AbstractFixer;
use PhpCsFixer\FixerDefinition\CodeSample;
use PhpCsFixer\FixerDefinition\FixerDefinition;
use PhpCsFixer\Tokenizer\CT;
use PhpCsFixer\Tokenizer\Analyzer\ArgumentsAnalyzer;
use PhpCsFixer\Tokenizer\Token;
use PhpCsFixer\Tokenizer\Tokens;

Expand Down Expand Up @@ -71,142 +71,157 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
continue;
}

$unsetStart = $tokens->getNextTokenOfKind($index, ['(']);
$unsetEnd = $tokens->findBlockEnd(
Tokens::BLOCK_TYPE_PARENTHESIS_BRACE,
$unsetStart
);
$closingSemiColon = $tokens->getNextTokenOfKind($unsetEnd, [';']);

$unsetTokensSubset = $this->createTokensSubSet($tokens, $index, $unsetEnd);
$unsetOpening = $unsetTokensSubset->getNextTokenOfKind(0, ['(']);
$unsetsInfo = $this->getUnsetsInfo($tokens, $index);

//We want to remove the `unset` and the `(`
$unsetTokensSubset->clearAt(0);
$unsetTokensSubset->clearAt($unsetOpening);
$unsetTokensSubset->clearEmptyTokens();

if (!$this->doTokensInvolveProperty($unsetTokensSubset)) {
if (!$this->isAnyUnsetToTransform($unsetsInfo)) {
continue;
}

$this->updateUnset($unsetTokensSubset);

$tokens->overrideRange($index, $closingSemiColon, $unsetTokensSubset);
$isLastUnset = true; // yes, last - we reverse the array below
foreach (array_reverse($unsetsInfo) as $unsetInfo) {
$this->updateTokens($tokens, $unsetInfo, $isLastUnset);
$isLastUnset = false;
}
}
}

/**
* Attempts to change unset into is null where possible.
*
* @param Tokens $tokens
* @param int $index
*
* @return string[]
*/
private function updateUnset(Tokens $tokens)
private function getUnsetsInfo(Tokens $tokens, $index)
{
$index = 0;
$atLastUnset = false;
do {
$next = $tokens->getNextTokenOfKind($index, [',']);
if (null === $next) {
$atLastUnset = true;
$next = $tokens->count() - 1;
$nextTokenSet = $this->createTokensSubSet($tokens, $index, $next + 1);
} else {
$nextTokenSet = $this->createTokensSubSet($tokens, $index, $next);
}

if ($this->doTokensInvolveProperty($nextTokenSet)
&& !$this->doTokensInvolveArrayAccess($nextTokenSet)
) {
$replacement = $this->createReplacementIsNull($nextTokenSet);
} else {
$replacement = $this->createReplacementUnset($nextTokenSet);
}
$argumentsAnalyzer = new ArgumentsAnalyzer();

$unsetStart = $tokens->getNextTokenOfKind($index, ['(']);
$unsetEnd = $tokens->findBlockEnd(Tokens::BLOCK_TYPE_PARENTHESIS_BRACE, $unsetStart);
$isFirst = true;

$unsets = [];
foreach ($argumentsAnalyzer->getArguments($tokens, $unsetStart, $unsetEnd) as $startIndex => $endIndex) {
$startIndex = $tokens->getNextMeaningfulToken($startIndex - 1);
$endIndex = $tokens->getPrevMeaningfulToken($endIndex + 1);
$unsets[] = [
'startIndex' => $startIndex,
'endIndex' => $endIndex,
'isToTransform' => $this->isProperty($tokens, $startIndex, $endIndex),
'isFirst' => $isFirst,
];
$isFirst = false;
}

$tokens->overrideRange($index, $next, $replacement);
$index = $tokens->getNextTokenOfKind($index, [';']) + 1;
} while (!$atLastUnset);
return $unsets;
}

/**
* @param Tokens $filteredTokens
* @param Tokens $tokens
* @param int $index
* @param int $endIndex
*
* @return Token[]
* @return bool
*/
private function createReplacementIsNull(Tokens $filteredTokens)
private function isProperty(Tokens $tokens, $index, $endIndex)
{
return array_merge(
$filteredTokens->toArray(),
[
new Token([T_WHITESPACE, ' ']),
new Token('='),
new Token([T_WHITESPACE, ' ']),
new Token([T_STRING, 'null']),
new Token(';'),
]
);
}
if ($tokens[$index]->isGivenKind(T_VARIABLE)) {
$nextIndex = $tokens->getNextMeaningfulToken($index);
if (null === $nextIndex || !$tokens[$nextIndex]->isGivenKind(T_OBJECT_OPERATOR)) {
return false;
}
$nextIndex = $tokens->getNextMeaningfulToken($nextIndex);
$nextNextIndex = $tokens->getNextMeaningfulToken($nextIndex);
if (null !== $nextNextIndex && $nextNextIndex < $endIndex) {
return false;
}

/**
* @param Tokens $filteredTokens
*
* @return Token[]
*/
private function createReplacementUnset(Tokens $filteredTokens)
{
if ($filteredTokens[0]->isWhitespace()) {
$filteredTokens->clearAt(0);
return null !== $nextIndex && $tokens[$nextIndex]->isGivenKind(T_STRING);
}

$unsetOpeningTokens = [];
if ($filteredTokens[0]->isWhitespace()) {
$unsetOpeningTokens[] = new Token([T_WHITESPACE, ' ']);
}
if ($tokens[$index]->isGivenKind([T_NS_SEPARATOR, T_STRING])) {
$nextIndex = $tokens->getTokenNotOfKindSibling($index, 1, [[T_DOUBLE_COLON], [T_NS_SEPARATOR], [T_STRING]]);
$nextNextIndex = $tokens->getNextMeaningfulToken($nextIndex);
if (null !== $nextNextIndex && $nextNextIndex < $endIndex) {
return false;
}

array_push($unsetOpeningTokens, new Token([T_UNSET, 'unset']), new Token('('));
return null !== $nextIndex && $tokens[$nextIndex]->isGivenKind(T_VARIABLE);
}

return array_merge(
$unsetOpeningTokens,
$filteredTokens->toArray(),
[
new Token(')'),
new Token(';'),
]
);
return false;
}

/**
* @param Tokens $tokens
* @param int $startIndex
* @param int $endIndex
* @param string[] $unsetsInfo
*
* @return Tokens
* @return bool
*/
private function createTokensSubSet(Tokens $tokens, $startIndex, $endIndex)
private function isAnyUnsetToTransform(array $unsetsInfo)
{
$array = $tokens->toArray();
$toAnalyze = array_splice($array, $startIndex, $endIndex - $startIndex);
foreach ($unsetsInfo as $unsetInfo) {
if ($unsetInfo['isToTransform']) {
return true;
}
}

return Tokens::fromArray($toAnalyze);
return false;
}

/**
* @param Tokens $tokens
*
* @return bool
* @param Tokens $tokens
* @param string[] $unsetInfo
* @param bool $isLastUnset
*/
private function doTokensInvolveProperty(Tokens $tokens)
private function updateTokens(Tokens $tokens, array $unsetInfo, $isLastUnset)
{
return $tokens->isAnyTokenKindsFound([T_PAAMAYIM_NEKUDOTAYIM, T_OBJECT_OPERATOR]);
}
// if entry is first and to be transform we remove leading "unset("
if ($unsetInfo['isFirst'] && $unsetInfo['isToTransform']) {
$braceIndex = $tokens->getPrevTokenOfKind($unsetInfo['startIndex'], ['(']);
$unsetIndex = $tokens->getPrevTokenOfKind($braceIndex, [[T_UNSET]]);
$tokens->clearTokenAndMergeSurroundingWhitespace($braceIndex);
$tokens->clearTokenAndMergeSurroundingWhitespace($unsetIndex);
}

/**
* @param Tokens $tokens
*
* @return bool
*/
private function doTokensInvolveArrayAccess(Tokens $tokens)
{
return $tokens->isAnyTokenKindsFound(['[', CT::T_ARRAY_INDEX_CURLY_BRACE_OPEN]);
// if entry is last and to be transformed we remove trailing ")"
if ($isLastUnset && $unsetInfo['isToTransform']) {
$braceIndex = $tokens->getNextTokenOfKind($unsetInfo['endIndex'], [')']);
$tokens->clearTokenAndMergeSurroundingWhitespace($braceIndex);
}

// if entry is not last we replace comma with semicolon (last entry already has semicolon - from original unset)
if (!$isLastUnset) {
$commaIndex = $tokens->getNextTokenOfKind($unsetInfo['endIndex'], [',']);
$tokens[$commaIndex] = new Token(';');
}

// if entry is to be unset and is not last we add trailing ")"
if (!$unsetInfo['isToTransform'] && !$isLastUnset) {
$tokens->insertAt($unsetInfo['endIndex'] + 1, new Token(')'));
}

// if entry is to be unset and is not first we add leading "unset("
if (!$unsetInfo['isToTransform'] && !$unsetInfo['isFirst']) {
$tokens->insertAt(
$unsetInfo['startIndex'],
[
new Token([T_UNSET, 'unset']),
new Token('('),
]
);
}

// and finally
// if entry is to be transformed we add trailing " = null"
if ($unsetInfo['isToTransform']) {
$tokens->insertAt(
$unsetInfo['endIndex'] + 1,
[
new Token([T_WHITESPACE, ' ']),
new Token('='),
new Token([T_WHITESPACE, ' ']),
new Token([T_STRING, 'null']),
]
);
}
}
}
37 changes: 35 additions & 2 deletions tests/Fixer/LanguageConstruct/NoUnsetOnPropertyFixerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public function provideFixCases()
'It does not replace unsets on arrays' => [
'<?php unset($bar->foo[0]);',
],
'It works in a more complex unset ' => [
'It works in a more complex unset' => [
'<?php unset($bar->foo[0]); self::$foo = null; \Test\Baz::$fooBar = null; unset($bar->foo[0]); $this->foo = null; unset($a); unset($b);',
'<?php unset($bar->foo[0], self::$foo, \Test\Baz::$fooBar, $bar->foo[0], $this->foo, $a, $b);',
],
Expand All @@ -89,7 +89,7 @@ public function provideFixCases()
',
],
'It works with weirdly placed comments' => [
'<?php unset(/*foo*//*bar*/$bar->foo[0]); self::$foo/*baz*/ = null; /*ello*/\Test\Baz::$fooBar/*comment*/ = null; unset($bar->foo[0]); $this->foo = null; unset($a); unset($b);
'<?php unset/*foo*/(/*bar*/$bar->foo[0]); self::$foo = null/*baz*/; /*ello*/\Test\Baz::$fooBar = null/*comment*/; unset($bar->foo[0]); $this->foo = null; unset($a); unset($b);
unset/*foo*/(/*bar*/$bar);',
'<?php unset/*foo*/(/*bar*/$bar->foo[0], self::$foo/*baz*/, /*ello*/\Test\Baz::$fooBar/*comment*/, $bar->foo[0], $this->foo, $a, $b);
unset/*foo*/(/*bar*/$bar);',
Expand All @@ -100,6 +100,39 @@ public function provideFixCases()
'<?php unset($a, $b, $c);
unset($this->a);',
],
'It does not replace function call with class constant inside' => [
'<?php unset($foos[array_search(BadFoo::NAME, $foos)]);',
],
'It does not replace function call with class constant and property inside' => [
'<?php unset($this->property[array_search(\Types::TYPE_RANDOM, $this->property)]);',
],
];
}

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

public function provideFix70Cases()
{
return [
'It does not break complex expressions' => [
'<?php
unset(a()[b()["a"]]);
unset(a()[b()]);
unset(a()["a"]);
unset(a(){"a"});
unset(c($a)->a);
',
],
];
}
}

0 comments on commit e147eab

Please sign in to comment.