From 692237d7ad70a9c215ab4c96a1ec4cb79c39dce4 Mon Sep 17 00:00:00 2001 From: Greg Sherwood Date: Mon, 6 Jan 2020 15:48:57 +1100 Subject: [PATCH] Fixed bug #2763 : PSR12 standard reports errors for multi-line FOR definitions This was done by adding a new ignoreNewlines property to Squiz.ControlStructures.ForLoopDeclaration so that the PSR12 standard could use it. In the case of newlines, PSR12 should ignore the statement and let other sniffs enforce the multi-line rules. But when there is no newline, it needs this sniff to enforce a single space. --- package.xml | 4 ++ src/Standards/PSR12/ruleset.xml | 6 +- .../ForLoopDeclarationSniff.php | 57 ++++++++++++++----- .../ForLoopDeclarationUnitTest.inc | 10 ++++ .../ForLoopDeclarationUnitTest.inc.fixed | 10 ++++ .../ForLoopDeclarationUnitTest.php | 2 +- 6 files changed, 72 insertions(+), 17 deletions(-) diff --git a/package.xml b/package.xml index 6a0ec97d21..08c5ec46e2 100644 --- a/package.xml +++ b/package.xml @@ -40,6 +40,9 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Thanks to Vincent Langlet for the patch - PSR12.Files.ImportStatement now auto-fixes import statements by removing the leading slash -- Thanks to Michał Bundyra for the patch + - Squiz.ControlStructures.ForLoopDeclaration now has a setting to ignore newline characters + -- Default remains FALSE, so newlines are not allowed within FOR definitions + -- Override the "ignoreNewlines" setting in a ruleset.xml file to change - Squiz.PHP.InnerFunctions now handles multiple nested anon classes correctly - Fixed bug #2688 : Case statements not tokenized correctly when switch is contained within ternary - Fixed bug #2698 : PHPCS throws errors determining auto report width when shell_exec is disabled @@ -48,6 +51,7 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Thanks to Michał Bundyra for the patch - Fixed bug #2751 : Autoload relative paths first to avoid confusion with files from the global include path -- Thanks to Klaus Purer for the patch + - Fixed bug #2763 : PSR12 standard reports errors for multi-line FOR definitions - Fixed bug #2768 : Generic.Files.LineLength false positive for non-breakable strings at exactly the soft limit -- Thanks to Alex Miles for the patch - Fixed bug #2791 : PSR12.Functions.NullableTypeDeclaration false positive when ternary operator used with instanceof diff --git a/src/Standards/PSR12/ruleset.xml b/src/Standards/PSR12/ruleset.xml index da6950c820..312f27fc70 100644 --- a/src/Standards/PSR12/ruleset.xml +++ b/src/Standards/PSR12/ruleset.xml @@ -228,7 +228,11 @@ - + + + + + 0 diff --git a/src/Standards/Squiz/Sniffs/ControlStructures/ForLoopDeclarationSniff.php b/src/Standards/Squiz/Sniffs/ControlStructures/ForLoopDeclarationSniff.php index 8f8a4b1b02..6803f9abb4 100644 --- a/src/Standards/Squiz/Sniffs/ControlStructures/ForLoopDeclarationSniff.php +++ b/src/Standards/Squiz/Sniffs/ControlStructures/ForLoopDeclarationSniff.php @@ -30,6 +30,13 @@ class ForLoopDeclarationSniff implements Sniff */ public $requiredSpacesBeforeClose = 0; + /** + * Allow newlines instead of spaces. + * + * @var boolean + */ + public $ignoreNewlines = false; + /** * A list of tokenizers this sniff supports. * @@ -77,20 +84,27 @@ public function process(File $phpcsFile, $stackPtr) $closingBracket = $tokens[$openingBracket]['parenthesis_closer']; - if ($this->requiredSpacesAfterOpen === 0 && $tokens[($openingBracket + 1)]['code'] === T_WHITESPACE) { - $error = 'Whitespace found after opening bracket of FOR loop'; - $fix = $phpcsFile->addFixableError($error, $openingBracket, 'SpacingAfterOpen'); - if ($fix === true) { - $phpcsFile->fixer->beginChangeset(); - for ($i = ($openingBracket + 1); $i < $closingBracket; $i++) { - if ($tokens[$i]['code'] !== T_WHITESPACE) { - break; + if ($this->requiredSpacesAfterOpen === 0 + && $tokens[($openingBracket + 1)]['code'] === T_WHITESPACE + ) { + $nextNonWhiteSpace = $phpcsFile->findNext(T_WHITESPACE, ($openingBracket + 1), $closingBracket, true); + if ($this->ignoreNewlines === false + || $tokens[$nextNonWhiteSpace]['line'] === $tokens[$openingBracket]['line'] + ) { + $error = 'Whitespace found after opening bracket of FOR loop'; + $fix = $phpcsFile->addFixableError($error, $openingBracket, 'SpacingAfterOpen'); + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + for ($i = ($openingBracket + 1); $i < $closingBracket; $i++) { + if ($tokens[$i]['code'] !== T_WHITESPACE) { + break; + } + + $phpcsFile->fixer->replaceToken($i, ''); } - $phpcsFile->fixer->replaceToken($i, ''); + $phpcsFile->fixer->endChangeset(); } - - $phpcsFile->fixer->endChangeset(); } } else if ($this->requiredSpacesAfterOpen > 0) { $nextNonWhiteSpace = $phpcsFile->findNext(T_WHITESPACE, ($openingBracket + 1), $closingBracket, true); @@ -101,7 +115,10 @@ public function process(File $phpcsFile, $stackPtr) $spaceAfterOpen = $tokens[($openingBracket + 1)]['length']; } - if ($spaceAfterOpen !== $this->requiredSpacesAfterOpen) { + if ($spaceAfterOpen !== $this->requiredSpacesAfterOpen + && ($this->ignoreNewlines === false + || $spaceAfterOpen !== 'newline') + ) { $error = 'Expected %s spaces after opening bracket; %s found'; $data = [ $this->requiredSpacesAfterOpen, @@ -133,7 +150,11 @@ public function process(File $phpcsFile, $stackPtr) $beforeClosefixable = false; } - if ($this->requiredSpacesBeforeClose === 0 && $tokens[($closingBracket - 1)]['code'] === T_WHITESPACE) { + if ($this->requiredSpacesBeforeClose === 0 + && $tokens[($closingBracket - 1)]['code'] === T_WHITESPACE + && ($this->ignoreNewlines === false + || $tokens[$prevNonWhiteSpace]['line'] === $tokens[$closingBracket]['line']) + ) { $error = 'Whitespace found before closing bracket of FOR loop'; if ($beforeClosefixable === false) { @@ -161,7 +182,10 @@ public function process(File $phpcsFile, $stackPtr) $spaceBeforeClose = $tokens[($closingBracket - 1)]['length']; } - if ($this->requiredSpacesBeforeClose !== $spaceBeforeClose) { + if ($this->requiredSpacesBeforeClose !== $spaceBeforeClose + && ($this->ignoreNewlines === false + || $spaceBeforeClose !== 'newline') + ) { $error = 'Expected %s spaces before closing bracket; %s found'; $data = [ $this->requiredSpacesBeforeClose, @@ -264,7 +288,10 @@ public function process(File $phpcsFile, $stackPtr) $spaces = 'newline'; } - if ($spaces !== 1) { + if ($spaces !== 1 + && ($this->ignoreNewlines === false + || $spaces !== 'newline') + ) { $error = 'Expected 1 space after %s semicolon of FOR loop; %s found'; $errorCode = 'SpacingAfter'.$humanReadableCode; $data[] = $spaces; diff --git a/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc b/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc index 9875966b04..5022e74c9a 100644 --- a/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc +++ b/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc @@ -115,5 +115,15 @@ for ( for ($i = function() { return $this->i ; }; $i < function() { return $this->max; }; $i++) {} for ($i = function() { return $this->i; }; $i < function() { return $this->max; } ; $i++) {} +// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines true +for ( + $i = 0; + $i < 5; + $i++ +) { + // body here +} +// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines false + // This test has to be the last one in the file! Intentional parse error check. for diff --git a/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc.fixed b/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc.fixed index ad5040c37e..6a1e7634bd 100644 --- a/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.inc.fixed @@ -81,5 +81,15 @@ for ( $i = 0; $i < 10; $i++ ) {} for ($i = function() { return $this->i ; }; $i < function() { return $this->max; }; $i++) {} for ($i = function() { return $this->i; }; $i < function() { return $this->max; }; $i++) {} +// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines true +for ( + $i = 0; + $i < 5; + $i++ +) { + // body here +} +// phpcs:set Squiz.ControlStructures.ForLoopDeclaration ignoreNewlines false + // This test has to be the last one in the file! Intentional parse error check. for diff --git a/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.php b/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.php index 8358a82466..51638866ed 100644 --- a/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.php +++ b/src/Standards/Squiz/Tests/ControlStructures/ForLoopDeclarationUnitTest.php @@ -117,7 +117,7 @@ public function getWarningList($testFile='ForLoopDeclarationUnitTest.inc') { switch ($testFile) { case 'ForLoopDeclarationUnitTest.inc': - return [119 => 1]; + return [129 => 1]; case 'ForLoopDeclarationUnitTest.js': return [125 => 1];