Skip to content

Commit

Permalink
Merge branch '2.15' into 2.16
Browse files Browse the repository at this point in the history 
* 2.15:
  Fix unexpected removal of regular comments
  YodaStyle - rewrite same logic for more easy read YodaStyle - statements in braces should be treated as variables in strict mode
  • Loading branch information
@SpacePossum
SpacePossum committed Oct 26, 2020
2 parents 2a02223 + 0cca8a5 commit f446a35
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 43 deletions.
19 changes: 15 additions & 4 deletions src/Fixer/Comment/HeaderCommentFixer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,17 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
}

$this->insertHeader($tokens, $headerNewIndex);
} elseif ($this->getHeaderAsComment() !== $tokens[$headerCurrentIndex]->getContent() || $possibleLocation !== $location) {
$this->removeHeader($tokens, $headerCurrentIndex);

continue;
}

$sameComment = $this->getHeaderAsComment() === $tokens[$headerCurrentIndex]->getContent();
$expectedLocation = $possibleLocation === $location;

if (!$sameComment || !$expectedLocation) {
if ($expectedLocation ^ $sameComment) {
$this->removeHeader($tokens, $headerCurrentIndex);
}

if ('' === $this->configuration['header']) {
continue;
Expand All @@ -164,9 +173,11 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens)
if ($possibleLocation === $location) {
$this->insertHeader($tokens, $headerNewIndex);
}
} else {
$this->fixWhiteSpaceAroundHeader($tokens, $headerCurrentIndex);

continue;
}

$this->fixWhiteSpaceAroundHeader($tokens, $headerCurrentIndex);
}
}

Expand Down
58 changes: 25 additions & 33 deletions src/Fixer/ControlStructure/YodaStyleFixer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,40 +359,32 @@ private function getCompareFixableInfo(Tokens $tokens, $index, $yoda)
$left = $this->getLeftSideCompareFixableInfo($tokens, $index);
$right = $this->getRightSideCompareFixableInfo($tokens, $index);

if ($yoda) {
$expectedAssignableSide = $right;
$expectedValueSide = $left;
} else {
if ($tokens[$tokens->getNextMeaningfulToken($right['end'])]->equals('=')) {
return null;
}
if (!$yoda && $tokens[$tokens->getNextMeaningfulToken($right['end'])]->equals('=')) {
return null;
}

$expectedAssignableSide = $left;
$expectedValueSide = $right;
if ($this->isListStatement($tokens, $left['start'], $left['end']) || $this->isListStatement($tokens, $right['start'], $right['end'])) {
return null; // do not fix lists assignment inside statements
}

if (
// variable cannot be moved to expected side
!(
!$this->isVariable($tokens, $expectedAssignableSide['start'], $expectedAssignableSide['end'], false)
&& !$this->isListStatement($tokens, $expectedAssignableSide['start'], $expectedAssignableSide['end'])
&& $this->isVariable($tokens, $expectedValueSide['start'], $expectedValueSide['end'], false)
)
// variable cannot be moved to expected side (strict mode)
&& !(
$this->configuration['always_move_variable']
&& !$this->isVariable($tokens, $expectedAssignableSide['start'], $expectedAssignableSide['end'], true)
&& !$this->isListStatement($tokens, $expectedAssignableSide['start'], $expectedAssignableSide['end'])
&& $this->isVariable($tokens, $expectedValueSide['start'], $expectedValueSide['end'], true)
)
) {
return null;
$strict = $this->configuration['always_move_variable'];

$leftSideIsVariable = $this->isVariable($tokens, $left['start'], $left['end'], $strict);
$rightSideIsVariable = $this->isVariable($tokens, $right['start'], $right['end'], $strict);

if (!($leftSideIsVariable ^ $rightSideIsVariable)) {
return null; // both are (not) variables, do not touch
}

return [
'left' => $left,
'right' => $right,
];
if (!$strict) { // special handling for braces with not "always_move_variable"
$leftSideIsVariable = $leftSideIsVariable && !$tokens[$left['start']]->equals('(');
$rightSideIsVariable = $rightSideIsVariable && !$tokens[$right['start']]->equals('(');
}

return ($yoda && !$leftSideIsVariable) || (!$yoda && !$rightSideIsVariable)
? null
: ['left' => $left, 'right' => $right]
;
}

/**
Expand Down Expand Up @@ -522,11 +514,11 @@ private function isVariable(Tokens $tokens, $start, $end, $strict)
return $tokens[$start]->isGivenKind(T_VARIABLE);
}

if ($strict) {
if ($tokens[$start]->equals('(')) {
return false;
}
if ($tokens[$start]->equals('(')) {
return true;
}

if ($strict) {
for ($index = $start; $index <= $end; ++$index) {
if (
$tokens[$index]->isCast()
Expand Down
70 changes: 70 additions & 0 deletions tests/Fixer/Comment/HeaderCommentFixerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,9 @@ public function provideFixCases()
*/
declare(strict_types=1);
/**
* bar
*/
namespace A;
Expand Down Expand Up @@ -477,6 +480,73 @@ class Foo {}',
*/
class Foo {}',
],
[
[
'header' => 'bar',
'location' => 'after_open',
],
'<?php
/*
* bar
*/
declare(strict_types=1);
// foo
foo();',
'<?php
/*
* foo
*/
declare(strict_types=1);
// foo
foo();',
],
[
[
'header' => 'bar',
'location' => 'after_open',
],
'<?php
/*
* bar
*/
declare(strict_types=1);
/* foo */
foo();',
'<?php
/*
* foo
*/
declare(strict_types=1);
/* foo */
foo();',
],
[
[
'header' => 'tmp',
'location' => 'after_declare_strict',
],
'<?php
/*
* tmp
*/
declare(strict_types=1) ?>',
'<?php
declare(strict_types=1) ?>',
],
];
}

Expand Down
56 changes: 50 additions & 6 deletions tests/Fixer/ControlStructure/YodaStyleFixerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,43 @@ public function testFixInverse($expected, $input = null, array $extraConfig = []
public function provideFixCases()
{
return [
[
'<?php $a = ($b + $c) || 1 === true ? 1 : 2;',
null,
['always_move_variable' => true],
],
[
'<?php $a = 1 + ($b + $c) === true ? 1 : 2;',
null,
['always_move_variable' => true],
],
[
'<?php $a = true === ($b + $c) ? 1 : 2;',
'<?php $a = ($b + $c) === true ? 1 : 2;',
['always_move_variable' => true],
],
[
'<?php
if ((1 === $a) === 1) {
return;
}',
'<?php
if (($a === 1) === 1) {
return;
}',
['always_move_variable' => false],
],
[
'<?php
if (true === (1 !== $foo[0])) {
return;
}',
'<?php
if (($foo[0] !== 1) === true) {
return;
}',
['always_move_variable' => true],
],
[
'<?php return 1 !== $a [$b];',
'<?php return $a [$b] !== 1;',
Expand All @@ -81,8 +118,8 @@ public function provideFixCases()
',
],
[
'<?php 1 === lala($a) ? 1 : 2;',
'<?php lala($a) === 1 ? 1 : 2;',
'<?php 1 === foo($a) ? 1 : 2;',
'<?php foo($a) === 1 ? 1 : 2;',
],
[
'<?php 1 === $a::$a ? 1 : 2;',
Expand Down Expand Up @@ -200,8 +237,8 @@ public function provideFixCases()
['<?php $c = $$b === $$c;'],
['<?php $d = count($this->array[$var]) === $a;'],
['<?php $e = $a === count($this->array[$var]);'],
['<?php $f = ($a & self::MY_BITMASK) === $a;'],
['<?php $g = $a === ($a & self::MY_BITMASK);'],
['<?php $f = ($a123 & self::MY_BITMASK) === $a;'],
['<?php $g = $a === ($a456 & self::MY_BITMASK);'],
['<?php $h = $this->getStuff() === $myVariable;'],
['<?php $i = $myVariable === $this->getStuff();'],
['<?php $j = 2 * $myVar % 3 === $a;'],
Expand Down Expand Up @@ -247,9 +284,16 @@ public function provideFixCases()
# aa
/**/==/**/2/***/;',
],
[
'<?php return 2 == ($a)?>',
],
[
'<?php return ($a) == 2?>',
],
[
'<?php return 2 == ($a)?>',
'<?php return ($a) == 2?>',
['always_move_variable' => true],
],
[
'<?php $a = ($c === ((null === $b)));',
Expand Down Expand Up @@ -422,8 +466,8 @@ function foo() {
['always_move_variable' => true],
],
[
'<?php $g = ($a & self::MY_BITMASK) === $a;',
'<?php $g = $a === ($a & self::MY_BITMASK);',
'<?php $g = ($a789 & self::MY_BITMASK) === $a;',
null,
['always_move_variable' => true],
],
[
Expand Down

0 comments on commit f446a35

Please sign in to comment.