Skip to content

Commit 384f8e8

Browse files
jrfnljrfnl
authored
Merge pull request #840 from PHPCSStandards/feature/squiz-membervarspacing-bug-fix-blank-lines-in-preamble
Squiz/MemberVarSpacing: bug fix - handling of blank lines in pre-amble
2 parents cb44a06 + fc70e9d commit 384f8e8

File tree

4 files changed

+112
-54
lines changed

4 files changed

+112
-54
lines changed

‎src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php

Lines changed: 46 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -82,60 +82,56 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
8282
break;
8383
}
8484

85-
if (isset(Tokens::$commentTokens[$tokens[$prev]['code']]) === true) {
85+
if ($tokens[$prev]['code'] === T_DOC_COMMENT_CLOSE_TAG) {
86+
$start = $prev;
87+
} else if (isset(Tokens::$commentTokens[$tokens[$prev]['code']]) === true) {
8688
// Assume the comment belongs to the member var if it is on a line by itself.
8789
$prevContent = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true);
8890
if ($tokens[$prevContent]['line'] !== $tokens[$prev]['line']) {
89-
// Check the spacing, but then skip it.
90-
$foundLines = ($tokens[$startOfStatement]['line'] - $tokens[$prev]['line'] - 1);
91-
if ($foundLines > 0) {
92-
for ($i = ($prev + 1); $i < $startOfStatement; $i++) {
93-
if ($tokens[$i]['column'] !== 1) {
94-
continue;
95-
}
96-
97-
if ($tokens[$i]['code'] === T_WHITESPACE
98-
&& $tokens[$i]['line'] !== $tokens[($i + 1)]['line']
99-
) {
100-
$error = 'Expected 0 blank lines after member var comment; %s found';
101-
$data = [$foundLines];
102-
$fix = $phpcsFile->addFixableError($error, $prev, 'AfterComment', $data);
103-
if ($fix === true) {
104-
$phpcsFile->fixer->beginChangeset();
105-
// Inline comments have the newline included in the content but
106-
// docblocks do not.
107-
if ($tokens[$prev]['code'] === T_COMMENT) {
108-
$phpcsFile->fixer->replaceToken($prev, rtrim($tokens[$prev]['content']));
109-
}
110-
111-
for ($i = ($prev + 1); $i <= $startOfStatement; $i++) {
112-
if ($tokens[$i]['line'] === $tokens[$startOfStatement]['line']) {
113-
break;
114-
}
115-
116-
// Remove the newline after the docblock, and any entirely
117-
// empty lines before the member var.
118-
if (($tokens[$i]['code'] === T_WHITESPACE
119-
&& $tokens[$i]['line'] === $tokens[$prev]['line'])
120-
|| ($tokens[$i]['column'] === 1
121-
&& $tokens[$i]['line'] !== $tokens[($i + 1)]['line'])
122-
) {
123-
$phpcsFile->fixer->replaceToken($i, '');
124-
}
125-
}
126-
127-
$phpcsFile->fixer->addNewline($prev);
128-
$phpcsFile->fixer->endChangeset();
129-
}//end if
130-
131-
break;
132-
}//end if
133-
}//end for
134-
}//end if
135-
13691
$start = $prev;
137-
}//end if
138-
}//end if
92+
}
93+
}
94+
95+
// Check for blank lines between the docblock/comment and the property declaration.
96+
for ($i = ($start + 1); $i < $startOfStatement; $i++) {
97+
if (isset($tokens[$i]['attribute_closer']) === true) {
98+
$i = $tokens[$i]['attribute_closer'];
99+
continue;
100+
}
101+
102+
if ($tokens[$i]['column'] !== 1
103+
|| $tokens[$i]['code'] !== T_WHITESPACE
104+
|| $tokens[$i]['line'] === $tokens[($i + 1)]['line']
105+
// Do not report blank lines after a PHPCS annotation as removing the blank lines could change the meaning.
106+
|| isset(Tokens::$phpcsCommentTokens[$tokens[($i - 1)]['code']]) === true
107+
) {
108+
continue;
109+
}
110+
111+
// We found a blank line which should be reported.
112+
$nextNonWhitespace = $phpcsFile->findNext(T_WHITESPACE, ($i + 1), null, true);
113+
$foundLines = ($tokens[$nextNonWhitespace]['line'] - $tokens[$i]['line']);
114+
115+
$error = 'Expected no blank lines between the member var comment and the declaration; %s found';
116+
$data = [$foundLines];
117+
$fix = $phpcsFile->addFixableError($error, $i, 'AfterComment', $data);
118+
119+
if ($fix === true) {
120+
$phpcsFile->fixer->beginChangeset();
121+
122+
for ($j = $i; $j < $nextNonWhitespace; $j++) {
123+
if ($tokens[$j]['line'] === $tokens[$nextNonWhitespace]['line']) {
124+
break;
125+
}
126+
127+
$phpcsFile->fixer->replaceToken($j, '');
128+
}
129+
130+
$phpcsFile->fixer->endChangeset();
131+
}
132+
133+
$i = $nextNonWhitespace;
134+
}//end for
139135

140136
// There needs to be n blank lines before the var, not counting comments.
141137
if ($start === $startOfStatement) {

‎src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.1.inc

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,3 +385,33 @@ class NoPreambleMultilineDeclaration {
385385
static
386386
int $prop = 1;
387387
}
388+
389+
class MultipleBlankLinesInPreAmble {
390+
391+
/**
392+
* Docblock.
393+
*/
394+
395+
#[MyAttribute]
396+
397+
398+
#[
399+
400+
BlankLinesWithinAnAttributeShouldBeLeftAlone
401+
402+
]
403+
404+
public $prop;
405+
}
406+
407+
final class BlankLinesBetweenVsAttributesWithoutCommentIssueSquiz3594
408+
{
409+
410+
// PHPCS can fix blank lines for the first property, but not for the second. (fixed now)
411+
#[SingleAttribute]
412+
413+
public $property1;
414+
#[SingleAttribute]
415+
416+
public $property2;
417+
}

‎src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.1.inc.fixed

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,3 +375,28 @@ class NoPreambleMultilineDeclaration {
375375
static
376376
int $prop = 1;
377377
}
378+
379+
class MultipleBlankLinesInPreAmble {
380+
381+
/**
382+
* Docblock.
383+
*/
384+
#[MyAttribute]
385+
#[
386+
387+
BlankLinesWithinAnAttributeShouldBeLeftAlone
388+
389+
]
390+
public $prop;
391+
}
392+
393+
final class BlankLinesBetweenVsAttributesWithoutCommentIssueSquiz3594
394+
{
395+
396+
// PHPCS can fix blank lines for the first property, but not for the second. (fixed now)
397+
#[SingleAttribute]
398+
public $property1;
399+
400+
#[SingleAttribute]
401+
public $property2;
402+
}

‎src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.php

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@ public function getErrorList($testFile='')
4444
50 => 1,
4545
73 => 1,
4646
86 => 1,
47-
106 => 1,
47+
107 => 1,
4848
115 => 1,
49-
150 => 1,
49+
151 => 1,
5050
160 => 1,
5151
165 => 1,
5252
177 => 1,
@@ -66,16 +66,23 @@ public function getErrorList($testFile='')
6666
288 => 1,
6767
292 => 1,
6868
333 => 1,
69-
342 => 1,
69+
343 => 1,
70+
345 => 1,
7071
346 => 1,
71-
353 => 1,
72+
355 => 1,
7273
357 => 1,
7374
366 => 1,
7475
377 => 1,
7576
378 => 1,
7677
379 => 1,
7778
380 => 1,
7879
384 => 1,
80+
394 => 1,
81+
396 => 1,
82+
403 => 1,
83+
412 => 1,
84+
415 => 1,
85+
416 => 1,
7986
];
8087

8188
default:

0 commit comments

Comments
 (0)