Skip to content

Commit 0ed5577

Browse files
rodrigoprimojrfnl
authored andcommitted
Squiz/EmbeddedPhp: fix false positive when handling short open tags
The content of a PHP long open tag token is `<?php ` (note the space after the tag). The content of a PHP short open tag is `<?` (no space after the tag). The sniff did not account correctly for this difference when checking the expected number of spaces after a short open tag, resulting in false positives and incorrect fixes. For example, the code below: ``` <? echo 'one space after short open tag'; ?> ``` Resulted in the error (there is just one space after the opening tag and not two, as stated in the error): ``` Expected 1 space after opening PHP tag; 2 found (Squiz.PHP.EmbeddedPhp.SpacingAfterOpen) ``` And the incorrect fix: ``` <?echo 'without space after short open tag'; ?> ``` This commit fixes this problem by changing the sniff code to consider that only long open tags contain a space after the tag in the `content` key of the token array.
1 parent c76678a commit 0ed5577

File tree

6 files changed

+78
-4
lines changed

6 files changed

+78
-4
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,10 +379,14 @@ private function validateInlineEmbeddedPhp($phpcsFile, $stackPtr, $closeTag)
379379
}
380380

381381
// Check that there is one, and only one space at the start of the statement.
382-
$leadingSpace = 0;
383-
if ($tokens[$stackPtr]['code'] === T_OPEN_TAG) {
382+
$leadingSpace = 0;
383+
$isLongOpenTag = false;
384+
if ($tokens[$stackPtr]['code'] === T_OPEN_TAG
385+
&& stripos($tokens[$stackPtr]['content'], '<?php') === 0
386+
) {
384387
// The long open tag token in a single line tag set always contains a single space after it.
385-
$leadingSpace = 1;
388+
$leadingSpace = 1;
389+
$isLongOpenTag = true;
386390
}
387391

388392
if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
@@ -394,7 +398,7 @@ private function validateInlineEmbeddedPhp($phpcsFile, $stackPtr, $closeTag)
394398
$data = [$leadingSpace];
395399
$fix = $phpcsFile->addFixableError($error, $stackPtr, 'SpacingAfterOpen', $data);
396400
if ($fix === true) {
397-
if ($tokens[$stackPtr]['code'] === T_OPEN_TAG) {
401+
if ($isLongOpenTag === true) {
398402
$phpcsFile->fixer->replaceToken(($stackPtr + 1), '');
399403
} else if ($tokens[($stackPtr + 1)]['code'] === T_WHITESPACE) {
400404
// Short open tag with too much whitespace.

src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,10 @@ echo 'the PHP tag is correctly indented as an indent less than the previous code
265265
echo 'the PHP tag is incorrectly indented as the indent is more than 4 different from the indent of the previous code';
266266
?>
267267

268+
<?PHP echo 'Uppercase long open tag'; ?>
269+
270+
<?PHP echo 'Extra space after uppercase long open tag '; ?>
271+
268272
<?php
269273
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
270274
// the tests running into the "last PHP closing tag excepted" condition breaking tests.

src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.1.inc.fixed

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,10 @@ echo 'the PHP tag is correctly indented as an indent less than the previous code
277277
echo 'the PHP tag is incorrectly indented as the indent is more than 4 different from the indent of the previous code';
278278
?>
279279

280+
<?PHP echo 'Uppercase long open tag'; ?>
281+
282+
<?PHP echo 'Extra space after uppercase long open tag '; ?>
283+
280284
<?php
281285
// This test case file MUST always end with an unclosed long open PHP tag (with this comment) to prevent
282286
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?
2+
3+
// This test case file MUST always start with a open PHP tag set (with this comment) to prevent
4+
// the tests running into the "first PHP open tag excepted" condition breaking the tests.
5+
// Tests related to that "first PHP open tag excepted" condition should go in separate files.
6+
7+
// This test case file only deals with SHORT OPEN TAGS.
8+
9+
?>
10+
11+
<?
12+
/* Contrary to the long open tag token, the short open tag token does not contain a space after the
13+
tag and the sniff should handle it accordingly. The test below protects against regressions
14+
related to https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/588. */
15+
?>
16+
<? echo 'one space after short open tag'; ?>
17+
18+
<? echo 'two spaces after short open tag'; ?>
19+
20+
<?echo 'without space after short open tag'; ?>
21+
22+
<?
23+
// This test case file MUST always end with an unclosed open PHP tag (with this comment) to prevent
24+
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
25+
// Tests related to that "last PHP closing tag excepted" condition should go in separate files.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
<?
2+
3+
// This test case file MUST always start with a open PHP tag set (with this comment) to prevent
4+
// the tests running into the "first PHP open tag excepted" condition breaking the tests.
5+
// Tests related to that "first PHP open tag excepted" condition should go in separate files.
6+
7+
// This test case file only deals with SHORT OPEN TAGS.
8+
9+
?>
10+
11+
<?
12+
/* Contrary to the long open tag token, the short open tag token does not contain a space after the
13+
tag and the sniff should handle it accordingly. The test below protects against regressions
14+
related to https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/588. */
15+
?>
16+
<? echo 'one space after short open tag'; ?>
17+
18+
<? echo 'two spaces after short open tag'; ?>
19+
20+
<? echo 'without space after short open tag'; ?>
21+
22+
<?
23+
// This test case file MUST always end with an unclosed open PHP tag (with this comment) to prevent
24+
// the tests running into the "last PHP closing tag excepted" condition breaking tests.
25+
// Tests related to that "last PHP closing tag excepted" condition should go in separate files.

src/Standards/Squiz/Tests/PHP/EmbeddedPhpUnitTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ public function getErrorList($testFile='')
100100
258 => 1,
101101
263 => 1,
102102
264 => 1,
103+
270 => 1,
103104
];
104105

105106
case 'EmbeddedPhpUnitTest.2.inc':
@@ -190,6 +191,17 @@ public function getErrorList($testFile='')
190191
22 => 2,
191192
];
192193

194+
case 'EmbeddedPhpUnitTest.24.inc':
195+
$shortOpenTagDirective = (bool) ini_get('short_open_tag');
196+
if ($shortOpenTagDirective === true) {
197+
return [
198+
18 => 1,
199+
20 => 1,
200+
];
201+
}
202+
203+
return [];
204+
193205
default:
194206
return [];
195207
}//end switch

0 commit comments

Comments
 (0)