From 64f2a81e24517da68c4cdf6312983fc6be31037e Mon Sep 17 00:00:00 2001 From: Greg Sherwood Date: Thu, 29 Jul 2021 13:22:00 +1000 Subject: [PATCH] Fix false positives when attributes are used for member vars --- .../WhiteSpace/MemberVarSpacingSniff.php | 80 +++++++++++++------ .../WhiteSpace/MemberVarSpacingUnitTest.inc | 24 ++++++ .../MemberVarSpacingUnitTest.inc.fixed | 21 +++++ .../WhiteSpace/MemberVarSpacingUnitTest.php | 4 + 4 files changed, 106 insertions(+), 23 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php b/src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php index f0c84fb8e2..ccd48995e7 100644 --- a/src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php +++ b/src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php @@ -55,11 +55,25 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) $endOfStatement = $phpcsFile->findNext(T_SEMICOLON, ($stackPtr + 1), null, false, null, true); - $ignore = $validPrefixes; - $ignore[] = T_WHITESPACE; + $ignore = $validPrefixes; + $ignore[T_WHITESPACE] = T_WHITESPACE; $start = $startOfStatement; - $prev = $phpcsFile->findPrevious($ignore, ($startOfStatement - 1), null, true); + for ($prev = ($startOfStatement - 1); $prev >= 0; $prev--) { + if (isset($ignore[$tokens[$prev]['code']]) === true) { + continue; + } + + if ($tokens[$prev]['code'] === T_ATTRIBUTE_END + && isset($tokens[$prev]['attribute_opener']) === true + ) { + $prev = $tokens[$prev]['attribute_opener']; + continue; + } + + break; + } + if (isset(Tokens::$commentTokens[$tokens[$prev]['code']]) === true) { // Assume the comment belongs to the member var if it is on a line by itself. $prevContent = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prev - 1), null, true); @@ -67,28 +81,48 @@ protected function processMemberVar(File $phpcsFile, $stackPtr) // Check the spacing, but then skip it. $foundLines = ($tokens[$startOfStatement]['line'] - $tokens[$prev]['line'] - 1); if ($foundLines > 0) { - $error = 'Expected 0 blank lines after member var comment; %s found'; - $data = [$foundLines]; - $fix = $phpcsFile->addFixableError($error, $prev, 'AfterComment', $data); - if ($fix === true) { - $phpcsFile->fixer->beginChangeset(); - // Inline comments have the newline included in the content but - // docblock do not. - if ($tokens[$prev]['code'] === T_COMMENT) { - $phpcsFile->fixer->replaceToken($prev, rtrim($tokens[$prev]['content'])); + for ($i = ($prev + 1); $i < $startOfStatement; $i++) { + if ($tokens[$i]['column'] !== 1) { + continue; } - for ($i = ($prev + 1); $i <= $startOfStatement; $i++) { - if ($tokens[$i]['line'] === $tokens[$startOfStatement]['line']) { - break; - } - - $phpcsFile->fixer->replaceToken($i, ''); - } - - $phpcsFile->fixer->addNewline($prev); - $phpcsFile->fixer->endChangeset(); - } + if ($tokens[$i]['code'] === T_WHITESPACE + && $tokens[$i]['line'] !== $tokens[($i + 1)]['line'] + ) { + $error = 'Expected 0 blank lines after member var comment; %s found'; + $data = [$foundLines]; + $fix = $phpcsFile->addFixableError($error, $prev, 'AfterComment', $data); + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + // Inline comments have the newline included in the content but + // docblocks do not. + if ($tokens[$prev]['code'] === T_COMMENT) { + $phpcsFile->fixer->replaceToken($prev, rtrim($tokens[$prev]['content'])); + } + + for ($i = ($prev + 1); $i <= $startOfStatement; $i++) { + if ($tokens[$i]['line'] === $tokens[$startOfStatement]['line']) { + break; + } + + // Remove the newline after the docblock, and any entirely + // empty lines before the member var. + if ($tokens[$i]['code'] === T_WHITESPACE + && $tokens[$i]['line'] === $tokens[$prev]['line'] + || ($tokens[$i]['column'] === 1 + && $tokens[$i]['line'] !== $tokens[($i + 1)]['line']) + ) { + $phpcsFile->fixer->replaceToken($i, ''); + } + } + + $phpcsFile->fixer->addNewline($prev); + $phpcsFile->fixer->endChangeset(); + }//end if + + break; + }//end if + }//end for }//end if $start = $prev; diff --git a/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc b/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc index fd7c6e34fc..ff467bb2df 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc +++ b/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc @@ -332,3 +332,27 @@ class CommentedOutCodeAtStartOfClassNoBlankLine { */ public $property = true; } + +class HasAttributes +{ + /** + * Short description of the member variable. + * + * @var array + */ + + #[ORM\Id]#[ORM\Column("integer")] + + private $id; + + + /** + * Short description of the member variable. + * + * @var array + */ + #[ORM\GeneratedValue] + + #[ORM\Column(ORM\Column::T_INTEGER)] + protected $height; +} diff --git a/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc.fixed b/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc.fixed index b6ebcc9ab1..5c42b1eda1 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.inc.fixed @@ -319,3 +319,24 @@ class CommentedOutCodeAtStartOfClassNoBlankLine { */ public $property = true; } + +class HasAttributes +{ + + /** + * Short description of the member variable. + * + * @var array + */ + #[ORM\Id]#[ORM\Column("integer")] + private $id; + + /** + * Short description of the member variable. + * + * @var array + */ + #[ORM\GeneratedValue] + #[ORM\Column(ORM\Column::T_INTEGER)] + protected $height; +} diff --git a/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.php b/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.php index 08a11bca41..b43a972b76 100644 --- a/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.php +++ b/src/Standards/Squiz/Tests/WhiteSpace/MemberVarSpacingUnitTest.php @@ -57,6 +57,10 @@ public function getErrorList() 288 => 1, 292 => 1, 333 => 1, + 342 => 1, + 346 => 1, + 353 => 1, + 357 => 1, ]; }//end getErrorList()