Skip to content

Commit b00f2e4

Browse files
jrfnljrfnl
committed
Tokenizer/PHP: change tokenization of long PHP open tags
... to no longer include whitespace (single space/new line). This ensures the tokenization of all PHP open tags - long open tags `<?php`, short open tags `<?` and short open echo tags `<?=` - is consistent and should simplify handling of open tag tokens in sniffs and lower the cognitive load of sniff developers. As per the detailed proposal in ticket 593. Includes extensive unit tests, both ensuring the correct tokenization, as well as safeguarding consistency across the different open tags. Includes the following additional updates to maintain the pre-existing behaviour: * `AbstractPatternSniff` The `AbstractPatternSniff` tokenizes an arbitrary code pattern to determine the token pattern to sniff for. This tokenization now to takes the change in the PHP open tag tokenization into account. * `Generic/EmptyPHPStatement` - replace now redundant special condition for PHP open tags + update "fixed" file for improved output + add extra test. * `PSR12/OpenTag` - allow for the changed tokenization. The updated code also means that the fixer will no longer leave stray whitespace behind after a PHP open tag. * `Squiz/CommentedOutCode` - allow for the changed tokenization. * `Squiz/EmbeddedPhp` - code simplification. * `FindStartOfStatementTest` - fixed token offset expectation as there is now an extra whitespace token. * `ReplaceTabsInTokenMiscTest` - updated target finding Fixes 593
1 parent 268d6ad commit b00f2e4

15 files changed

Lines changed: 631 additions & 41 deletions

‎src/Sniffs/AbstractPatternSniff.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -915,12 +915,12 @@ private function createTokenPattern($str)
915915

916916
// Don't add a space after the closing php tag as it will add a new
917917
// whitespace token.
918-
$tokenizer = new PHP('<?php '.$str.'?>', null);
918+
$tokenizer = new PHP('<?php'."\n".$str.'?>', null);
919919
StatusWriter::resume();
920920

921921
// Remove the <?php tag from the front and the end php tag from the back.
922922
$tokens = $tokenizer->getTokens();
923-
$tokens = array_slice($tokens, 1, (count($tokens) - 2));
923+
$tokens = array_slice($tokens, 2, (count($tokens) - 3));
924924

925925
$patterns = [];
926926
foreach ($tokens as $patternInfo) {

‎src/Standards/Generic/Sniffs/CodeAnalysis/EmptyPHPStatementSniff.php‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -113,19 +113,15 @@ private function processSemicolon(File $phpcsFile, $stackPtr)
113113
if ($fix === true) {
114114
$phpcsFile->fixer->beginChangeset();
115115

116-
if ($tokens[$prevNonEmpty]['code'] === T_OPEN_TAG
117-
|| $tokens[$prevNonEmpty]['code'] === T_OPEN_TAG_WITH_ECHO
118-
) {
119-
// Check for superfluous whitespace after the semicolon which should be
120-
// removed as the `<?php ` open tag token already contains whitespace,
121-
// either a space or a new line.
122-
if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
123-
$replacement = str_replace(' ', '', $tokens[($stackPtr + 1)]['content']);
124-
$phpcsFile->fixer->replaceToken(($stackPtr + 1), $replacement);
125-
}
116+
// Make sure there always remains one space between the open tag and the next content.
117+
$replacement = ' ';
118+
if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
119+
$replacement = '';
126120
}
127121

128-
for ($i = $stackPtr; $i > $prevNonEmpty; $i--) {
122+
$phpcsFile->fixer->replaceToken($stackPtr, $replacement);
123+
124+
for ($i = ($stackPtr - 1); $i > $prevNonEmpty; $i--) {
129125
if ($tokens[$i]['code'] !== T_SEMICOLON
130126
&& $tokens[$i]['code'] !== T_WHITESPACE
131127
) {

‎src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!-- Tests with short open tag. -->
2-
2+
<input name="<? ;something_else(); ?>" />
33
<input name="<? ; something_else(); ?>" />
44
<input name="<? something_else(); ; ?>" />
55

‎src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.2.inc.fixed‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<!-- Tests with short open tag. -->
2-
3-
<input name="<?something_else(); ?>" />
2+
<input name="<? something_else(); ?>" />
3+
<input name="<? something_else(); ?>" />
44
<input name="<? something_else(); ?>" />
55

66
/*

‎src/Standards/Generic/Tests/CodeAnalysis/EmptyPHPStatementUnitTest.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ public function getWarningList($testFile='')
9393
];
9494
case 'EmptyPHPStatementUnitTest.2.inc':
9595
return [
96+
2 => 1,
9697
3 => 1,
9798
4 => 1,
9899
13 => 1,

‎src/Standards/PSR12/Sniffs/Files/OpenTagSniff.php‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,16 +56,24 @@ public function process(File $phpcsFile, $stackPtr)
5656
return $phpcsFile->numTokens;
5757
}
5858

59-
$next = $phpcsFile->findNext(T_INLINE_HTML, 0);
60-
if ($next !== false) {
59+
$hasInlineHTML = $phpcsFile->findNext(T_INLINE_HTML, 0);
60+
if ($hasInlineHTML !== false) {
6161
// This rule only applies to PHP-only files.
6262
return $phpcsFile->numTokens;
6363
}
6464

6565
$error = 'Opening PHP tag must be on a line by itself';
6666
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'NotAlone');
6767
if ($fix === true) {
68+
$phpcsFile->fixer->beginChangeset();
69+
70+
// Remove whitespace between the open tag and the next non-empty token.
71+
for ($i = ($stackPtr + 1); $i < $next; $i++) {
72+
$phpcsFile->fixer->replaceToken($i, '');
73+
}
74+
6875
$phpcsFile->fixer->addNewline($stackPtr);
76+
$phpcsFile->fixer->endChangeset();
6977
}
7078

7179
return $phpcsFile->numTokens;

‎src/Standards/PSR12/Tests/Files/OpenTagUnitTest.2.inc.fixed‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
<?php
1+
<?php
22
echo 'hi';

‎src/Standards/Squiz/Sniffs/PHP/CommentedOutCodeSniff.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,11 +194,13 @@ public function process(File $phpcsFile, $stackPtr)
194194
*/
195195

196196
// First token is always the opening tag.
197-
if ($stringTokens[0]['code'] !== T_OPEN_TAG) {
197+
if ($stringTokens[0]['code'] !== T_OPEN_TAG || $stringTokens[1]['code'] !== T_WHITESPACE) {
198198
return ($lastCommentBlockToken + 1);
199199
} else {
200+
// Remove the PHP open tag + the whitespace token following it.
200201
array_shift($stringTokens);
201-
--$numTokens;
202+
array_shift($stringTokens);
203+
$numTokens -= 2;
202204
}
203205

204206
// Last token is always the closing tag, unless something went wrong.

‎src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -347,16 +347,7 @@ private function validateInlineEmbeddedPhp($phpcsFile, $stackPtr, $closeTag)
347347
}
348348

349349
// Check that there is one, and only one space at the start of the statement.
350-
$leadingSpace = 0;
351-
$isLongOpenTag = false;
352-
if ($tokens[$stackPtr]['code'] === T_OPEN_TAG
353-
&& stripos($tokens[$stackPtr]['content'], '<?php') === 0
354-
) {
355-
// The long open tag token in a single line tag set always contains a single space after it.
356-
$leadingSpace = 1;
357-
$isLongOpenTag = true;
358-
}
359-
350+
$leadingSpace = 0;
360351
if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
361352
$leadingSpace += $tokens[($stackPtr + 1)]['length'];
362353
}
@@ -366,13 +357,11 @@ private function validateInlineEmbeddedPhp($phpcsFile, $stackPtr, $closeTag)
366357
$data = [$leadingSpace];
367358
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfterOpen', $data);
368359
if ($fix === true) {
369-
if ($isLongOpenTag === true) {
370-
$phpcsFile->fixer->replaceToken(($stackPtr + 1), '');
371-
} else if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
372-
// Short open tag with too much whitespace.
360+
if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
361+
// Open tag with too much whitespace.
373362
$phpcsFile->fixer->replaceToken(($stackPtr + 1), ' ');
374363
} else {
375-
// Short open tag without whitespace.
364+
// Open tag without whitespace.
376365
$phpcsFile->fixer->addContent($stackPtr, ' ');
377366
}
378367
}

‎src/Tokenizers/PHP.php‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,59 @@ protected function tokenize($string)
823823
continue;
824824
}//end if
825825

826+
/*
827+
Split whitespace off from long PHP open tag tokens and potentially join the whitespace
828+
with a subsequent whitespace token.
829+
*/
830+
831+
if ($tokenIsArray === true
832+
&& $token[0] === T_OPEN_TAG
833+
&& stripos($token[1], '<?php') === 0
834+
) {
835+
$openTagAndWhiteSpace = str_split($token[1], 5);
836+
837+
$finalTokens[$newStackPtr] = [
838+
'code' => T_OPEN_TAG,
839+
'type' => 'T_OPEN_TAG',
840+
'content' => $openTagAndWhiteSpace[0],
841+
];
842+
$newStackPtr++;
843+
844+
if (isset($openTagAndWhiteSpace[1]) === true) {
845+
// The original open tag token included whitespace.
846+
// Check whether a new whitespace token needs to be inserted or if the
847+
// whitespace needs to be joined with a pre-existing whitespace
848+
// token on the same line as the open tag.
849+
if (isset($tokens[($stackPtr + 1)]) === true
850+
&& $openTagAndWhiteSpace[1] === ' '
851+
&& is_array($tokens[($stackPtr + 1)]) === true
852+
&& $tokens[($stackPtr + 1)][0] === T_WHITESPACE
853+
) {
854+
// Adjusting the original token stack as the "new line may be split over two tokens"
855+
// check should still be run on this token.
856+
$tokens[($stackPtr + 1)][1] = $openTagAndWhiteSpace[1].$tokens[($stackPtr + 1)][1];
857+
858+
if (PHP_CODESNIFFER_VERBOSITY > 1) {
859+
StatusWriter::write("* removed whitespace from T_OPEN_TAG token $stackPtr and merged it with the next token T_WHITESPACE", 2);
860+
}
861+
} else {
862+
$finalTokens[$newStackPtr] = [
863+
'code' => T_WHITESPACE,
864+
'type' => 'T_WHITESPACE',
865+
'content' => $openTagAndWhiteSpace[1],
866+
];
867+
868+
if (PHP_CODESNIFFER_VERBOSITY > 1) {
869+
StatusWriter::write("* T_OPEN_TAG token $stackPtr split into T_OPEN_TAG (without whitespace) and new T_WHITESPACE token", 2);
870+
}
871+
872+
$newStackPtr++;
873+
}//end if
874+
}//end if
875+
876+
continue;
877+
}//end if
878+
826879
/*
827880
Parse doc blocks into something that can be easily iterated over.
828881
*/

0 commit comments

Comments
 (0)