PHP 8.0 | PEAR/FunctionDeclaration: fix handling of constructor prope…

…rty promotion Adjusts the `PEAR.Functions.FunctionDeclaration` sniff and by extension, the `Squiz.Functions.MultiLineFunctionDeclaration` and the `PSR12.Classes.AnonClassDeclaration` sniffs to handle constructor properties preceded by docblocks correctly. There is one functional change: The sniffs will no longer throw a `FirstParamSpacing` error when there are blank lines between the function open parenthesis and the first parameter. As the sniffs still throw a `EmptyLine` error in that case - and will auto-fix that error - I've deemed that acceptable as the "fixed" file will still be the same. Fixes 3342
squizlabs · May 10, 2021 · 596bdf9 · 596bdf9
1 parent fde816c
commit 596bdf9
Show file tree

Hide file tree

Showing 9 changed files with 272 additions and 7 deletions.
diff --git a/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php b/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php
@@ -456,9 +456,10 @@ public function processArgumentList($phpcsFile, $stackPtr, $indent, $type='funct
                 }
 
                 // We changed lines, so this should be a whitespace indent token.
-                if ($tokens[$i]['code'] !== T_WHITESPACE) {
-                    $foundIndent = 0;
-                } else if ($tokens[$i]['line'] !== $tokens[($i + 1)]['line']) {
+                $foundIndent = 0;
+                if ($tokens[$i]['code'] === T_WHITESPACE
+                    && $tokens[$i]['line'] !== $tokens[($i + 1)]['line']
+                ) {
                     // This is an empty line, so don't check the indent.
                     $foundIndent = $expectedIndent;
 
@@ -467,8 +468,11 @@ public function processArgumentList($phpcsFile, $stackPtr, $indent, $type='funct
                     if ($fix === true) {
                         $phpcsFile->fixer->replaceToken($i, '');
                     }
-                } else {
+                } else if ($tokens[$i]['code'] === T_WHITESPACE) {
+                    $foundIndent = $tokens[$i]['length'];
+                } else if ($tokens[$i]['code'] === T_DOC_COMMENT_WHITESPACE) {
                     $foundIndent = $tokens[$i]['length'];
+                    ++$expectedIndent;
                 }
 
                 if ($expectedIndent !== $foundIndent) {

diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc
@@ -314,3 +314,61 @@ if(true) {
         ;
     }
 }
+
+class ConstructorPropertyPromotionSingleLineDocblockIndentOK
+{
+    public function __construct(
+        /** @var string */
+        public string $public,
+        /** @var string */
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIndentOK
+{
+    public function __construct(
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        public string $public,
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        #[NotBlank]
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionSingleLineDocblockIncorrectIndent
+{
+    public function __construct(
+      /** @var string */
+      public string $public,
+          /** @var string */
+          private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIncorrectIndent
+{
+    public function __construct(
+    /**
+     * @var string
+     * @Assert\NotBlank()
+     */
+    public string $public,
+/**
+ * @var string
+ * @Assert\NotBlank()
+ */
+#[NotBlank]
+private string $private,
+    ) {
+    }
+}
diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.inc.fixed
@@ -312,3 +312,61 @@ if(true) {
         abstract function baz();
     }
 }
+
+class ConstructorPropertyPromotionSingleLineDocblockIndentOK
+{
+    public function __construct(
+        /** @var string */
+        public string $public,
+        /** @var string */
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIndentOK
+{
+    public function __construct(
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        public string $public,
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        #[NotBlank]
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionSingleLineDocblockIncorrectIndent
+{
+    public function __construct(
+        /** @var string */
+        public string $public,
+        /** @var string */
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIncorrectIndent
+{
+    public function __construct(
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        public string $public,
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        #[NotBlank]
+        private string $private,
+    ) {
+    }
+}
diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php
@@ -82,6 +82,21 @@ public function getErrorList($testFile='FunctionDeclarationUnitTest.inc')
                 309 => 1,
                 313 => 1,
                 314 => 1,
+                350 => 1,
+                351 => 1,
+                352 => 1,
+                353 => 1,
+                361 => 1,
+                362 => 1,
+                363 => 1,
+                364 => 1,
+                365 => 1,
+                366 => 1,
+                367 => 1,
+                368 => 1,
+                369 => 1,
+                370 => 1,
+                371 => 1,
             ];
         } else {
             $errors = [

diff --git a/src/Standards/PSR12/Tests/Classes/AnonClassDeclarationUnitTest.php b/src/Standards/PSR12/Tests/Classes/AnonClassDeclarationUnitTest.php
@@ -32,7 +32,7 @@ public function getErrorList()
             32 => 1,
             33 => 1,
             34 => 1,
-            35 => 2,
+            35 => 1,
             36 => 1,
             37 => 3,
             39 => 1,

diff --git a/src/Standards/Squiz/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php b/src/Standards/Squiz/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php
@@ -200,7 +200,7 @@ public function processBracket($phpcsFile, $openBracket, $tokens, $type='functio
         // The open bracket should be the last thing on the line.
         if ($tokens[$openBracket]['line'] !== $tokens[$closeBracket]['line']) {
             $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($openBracket + 1), null, true);
-            if ($tokens[$next]['line'] !== ($tokens[$openBracket]['line'] + 1)) {
+            if ($tokens[$next]['line'] === $tokens[$openBracket]['line']) {
                 $error = 'The first parameter of a multi-line '.$type.' declaration must be on the line after the opening bracket';
                 $fix   = $phpcsFile->addFixableError($error, $next, $errorPrefix.'FirstParamSpacing');
                 if ($fix === true) {

diff --git a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc
@@ -197,3 +197,61 @@ function foo(
     $bar
 ) {
 }
+
+class ConstructorPropertyPromotionSingleLineDocblockIndentOK
+{
+    public function __construct(
+        /** @var string */
+        public string $public,
+        /** @var string */
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIndentOK
+{
+    public function __construct(
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        public string $public,
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        #[NotBlank]
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionSingleLineDocblockIncorrectIndent
+{
+    public function __construct(
+      /** @var string */
+      public string $public,
+          /** @var string */
+          private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIncorrectIndent
+{
+    public function __construct(
+    /**
+     * @var string
+     * @Assert\NotBlank()
+     */
+    public string $public,
+/**
+ * @var string
+ * @Assert\NotBlank()
+ */
+#[NotBlank]
+private string $private,
+    ) {
+    }
+}
diff --git a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed
@@ -209,3 +209,61 @@ function foo(
     $bar
 ) {
 }
+
+class ConstructorPropertyPromotionSingleLineDocblockIndentOK
+{
+    public function __construct(
+        /** @var string */
+        public string $public,
+        /** @var string */
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIndentOK
+{
+    public function __construct(
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        public string $public,
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        #[NotBlank]
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionSingleLineDocblockIncorrectIndent
+{
+    public function __construct(
+        /** @var string */
+        public string $public,
+        /** @var string */
+        private string $private,
+    ) {
+    }
+}
+
+class ConstructorPropertyPromotionMultiLineDocblockAndAttributeIncorrectIndent
+{
+    public function __construct(
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        public string $public,
+        /**
+         * @var string
+         * @Assert\NotBlank()
+         */
+        #[NotBlank]
+        private string $private,
+    ) {
+    }
+}
diff --git a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php
@@ -55,7 +55,21 @@ public function getErrorList($testFile='MultiLineFunctionDeclarationUnitTest.inc
                 190 => 2,
                 194 => 1,
                 195 => 1,
-                196 => 1,
+                233 => 1,
+                234 => 1,
+                235 => 1,
+                236 => 1,
+                244 => 1,
+                245 => 1,
+                246 => 1,
+                247 => 1,
+                248 => 1,
+                249 => 1,
+                250 => 1,
+                251 => 1,
+                252 => 1,
+                253 => 1,
+                254 => 1,
             ];
         } else {
             $errors = [