Skip to content

Commit

Permalink
Fixed bug #1925 : Generic.Formatting.MultipleStatementAlignment skipp…
Browse files Browse the repository at this point in the history 
…ing assignments within closures

I had to revert the patch to fix #1922 because it wouldn't allow for the sniff to do recursion, which is what it now does when it finds a closure or anon class.
  • Loading branch information
@gsherwood
gsherwood committed Mar 7, 2018
1 parent 281e44b commit 8f1ac94
Show file tree
Hide file tree
Showing 5 changed files with 237 additions and 6 deletions.
2 changes: 1 addition & 1 deletion package.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
-- Thanks to Juliette Reinders Folmer for the patch
- Fixed bug #1915 : JS tokenizer fails to tokenize regular expression proceeded by boolean not operator
- Fixed bug #1922 : Equal sign alignment check broken when list syntax used before assignment operator
-- Thanks to Juliette Reinders Folmer for the patch
- Fixed bug #1925 : Generic.Formatting.MultipleStatementAlignment skipping assignments within closures
</notes>
<contents>
<dir name="/">
Expand Down
74 changes: 69 additions & 5 deletions src/Standards/Generic/Sniffs/Formatting/MultipleStatementAlignmentSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,68 @@ public function checkAlignment($phpcsFile, $stackPtr)

for ($assign = $stackPtr; $assign < $phpcsFile->numTokens; $assign++) {
if (isset($find[$tokens[$assign]['code']]) === false) {
// A blank line indicates that the assignment block has ended.
if (isset(Tokens::$emptyTokens[$tokens[$assign]['code']]) === false
&& ($tokens[$assign]['line'] - $tokens[$lastCode]['line']) > 1
) {
break;
}

if (($tokens[$assign]['code'] === T_CLOSURE
|| $tokens[$assign]['code'] === T_ANON_CLASS)
&& isset($tokens[$assign]['scope_closer']) === true
) {
$start = $tokens[$assign]['scope_opener'];
$end = $tokens[$assign]['scope_closer'];

$innerFind = $find;

$innerFind[T_CLOSURE] = T_CLOSURE;
$innerFind[T_ANON_CLASS] = T_ANON_CLASS;

do {
$innerAssign = $phpcsFile->findNext($innerFind, ($start + 1), $end);
if ($innerAssign !== false) {
// There is an assignment block inside the closure/class
// that we would normally skip over, so process it now.
$start = $this->checkAlignment($phpcsFile, $innerAssign);
if ($start === $innerAssign
&& ($tokens[$innerAssign]['code'] === T_CLOSURE
|| $tokens[$innerAssign]['code'] === T_ANON_CLASS)
) {
// No assignments were found, so skip over the block.
$start = $tokens[$innerAssign]['scope_closer'];
}
}
} while ($innerAssign !== false);

$assign = $end;
$lastCode = $assign;
continue;
}//end if

// Skip past the content of arrays.
if ($tokens[$assign]['code'] === T_OPEN_SHORT_ARRAY
&& isset($tokens[$assign]['bracket_closer']) === true
) {
$assign = $tokens[$assign]['bracket_closer'];
$lastCode = $assign;
continue;
}

if ($tokens[$assign]['code'] === T_ARRAY
&& isset($tokens[$assign]['parenthesis_opener']) === true
&& isset($tokens[$tokens[$assign]['parenthesis_opener']]['parenthesis_closer']) === true
) {
$assign = $tokens[$tokens[$assign]['parenthesis_opener']]['parenthesis_closer'];
$lastCode = $assign;
continue;
}

// A blank line indicates that the assignment block has ended.
if (isset(Tokens::$emptyTokens[$tokens[$assign]['code']]) === false) {
if (($tokens[$assign]['line'] - $tokens[$lastCode]['line']) > 1) {
if ($prevAssign === null) {
// Processing an inner block but no assignments found.
break;
}

Expand All @@ -139,6 +198,10 @@ public function checkAlignment($phpcsFile, $stackPtr)
}
}//end if

continue;
} else if ($assign !== $stackPtr && $tokens[$assign]['line'] === $lastLine) {
// Skip multiple assignments on the same line. We only need to
// try and align the first assignment.
continue;
}//end if

Expand Down Expand Up @@ -171,6 +234,11 @@ public function checkAlignment($phpcsFile, $stackPtr)
$varEnd = $tokens[($var + 1)]['column'];
$assignLen = $tokens[$assign]['length'];
if ($assign !== $stackPtr) {
if ($prevAssign === null) {
// Processing an inner block but no assignments found.
break;
}

if (($varEnd + 1) > $assignments[$prevAssign]['assign_col']) {
$padding = 1;
$assignColumn = ($varEnd + 1);
Expand Down Expand Up @@ -236,10 +304,6 @@ public function checkAlignment($phpcsFile, $stackPtr)

$lastLine = $tokens[$assign]['line'];
$prevAssign = $assign;

// Skip past the value assignment.
$assign = ($phpcsFile->findEndOfStatement($assign) - 1);
$lastCode = $assign;
}//end for

if (empty($assignments) === true) {
Expand Down
72 changes: 72 additions & 0 deletions src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,3 +281,75 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$types = ['support.contact.question' => ContactData::QUESTION];
[$important, $questions, $incidents, $requests] = $this->createContractBuckets($options['contracts']);
}

$loggerResult = $util->setLogger(new class {
public function log($msg)
{
$a = $msg;
$foobar = $msg;
$foo = function() {
$a = $msg;
$foobar = $msg;
$loggerResult = $util->setLogger(new class {
public function log($msg)
{
$a = $msg;
$foobar = $msg;
$foo = function() {
foo(function() {
foo(function() {
echo 'hi';
});
$a = $msg;
$foobar = $msg;

$foo = function() {

$foo = 1;
$barbar=2;
}
$barbar = function() {
$foo = 1;
$barbar = 2;
}
});
$a = $msg;
$foobar = $msg;
}
$bar = $msg;
}

public function log2($msg)
{
$a = $msg;
$foobar = $msg;
$foo = function() {
foo(function() {
foo(function() {
echo 'hi';
});
$a = $msg;
$foobar = $msg;

$foo = function() {

$foo = 1;
$barbar=2;
}
$barbar = function() {
$foo = 1;
$barbar = 2;
}
});
$a = $msg;
$foobar = $msg;
}
$bar = $msg;
}
});
$foo = 5;
}
$bar = $msg;
}
});
$foo = 5;
72 changes: 72 additions & 0 deletions src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc.fixed
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,3 +281,75 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$types = ['support.contact.question' => ContactData::QUESTION];
[$important, $questions, $incidents, $requests] = $this->createContractBuckets($options['contracts']);
}

$loggerResult = $util->setLogger(new class {
public function log($msg)
{
$a = $msg;
$foobar = $msg;
$foo = function() {
$a = $msg;
$foobar = $msg;
$loggerResult = $util->setLogger(new class {
public function log($msg)
{
$a = $msg;
$foobar = $msg;
$foo = function() {
foo(function() {
foo(function() {
echo 'hi';
});
$a = $msg;
$foobar = $msg;

$foo = function() {

$foo = 1;
$barbar =2;
}
$barbar = function() {
$foo = 1;
$barbar = 2;
}
});
$a = $msg;
$foobar = $msg;
}
$bar = $msg;
}

public function log2($msg)
{
$a = $msg;
$foobar = $msg;
$foo = function() {
foo(function() {
foo(function() {
echo 'hi';
});
$a = $msg;
$foobar = $msg;

$foo = function() {

$foo = 1;
$barbar =2;
}
$barbar = function() {
$foo = 1;
$barbar = 2;
}
});
$a = $msg;
$foobar = $msg;
}
$bar = $msg;
}
});
$foo = 5;
}
$bar = $msg;
}
});
$foo = 5;
23 changes: 23 additions & 0 deletions src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,29 @@ public function getWarningList($testFile='MultipleStatementAlignmentUnitTest.inc
252 => 1,
257 => 1,
263 => 1,
288 => 1,
290 => 1,
291 => 1,
292 => 1,
296 => 1,
298 => 1,
303 => 1,
306 => 1,
308 => 1,
309 => 1,
316 => 1,
317 => 1,
319 => 1,
324 => 1,
326 => 1,
331 => 1,
334 => 1,
336 => 1,
337 => 1,
344 => 1,
345 => 1,
347 => 1,
352 => 1,
];
break;
case 'MultipleStatementAlignmentUnitTest.js':
Expand Down

0 comments on commit 8f1ac94

Please sign in to comment.