Skip to content

Commit 327a08c

Browse files
gsherwoodgsherwood
committed
Fixed bug #2732 : PSR12.Files.FileHeader misidentifies file header in mixed content file
This required a rewrite of the sniff to process the entire file looking for a file header. It's still impossible to determine if a docblock is a file comment or not, but it should work with more code now.
1 parent e9ebf52 commit 327a08c

10 files changed

+221
-43
lines changed

‎package.xml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
5454
-- Thanks to Matthew Peveler for the patch
5555
- Fixed bug #2730 : PSR12.ControlStructures.ControlStructureSpacing does not ignore comments between conditions
5656
-- Thanks to Juliette Reinders Folmer for the patch
57+
- Fixed bug #2732 : PSR12.Files.FileHeader misidentifies file header in mixed content file
5758
- Fixed bug #2745 : AbstractArraySniff wrong indices when mixed coalesce and ternary values
5859
-- Thanks to Michał Bundyra for the patch
5960
- Fixed bug #2748 : Wrong end of statement for fn closures

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

Lines changed: 104 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,86 @@ public function process(File $phpcsFile, $stackPtr)
4242
{
4343
$tokens = $phpcsFile->getTokens();
4444

45-
/*
46-
First, gather information about the statements inside
47-
the file header.
48-
*/
45+
$possibleHeaders = [];
46+
47+
$searchFor = Tokens::$ooScopeTokens;
48+
$searchFor[T_OPEN_TAG] = T_OPEN_TAG;
49+
50+
$openTag = $stackPtr;
51+
do {
52+
$headerLines = $this->getHeaderLines($phpcsFile, $openTag);
53+
if (empty($headerLines) === true && $openTag === $stackPtr) {
54+
// No content in the file.
55+
return;
56+
}
57+
58+
$possibleHeaders[$openTag] = $headerLines;
59+
if (count($headerLines) > 1) {
60+
break;
61+
}
62+
63+
$next = $phpcsFile->findNext($searchFor, ($openTag + 1));
64+
if (isset(Tokens::$ooScopeTokens[$tokens[$next]['code']]) === true) {
65+
// Once we find an OO token, the file content has
66+
// definitely started.
67+
break;
68+
}
69+
70+
$openTag = $next;
71+
} while ($openTag !== false);
72+
73+
if ($openTag === false) {
74+
// We never found a proper file header
75+
// so use the first one as the header.
76+
$openTag = $stackPtr;
77+
} else if (count($possibleHeaders) > 1) {
78+
// There are other PHP blocks before the file header.
79+
$error = 'The file header must be the first content in the file';
80+
$phpcsFile->addError($error, $openTag, 'HeaderPosition');
81+
} else {
82+
// The first possible header was the file header block,
83+
// so make sure it is the first content in the file.
84+
if ($openTag !== 0) {
85+
// Allow for hashbang lines.
86+
$hashbang = false;
87+
if ($tokens[($openTag - 1)]['code'] === T_INLINE_HTML) {
88+
$content = trim($tokens[($openTag - 1)]['content']);
89+
if (substr($content, 0, 2) === '#!') {
90+
$hashbang = true;
91+
}
92+
}
93+
94+
if ($hashbang === false) {
95+
$error = 'The file header must be the first content in the file';
96+
$phpcsFile->addError($error, $openTag, 'HeaderPosition');
97+
}
98+
}
99+
}//end if
100+
101+
$this->processHeaderLines($phpcsFile, $possibleHeaders[$openTag]);
102+
103+
return $phpcsFile->numTokens;
104+
105+
}//end process()
106+
107+
108+
/**
109+
* Gather information about the statements inside a possible file header.
110+
*
111+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
112+
* @param int $stackPtr The position of the current
113+
* token in the stack.
114+
*
115+
* @return array
116+
*/
117+
public function getHeaderLines(File $phpcsFile, $stackPtr)
118+
{
119+
$tokens = $phpcsFile->getTokens();
120+
121+
$next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true);
122+
if ($next === false) {
123+
return [];
124+
}
49125

50126
$headerLines = [];
51127
$headerLines[] = [
@@ -54,17 +130,18 @@ public function process(File $phpcsFile, $stackPtr)
54130
'end' => $stackPtr,
55131
];
56132

57-
$next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true);
58-
if ($next === false) {
59-
return;
60-
}
61-
62133
$foundDocblock = false;
63134

64135
$commentOpeners = Tokens::$scopeOpeners;
65136
unset($commentOpeners[T_NAMESPACE]);
66137
unset($commentOpeners[T_DECLARE]);
67138
unset($commentOpeners[T_USE]);
139+
unset($commentOpeners[T_IF]);
140+
unset($commentOpeners[T_WHILE]);
141+
unset($commentOpeners[T_FOR]);
142+
unset($commentOpeners[T_FOREACH]);
143+
unset($commentOpeners[T_DO]);
144+
unset($commentOpeners[T_TRY]);
68145

69146
do {
70147
switch ($tokens[$next]['code']) {
@@ -77,6 +154,7 @@ public function process(File $phpcsFile, $stackPtr)
77154
// Make sure this is not a code-level docblock.
78155
$end = $tokens[$next]['comment_closer'];
79156
$docToken = $phpcsFile->findNext(Tokens::$emptyTokens, ($end + 1), null, true);
157+
80158
if (isset($commentOpeners[$tokens[$docToken]['code']]) === false
81159
&& isset(Tokens::$methodPrefixes[$tokens[$docToken]['code']]) === false
82160
) {
@@ -155,17 +233,23 @@ public function process(File $phpcsFile, $stackPtr)
155233
$next = $phpcsFile->findNext(T_WHITESPACE, ($next + 1), null, true);
156234
} while ($next !== false);
157235

158-
if (count($headerLines) === 1) {
159-
// This is only an open tag and doesn't contain the file header.
160-
// We need to keep checking for one in case they put it further
161-
// down in the file.
162-
return;
163-
}
236+
return $headerLines;
164237

165-
/*
166-
Next, check the spacing and grouping of the statements
167-
inside each header block.
168-
*/
238+
}//end getHeaderLines()
239+
240+
241+
/**
242+
* Check the spacing and grouping of the statements inside each header block.
243+
*
244+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
245+
* @param array $headerLines Header information, as sourced
246+
* from getHeaderLines().
247+
*
248+
* @return int|null
249+
*/
250+
public function processHeaderLines(File $phpcsFile, $headerLines)
251+
{
252+
$tokens = $phpcsFile->getTokens();
169253

170254
$found = [];
171255

@@ -301,30 +385,7 @@ public function process(File $phpcsFile, $stackPtr)
301385
}//end if
302386
}//end foreach
303387

304-
/*
305-
Finally, make sure this header block is at the very
306-
top of the file.
307-
*/
308-
309-
if ($stackPtr !== 0) {
310-
// Allow for hashbang lines.
311-
$hashbang = false;
312-
if ($tokens[($stackPtr - 1)]['code'] === T_INLINE_HTML) {
313-
$content = trim($tokens[($stackPtr - 1)]['content']);
314-
if (substr($content, 0, 2) === '#!') {
315-
$hashbang = true;
316-
}
317-
}
318-
319-
if ($hashbang === false) {
320-
$error = 'The file header must be the first content in the file';
321-
$phpcsFile->addError($error, $stackPtr, 'HeaderPosition');
322-
}
323-
}
324-
325-
return $phpcsFile->numTokens;
326-
327-
}//end process()
388+
}//end processHeaderLines()
328389

329390

330391
}//end class

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.10.inc.fixed

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
3+
// Do nothing
4+
// Do nothing
5+
// Do nothing

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.11.inc

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
/**
3+
* Template file docblock.
4+
*
5+
* @package Vendor\Package
6+
*/
7+
8+
if (!defined('ABSPATH')) {
9+
exit;
10+
}
11+
12+
?>
13+
14+
<p><?php echo 'Some text string'; ?></p>
15+
16+
<?php
17+
18+
/**
19+
* Docblock.
20+
*/
21+
include 'foo.php';

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.11.inc.fixed

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/**
4+
* Template file docblock.
5+
*
6+
* @package Vendor\Package
7+
*/
8+
9+
if (!defined('ABSPATH')) {
10+
exit;
11+
}
12+
13+
?>
14+
15+
<p><?php echo 'Some text string'; ?></p>
16+
17+
<?php
18+
19+
/**
20+
* Docblock.
21+
*/
22+
include 'foo.php';

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.12.inc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
echo 'Some content';
3+
?>
4+
<?php
5+
/**
6+
* The header is not the first thing in the file.
7+
*/
8+
9+
namespace Vendor\Package;
10+
11+
/**
12+
* FooBar is an example class.
13+
*/
14+
class FooBar
15+
{
16+
// ... additional PHP code ...
17+
}

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.12.inc.fixed

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
echo 'Some content';
3+
?>
4+
<?php
5+
6+
/**
7+
* The header is not the first thing in the file.
8+
*/
9+
10+
namespace Vendor\Package;
11+
12+
/**
13+
* FooBar is an example class.
14+
*/
15+
class FooBar
16+
{
17+
// ... additional PHP code ...
18+
}

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.13.inc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/**
4+
* File docblock.
5+
*
6+
* @package Vendor\Package
7+
*/
8+
9+
class Foo {
10+
/**
11+
* Function docblock.
12+
*/
13+
public function bar() {
14+
do_something();
15+
?>
16+
<p>Demo</p>
17+
18+
<?php
19+
/**
20+
* Arbitrary docblock.
21+
*/
22+
api_call();
23+
}
24+
}

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.7.inc.fixed

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<?php
2+
3+
// Do nothing

‎src/Standards/PSR12/Tests/Files/FileHeaderUnitTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ public function getErrorList($testFile='')
5151
];
5252
case 'FileHeaderUnitTest.5.inc':
5353
return [4 => 1];
54+
case 'FileHeaderUnitTest.7.inc':
55+
case 'FileHeaderUnitTest.10.inc':
56+
case 'FileHeaderUnitTest.11.inc':
57+
return [1 => 1];
58+
case 'FileHeaderUnitTest.12.inc':
59+
return [4 => 2];
5460
default:
5561
return [];
5662
}//end switch

0 commit comments

Comments
 (0)