diff --git a/src/Sniffs/AbstractPatternSniff.php b/src/Sniffs/AbstractPatternSniff.php index 30a3ce7355..8de71df29b 100644 --- a/src/Sniffs/AbstractPatternSniff.php +++ b/src/Sniffs/AbstractPatternSniff.php @@ -915,12 +915,12 @@ private function createTokenPattern($str) // Don't add a space after the closing php tag as it will add a new // whitespace token. - $tokenizer = new PHP('', null); + $tokenizer = new PHP('', null); StatusWriter::resume(); // Remove the getTokens(); - $tokens = array_slice($tokens, 1, (count($tokens) - 2)); + $tokens = array_slice($tokens, 2, (count($tokens) - 3)); $patterns = []; foreach ($tokens as $patternInfo) { diff --git a/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php b/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php index 6fbfdc0b2b..7b13a9cf58 100644 --- a/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php +++ b/src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php @@ -113,19 +113,15 @@ private function processSemicolon(File $phpcsFile, $stackPtr) if ($fix === true) { $phpcsFile->fixer->beginChangeset(); - if ($tokens[$prevNonEmpty]['code'] === T_OPEN_TAG - || $tokens[$prevNonEmpty]['code'] === T_OPEN_TAG_WITH_ECHO - ) { - // Check for superfluous whitespace after the semicolon which should be - // removed as the `fixer->replaceToken(($stackPtr + 1), $replacement); - } + // Make sure there always remains one space between the open tag and the next content. + $replacement = ' '; + if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) { + $replacement = ''; } - for ($i = $stackPtr; $i > $prevNonEmpty; $i--) { + $phpcsFile->fixer->replaceToken($stackPtr, $replacement); + + for ($i = ($stackPtr - 1); $i > $prevNonEmpty; $i--) { if ($tokens[$i]['code'] !== T_SEMICOLON && $tokens[$i]['code'] !== T_WHITESPACE ) { diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc index a9f895d71b..acf860a2ba 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc @@ -1,5 +1,5 @@ - + diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed index fd27d0a25c..434bd53a0c 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed @@ -1,6 +1,6 @@ - - + + /* diff --git a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php index 2d159cfadc..e68d8aa7b7 100644 --- a/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php +++ b/src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php @@ -93,6 +93,7 @@ public function getWarningList($testFile='') ]; case 'EmptyPHPStatementUnitTest.2.inc': return [ + 2 => 1, 3 => 1, 4 => 1, 13 => 1, diff --git a/src/Standards/PSR12/Sniffs/Files/OpenTagSniff.php b/src/Standards/PSR12/Sniffs/Files/OpenTagSniff.php index d0ac5d363d..f8863ffab2 100644 --- a/src/Standards/PSR12/Sniffs/Files/OpenTagSniff.php +++ b/src/Standards/PSR12/Sniffs/Files/OpenTagSniff.php @@ -56,8 +56,8 @@ public function process(File $phpcsFile, $stackPtr) return $phpcsFile->numTokens; } - $next = $phpcsFile->findNext(T_INLINE_HTML, 0); - if ($next !== false) { + $hasInlineHTML = $phpcsFile->findNext(T_INLINE_HTML, 0); + if ($hasInlineHTML !== false) { // This rule only applies to PHP-only files. return $phpcsFile->numTokens; } @@ -65,7 +65,15 @@ public function process(File $phpcsFile, $stackPtr) $error = 'Opening PHP tag must be on a line by itself'; $fix = $phpcsFile->addFixableError($error, $stackPtr, 'NotAlone'); if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + + // Remove whitespace between the open tag and the next non-empty token. + for ($i = ($stackPtr + 1); $i < $next; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + $phpcsFile->fixer->addNewline($stackPtr); + $phpcsFile->fixer->endChangeset(); } return $phpcsFile->numTokens; diff --git a/src/Standards/PSR12/Tests/Files/OpenTagUnitTest.2.inc.fixed b/src/Standards/PSR12/Tests/Files/OpenTagUnitTest.2.inc.fixed index 04be0a80f3..9297140dbb 100644 --- a/src/Standards/PSR12/Tests/Files/OpenTagUnitTest.2.inc.fixed +++ b/src/Standards/PSR12/Tests/Files/OpenTagUnitTest.2.inc.fixed @@ -1,2 +1,2 @@ -addFixableError($error, $stackPtr, 'SpacingAfterOpen', $data); if ($fix === true) { - if ($isLongOpenTag === true) { - $phpcsFile->fixer->replaceToken(($stackPtr + 1), ''); - } else if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) { - // Short open tag with too much whitespace. + if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) { + // Open tag with too much whitespace. $phpcsFile->fixer->replaceToken(($stackPtr + 1), ' '); } else { - // Short open tag without whitespace. + // Open tag without whitespace. $phpcsFile->fixer->addContent($stackPtr, ' '); } } diff --git a/src/Tokenizers/PHP.php b/src/Tokenizers/PHP.php index 325b034567..54b819a5ce 100644 --- a/src/Tokenizers/PHP.php +++ b/src/Tokenizers/PHP.php @@ -823,6 +823,59 @@ protected function tokenize($string) continue; }//end if + /* + Split whitespace off from long PHP open tag tokens and potentially join the whitespace + with a subsequent whitespace token. + */ + + if ($tokenIsArray === true + && $token[0] === T_OPEN_TAG + && stripos($token[1], ' T_OPEN_TAG, + 'type' => 'T_OPEN_TAG', + 'content' => $openTagAndWhiteSpace[0], + ]; + $newStackPtr++; + + if (isset($openTagAndWhiteSpace[1]) === true) { + // The original open tag token included whitespace. + // Check whether a new whitespace token needs to be inserted or if the + // whitespace needs to be joined with a pre-existing whitespace + // token on the same line as the open tag. + if (isset($tokens[($stackPtr + 1)]) === true + && $openTagAndWhiteSpace[1] === ' ' + && is_array($tokens[($stackPtr + 1)]) === true + && $tokens[($stackPtr + 1)][0] === T_WHITESPACE + ) { + // Adjusting the original token stack as the "new line may be split over two tokens" + // check should still be run on this token. + $tokens[($stackPtr + 1)][1] = $openTagAndWhiteSpace[1].$tokens[($stackPtr + 1)][1]; + + if (PHP_CODESNIFFER_VERBOSITY > 1) { + StatusWriter::write("* removed whitespace from T_OPEN_TAG token $stackPtr and merged it with the next token T_WHITESPACE", 2); + } + } else { + $finalTokens[$newStackPtr] = [ + 'code' => T_WHITESPACE, + 'type' => 'T_WHITESPACE', + 'content' => $openTagAndWhiteSpace[1], + ]; + + if (PHP_CODESNIFFER_VERBOSITY > 1) { + StatusWriter::write("* T_OPEN_TAG token $stackPtr split into T_OPEN_TAG (without whitespace) and new T_WHITESPACE token", 2); + } + + $newStackPtr++; + }//end if + }//end if + + continue; + }//end if + /* Parse doc blocks into something that can be easily iterated over. */ diff --git a/tests/Core/File/FindStartOfStatementTest.php b/tests/Core/File/FindStartOfStatementTest.php index 41f2086b81..cde94f2ba2 100644 --- a/tests/Core/File/FindStartOfStatementTest.php +++ b/tests/Core/File/FindStartOfStatementTest.php @@ -520,7 +520,7 @@ public function testNestedMatch() public function testOpenTag() { $start = $this->getTargetToken('/* testOpenTag */', T_OPEN_TAG); - $start += 2; + $start += 3; $found = self::$phpcsFile->findStartOfStatement($start); $this->assertSame(($start - 1), $found); diff --git a/tests/Core/Tokenizers/PHP/PHPOpenTagEOF1Test.php b/tests/Core/Tokenizers/PHP/PHPOpenTagEOF1Test.php index 414258e51a..f0c2c41c1e 100644 --- a/tests/Core/Tokenizers/PHP/PHPOpenTagEOF1Test.php +++ b/tests/Core/Tokenizers/PHP/PHPOpenTagEOF1Test.php @@ -43,10 +43,18 @@ public function testLongOpenTagAtEndOfFile() $tokens[$stackPtr]['type'], 'Token tokenized as '.$tokens[$stackPtr]['type'].', not T_OPEN_TAG (type)' ); - $this->assertSame('assertSame('assertArrayHasKey(($stackPtr + 1), $tokens, 'Missing whitespace token after open tag'); + $this->assertSame( + T_WHITESPACE, + $tokens[($stackPtr + 1)]['code'], + 'Missing whitespace token after open tag (code)' + ); + $this->assertSame(' ', $tokens[($stackPtr + 1)]['content'], 'Missing whitespace token after open tag (content)'); // Now make sure that this is the very last token in the file and there are no tokens after it. - $this->assertArrayNotHasKey(($stackPtr + 1), $tokens); + $this->assertArrayNotHasKey(($stackPtr + 2), $tokens); }//end testLongOpenTagAtEndOfFile() diff --git a/tests/Core/Tokenizers/PHP/PHPOpenTagTest.inc b/tests/Core/Tokenizers/PHP/PHPOpenTagTest.inc new file mode 100644 index 0000000000..ae464e7fec --- /dev/null +++ b/tests/Core/Tokenizers/PHP/PHPOpenTagTest.inc @@ -0,0 +1,115 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/Core/Tokenizers/PHP/PHPOpenTagTest.php b/tests/Core/Tokenizers/PHP/PHPOpenTagTest.php new file mode 100644 index 0000000000..9a2c300912 --- /dev/null +++ b/tests/Core/Tokenizers/PHP/PHPOpenTagTest.php @@ -0,0 +1,419 @@ +> $expectedTokens The tokenization expected. + * + * @dataProvider dataLongOpenTag + * + * @return void + */ + public function testLongOpenTag($testMarker, array $expectedTokens) + { + $this->checkTokenSequence($testMarker, $expectedTokens); + + }//end testLongOpenTag() + + + /** + * Data provider. + * + * @return array>>> + */ + public static function dataLongOpenTag() + { + $tokenType = 'T_OPEN_TAG'; + $tokenContent = ' '/* testLongOpenTagWithDoubleNewLine */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => "\n", + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => "\n", + 'lineOffset' => 1, + ], + [ + 'type' => 'T_ECHO', + 'lineOffset' => 2, + ], + ], + ]; + $data['open tag + triple new line'] = [ + 'testMarker' => '/* testLongOpenTagWithTripleNewLine */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => "\n", + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => "\n", + 'lineOffset' => 1, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => "\n", + 'lineOffset' => 2, + ], + [ + 'type' => 'T_ECHO', + 'lineOffset' => 3, + ], + ], + ]; + $data['open tag + new line + indent on next line'] = [ + 'testMarker' => '/* testLongOpenTagWithNewLineAndIndentOnNextLine */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => "\n", + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => ' ', + 'lineOffset' => 1, + ], + [ + 'type' => 'T_ECHO', + 'lineOffset' => 1, + ], + ], + ]; + + return $data; + + }//end dataLongOpenTag() + + + /** + * Test that the tokenization of non-lowercase long PHP open tags does not include whitespace + * and that the case of the tag is unchanged. + * + * @param string $testMarker The comment prefacing the test. + * @param array> $expectedTokens The tokenization expected. + * + * @dataProvider dataCaseLongOpenTag + * + * @return void + */ + public function testCaseLongOpenTag($testMarker, array $expectedTokens) + { + $this->checkTokenSequence($testMarker, $expectedTokens); + + }//end testCaseLongOpenTag() + + + /** + * Data provider. + * + * @return array>>> + */ + public static function dataCaseLongOpenTag() + { + $data = self::getOpenTagTokenizationProvider('CaseLongOpen', 'T_OPEN_TAG', '> $expectedTokens The tokenization expected. + * + * @dataProvider dataShortOpenEchoTag + * + * @return void + */ + public function testShortOpenEchoTag($testMarker, array $expectedTokens) + { + $this->checkTokenSequence($testMarker, $expectedTokens); + + }//end testShortOpenEchoTag() + + + /** + * Data provider. + * + * @return array>>> + */ + public static function dataShortOpenEchoTag() + { + return self::getOpenTagTokenizationProvider('ShortOpenEcho', 'T_OPEN_TAG_WITH_ECHO', '> $expectedTokens The tokenization expected. + * + * @dataProvider dataShortOpenTag + * + * @return void + */ + public function testShortOpenTag($testMarker, array $expectedTokens) + { + if ((bool) ini_get('short_open_tag') === false) { + $this->markTestSkipped('short_open_tag=on is required for this test'); + } + + $this->checkTokenSequence($testMarker, $expectedTokens); + + }//end testShortOpenTag() + + + /** + * Data provider. + * + * @return array>>> + */ + public static function dataShortOpenTag() + { + return self::getOpenTagTokenizationProvider('ShortOpen', 'T_OPEN_TAG', '> $expectedTokens The tokenization expected. + * + * @return void + */ + private function checkTokenSequence($testMarker, array $expectedTokens) + { + $tokens = $this->phpcsFile->getTokens(); + $target = $this->getTargetToken($testMarker, [T_OPEN_TAG, T_OPEN_TAG_WITH_ECHO]); + $stackPtr = $target; + + foreach ($expectedTokens as $tokenInfo) { + $this->assertSame( + constant($tokenInfo['type']), + $tokens[$stackPtr]['code'], + 'Token tokenized as '.Tokens::tokenName($tokens[$stackPtr]['code']).', not '.$tokenInfo['type'].' (code)' + ); + $this->assertSame( + $tokenInfo['type'], + $tokens[$stackPtr]['type'], + 'Token tokenized as '.$tokens[$stackPtr]['type'].', not '.$tokenInfo['type'].' (type)' + ); + + if (isset($tokenInfo['content']) === true) { + $this->assertSame( + $tokenInfo['content'], + $tokens[$stackPtr]['content'], + 'Token content does not match expected content' + ); + } + + $this->assertSame( + ($tokens[$target]['line'] + $tokenInfo['lineOffset']), + $tokens[$stackPtr]['line'], + 'Line number does not match expected line number' + ); + + ++$stackPtr; + }//end foreach + + }//end checkTokenSequence() + + + /** + * Data provider generator. + * + * @param string $tagtype The type of tag being examined. + * @param string $tokenType The expected token type. + * @param string $tokenContent The expected token contents. + * + * @return array>>> + */ + private static function getOpenTagTokenizationProvider($tagtype, $tokenType, $tokenContent) + { + $lastTokenType = 'T_ECHO'; + if ($tagtype === 'ShortOpenEcho') { + $lastTokenType = 'T_CONSTANT_ENCAPSED_STRING'; + } + + return [ + 'open tag + new line' => [ + 'testMarker' => '/* test'.$tagtype.'TagWithNewLine */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => "\n", + 'lineOffset' => 0, + ], + [ + 'type' => $lastTokenType, + 'lineOffset' => 1, + ], + ], + ], + 'open tag + one space + new line' => [ + 'testMarker' => '/* test'.$tagtype.'TagWithOneSpaceAndNewLine */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => ' '."\n", + 'lineOffset' => 0, + ], + [ + 'type' => $lastTokenType, + 'lineOffset' => 1, + ], + ], + ], + 'open tag + trailing whitespace + new line' => [ + 'testMarker' => '/* test'.$tagtype.'TagWithTrailingWhiteSpaceAndNewLine */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => ' '."\n", + 'lineOffset' => 0, + ], + [ + 'type' => $lastTokenType, + 'lineOffset' => 1, + ], + ], + ], + 'open tag, no space' => [ + 'testMarker' => '/* test'.$tagtype.'TagNoSpace */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => $lastTokenType, + 'lineOffset' => 0, + ], + ], + ], + 'open tag, one space' => [ + 'testMarker' => '/* test'.$tagtype.'TagOneSpace */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => ' ', + 'lineOffset' => 0, + ], + [ + 'type' => $lastTokenType, + 'lineOffset' => 0, + ], + ], + ], + 'open tag, multi space' => [ + 'testMarker' => '/* test'.$tagtype.'TagMultiSpace */', + 'expectedTokens' => [ + [ + 'type' => $tokenType, + 'content' => $tokenContent, + 'lineOffset' => 0, + ], + [ + 'type' => 'T_WHITESPACE', + 'content' => ' ', + 'lineOffset' => 0, + ], + [ + 'type' => $lastTokenType, + 'lineOffset' => 0, + ], + ], + ], + ]; + + }//end getOpenTagTokenizationProvider() + + +}//end class diff --git a/tests/Core/Tokenizers/Tokenizer/ReplaceTabsInTokenMiscTest.php b/tests/Core/Tokenizers/Tokenizer/ReplaceTabsInTokenMiscTest.php index 18b9ac548c..deae18e9a6 100644 --- a/tests/Core/Tokenizers/Tokenizer/ReplaceTabsInTokenMiscTest.php +++ b/tests/Core/Tokenizers/Tokenizer/ReplaceTabsInTokenMiscTest.php @@ -41,10 +41,9 @@ public function testTabWidthNotSet() $phpcsFile->parse(); $tokens = $phpcsFile->getTokens(); - $target = $phpcsFile->findNext(T_WHITESPACE, 0); + $target = 2; // Verify initial state. - $this->assertIsInt($target, 'Target token was not found'); $this->assertSame(' ', $tokens[$target]['content'], 'Content after initial parsing does not contain tabs'); $this->assertSame(2, $tokens[$target]['length'], 'Length after initial parsing is not as expected'); $this->assertArrayNotHasKey('orig_content', $tokens[$target], "Key 'orig_content' found in the initial token array.");