Skip to content

Commit 64f2a81

Browse files
committed
Fix false positives when attributes are used for member vars
1 parent 66d7385 commit 64f2a81

File tree

4 files changed

+106
-23
lines changed

4 files changed

+106
-23
lines changed

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

+57-23
Original file line numberDiff line numberDiff line change
@@ -55,40 +55,74 @@ protected function processMemberVar(File $phpcsFile, $stackPtr)
5555

5656
$endOfStatement = $phpcsFile->findNext(T_SEMICOLON, ($stackPtr + 1), null, false, null, true);
5757

58-
$ignore = $validPrefixes;
59-
$ignore[] = T_WHITESPACE;
58+
$ignore = $validPrefixes;
59+
$ignore[T_WHITESPACE] = T_WHITESPACE;
6060

6161
$start = $startOfStatement;
62-
$prev = $phpcsFile->findPrevious($ignore, ($startOfStatement - 1), null, true);
62+
for ($prev = ($startOfStatement - 1); $prev >= 0; $prev--) {
63+
if (isset($ignore[$tokens[$prev]['code']]) === true) {
64+
continue;
65+
}
66+
67+
if ($tokens[$prev]['code'] === T_ATTRIBUTE_END
68+
&& isset($tokens[$prev]['attribute_opener']) === true
69+
) {
70+
$prev = $tokens[$prev]['attribute_opener'];
71+
continue;
72+
}
73+
74+
break;
75+
}
76+
6377
if (isset(Tokens::$commentTokens[$tokens[$prev]['code']]) === true) {
6478
// Assume the comment belongs to the member var if it is on a line by itself.
6579
$prevContent = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true);
6680
if ($tokens[$prevContent]['line'] !== $tokens[$prev]['line']) {
6781
// Check the spacing, but then skip it.
6882
$foundLines = ($tokens[$startOfStatement]['line'] - $tokens[$prev]['line'] - 1);
6983
if ($foundLines > 0) {
70-
$error = 'Expected 0 blank lines after member var comment; %s found';
71-
$data = [$foundLines];
72-
$fix = $phpcsFile->addFixableError($error, $prev, 'AfterComment', $data);
73-
if ($fix === true) {
74-
$phpcsFile->fixer->beginChangeset();
75-
// Inline comments have the newline included in the content but
76-
// docblock do not.
77-
if ($tokens[$prev]['code'] === T_COMMENT) {
78-
$phpcsFile->fixer->replaceToken($prev, rtrim($tokens[$prev]['content']));
84+
for ($i = ($prev + 1); $i < $startOfStatement; $i++) {
85+
if ($tokens[$i]['column'] !== 1) {
86+
continue;
7987
}
8088

81-
for ($i = ($prev + 1); $i <= $startOfStatement; $i++) {
82-
if ($tokens[$i]['line'] === $tokens[$startOfStatement]['line']) {
83-
break;
84-
}
85-
86-
$phpcsFile->fixer->replaceToken($i, '');
87-
}
88-
89-
$phpcsFile->fixer->addNewline($prev);
90-
$phpcsFile->fixer->endChangeset();
91-
}
89+
if ($tokens[$i]['code'] === T_WHITESPACE
90+
&& $tokens[$i]['line'] !== $tokens[($i + 1)]['line']
91+
) {
92+
$error = 'Expected 0 blank lines after member var comment; %s found';
93+
$data = [$foundLines];
94+
$fix = $phpcsFile->addFixableError($error, $prev, 'AfterComment', $data);
95+
if ($fix === true) {
96+
$phpcsFile->fixer->beginChangeset();
97+
// Inline comments have the newline included in the content but
98+
// docblocks do not.
99+
if ($tokens[$prev]['code'] === T_COMMENT) {
100+
$phpcsFile->fixer->replaceToken($prev, rtrim($tokens[$prev]['content']));
101+
}
102+
103+
for ($i = ($prev + 1); $i <= $startOfStatement; $i++) {
104+
if ($tokens[$i]['line'] === $tokens[$startOfStatement]['line']) {
105+
break;
106+
}
107+
108+
// Remove the newline after the docblock, and any entirely
109+
// empty lines before the member var.
110+
if ($tokens[$i]['code'] === T_WHITESPACE
111+
&& $tokens[$i]['line'] === $tokens[$prev]['line']
112+
|| ($tokens[$i]['column'] === 1
113+
&& $tokens[$i]['line'] !== $tokens[($i + 1)]['line'])
114+
) {
115+
$phpcsFile->fixer->replaceToken($i, '');
116+
}
117+
}
118+
119+
$phpcsFile->fixer->addNewline($prev);
120+
$phpcsFile->fixer->endChangeset();
121+
}//end if
122+
123+
break;
124+
}//end if
125+
}//end for
92126
}//end if
93127

94128
$start = $prev;

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

+24
Original file line numberDiff line numberDiff line change
@@ -332,3 +332,27 @@ class CommentedOutCodeAtStartOfClassNoBlankLine {
332332
*/
333333
public $property = true;
334334
}
335+
336+
class HasAttributes
337+
{
338+
/**
339+
* Short description of the member variable.
340+
*
341+
* @var array
342+
*/
343+
344+
#[ORM\Id]#[ORM\Column("integer")]
345+
346+
private $id;
347+
348+
349+
/**
350+
* Short description of the member variable.
351+
*
352+
* @var array
353+
*/
354+
#[ORM\GeneratedValue]
355+
356+
#[ORM\Column(ORM\Column::T_INTEGER)]
357+
protected $height;
358+
}

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

+21
Original file line numberDiff line numberDiff line change
@@ -319,3 +319,24 @@ class CommentedOutCodeAtStartOfClassNoBlankLine {
319319
*/
320320
public $property = true;
321321
}
322+
323+
class HasAttributes
324+
{
325+
326+
/**
327+
* Short description of the member variable.
328+
*
329+
* @var array
330+
*/
331+
#[ORM\Id]#[ORM\Column("integer")]
332+
private $id;
333+
334+
/**
335+
* Short description of the member variable.
336+
*
337+
* @var array
338+
*/
339+
#[ORM\GeneratedValue]
340+
#[ORM\Column(ORM\Column::T_INTEGER)]
341+
protected $height;
342+
}

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

+4
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,10 @@ public function getErrorList()
5757
288 => 1,
5858
292 => 1,
5959
333 => 1,
60+
342 => 1,
61+
346 => 1,
62+
353 => 1,
63+
357 => 1,
6064
];
6165

6266
}//end getErrorList()

0 commit comments

Comments
 (0)