Fix false positives when attributes are used for member vars

squizlabs · Jul 29, 2021 · 64f2a81 · 64f2a81
1 parent 66d7385
commit 64f2a81
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 23 deletions.
diff --git a/src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php b/src/Standards/Squiz/Sniffs/WhiteSpace/MemberVarSpacingSniff.php
@@ -55,40 +55,74 @@ 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);
             if ($tokens[$prevContent]['line'] !== $tokens[$prev]['line']) {
                 // 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
@@ -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
@@ -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
@@ -57,6 +57,10 @@ public function getErrorList()
             288 => 1,
             292 => 1,
             333 => 1,
+            342 => 1,
+            346 => 1,
+            353 => 1,
+            357 => 1,
         ];
 
     }//end getErrorList()