diff --git a/package.xml b/package.xml index b621f14dc4..3a00f7371a 100644 --- a/package.xml +++ b/package.xml @@ -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 diff --git a/src/Standards/Generic/Sniffs/Formatting/MultipleStatementAlignmentSniff.php b/src/Standards/Generic/Sniffs/Formatting/MultipleStatementAlignmentSniff.php index 36c16c84dc..e2e807dc99 100644 --- a/src/Standards/Generic/Sniffs/Formatting/MultipleStatementAlignmentSniff.php +++ b/src/Standards/Generic/Sniffs/Formatting/MultipleStatementAlignmentSniff.php @@ -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; } @@ -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 @@ -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); @@ -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) { diff --git a/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc b/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc index fc917f3f77..b02013dfaf 100644 --- a/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc +++ b/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc @@ -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; diff --git a/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc.fixed b/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc.fixed index 3c904b243f..25c5792b36 100644 --- a/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc.fixed +++ b/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.inc.fixed @@ -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; diff --git a/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.php b/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.php index 1959596c96..79eb18e903 100644 --- a/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.php +++ b/src/Standards/Generic/Tests/Formatting/MultipleStatementAlignmentUnitTest.php @@ -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':