PHP 8.0 | TypeDeclaration sniffs: examine properties declared in the constructor correctly

PHPCompatibility/Sniffs/Classes/NewTypedPropertiesSniff.php

@@ -14,6 +14,8 @@
 use PHPCompatibility\Helpers\ComplexVersionNewFeatureTrait;
 use PHP_CodeSniffer\Files\File;
 use PHP_CodeSniffer\Exceptions\RuntimeException;
+use PHPCSUtils\Utils\FunctionDeclarations;
+use PHPCSUtils\Utils\Scopes;
 use PHPCSUtils\Utils\Variables;
 
 /**
@@ -111,7 +113,10 @@ class NewTypedPropertiesSniff extends Sniff
      */
     public function register()
     {
-        return [\T_VARIABLE];
+        return [
+            \T_VARIABLE,
+            \T_FUNCTION,
+        ];
     }
 
     /**
@@ -129,40 +134,105 @@ public function register()
      */
     public function process(File $phpcsFile, $stackPtr)
     {
-        try {
-            $properties = Variables::getMemberProperties($phpcsFile, $stackPtr);
-        } catch (RuntimeException $e) {
-            // Not a class property.
+        $tokens = $phpcsFile->getTokens();
+
+        if ($tokens[$stackPtr]['code'] === \T_VARIABLE) {
+            try {
+                $properties = Variables::getMemberProperties($phpcsFile, $stackPtr);
+            } catch (RuntimeException $e) {
+                // Not a class property.
+                return;
+            }
+
+            if ($properties['type'] === '') {
+                // Not a typed property.
+                return;
+            }
+
+            $this->checkType($phpcsFile, $properties['type_token'], $properties);
+
+            $endOfStatement = $phpcsFile->findNext(\T_SEMICOLON, ($stackPtr + 1));
+            if ($endOfStatement !== false) {
+                // Don't throw the same error multiple times for multi-property declarations.
+                return ($endOfStatement + 1);
+            }
+
             return;
         }
 
-        if ($properties['type'] === '') {
-            // Not a typed property.
+        /*
+         * This must be a function declaration. Let's check for constructor property promotion.
+         */
+        if (Scopes::isOOMethod($phpcsFile, $stackPtr) === false) {
+            // Global function.
             return;
         }
 
-        // Still here ? In that case, this will be a typed property.
-        $type      = \ltrim($properties['type'], '?'); // Trim off potential nullability.
-        $type      = \strtolower($type);
-        $typeToken = $properties['type_token'];
+        $functionName = FunctionDeclarations::getName($phpcsFile, $stackPtr);
+        if (\strtolower($functionName) !== '__construct') {
+            // Not a class constructor.
+            return;
+        }
+
+        $parameters = FunctionDeclarations::getParameters($phpcsFile, $stackPtr);
+        foreach ($parameters as $param) {
+            if (empty($param['property_visibility']) === true) {
+                // Not property promotion.
+                continue;
+            }
+
+            // Juggle some of the array entries to what it expected for properties.
+            $paramInfo = [
+                'type'          => $param['type_hint'],
+                'nullable_type' => $param['nullable_type'],
+                'param_name'    => $param['name'],
+            ];
+
+            $this->checkType($phpcsFile, $param['type_hint_token'], $paramInfo);
+        }
+    }
+
+    /**
+     * Processes this test, when one of its tokens is encountered.
+     *
+     * @since 10.0.0 Split off from the process() method to allow for examining constructor
+     *               property promotion parameters.
+     *
+     * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
+     * @param int                         $typeToken The position of the current token in the
+     *                                               stack passed in $tokens.
+     * @param array                       $typeInfo  Information about the current property.
+     *
+     * @return void
+     */
+    protected function checkType(File $phpcsFile, $typeToken, $typeInfo)
+    {
+        $origType = $typeInfo['type'];
+        $type     = \ltrim($origType, '?'); // Trim off potential nullability.
+        $type     = \strtolower($type);
+
+        $errorSuffix = '';
+        if (isset($typeInfo['param_name']) === true) {
+            $errorSuffix = ' (promoted property ' . $typeInfo['param_name'] . ')';
+        }
 
         if ($this->supportsBelow('7.3') === true) {
             $phpcsFile->addError(
-                'Typed properties are not supported in PHP 7.3 or earlier. Found: %s',
+                'Typed properties are not supported in PHP 7.3 or earlier. Found: %s' . $errorSuffix,
                 $typeToken,
                 'Found',
-                [$type]
+                [$origType]
             );
         } else {
             $types       = \explode('|', $type);
             $isUnionType = (\strpos($type, '|') !== false);
 
             if ($this->supportsBelow('7.4') === true && $isUnionType === true) {
                 $phpcsFile->addError(
-                    'Union types are not present in PHP version 7.4 or earlier. Found: %s',
+                    'Union types are not present in PHP version 7.4 or earlier. Found: %s' . $errorSuffix,
                     $typeToken,
                     'UnionTypeFound',
-                    [$properties['type']]
+                    [$origType]
                 );
             }
 
@@ -178,11 +248,11 @@ public function process(File $phpcsFile, $stackPtr)
                      * to PHP 8 if the type hint referred to a class named "Mixed".
                      * Only throw an error if PHP 8+ needs to be supported.
                      */
-                    if (($type === 'mixed' && $properties['nullable_type'] === true)
+                    if (($type === 'mixed' && $typeInfo['nullable_type'] === true)
                         && $this->supportsAbove('8.0') === true
                     ) {
                         $phpcsFile->addError(
-                            'Mixed types cannot be nullable, null is already part of the mixed type',
+                            'Mixed types cannot be nullable, null is already part of the mixed type' . $errorSuffix,
                             $typeToken,
                             'NullableMixed'
                         );
@@ -197,24 +267,18 @@ public function process(File $phpcsFile, $stackPtr)
                         );
                     }
                 } elseif (isset($this->invalidTypes[$type])) {
-                    $error = '%s is not supported as a type declaration for properties';
+                    $error = '%s is not supported as a type declaration for properties' . $errorSuffix;
                     $data  = [$type];
 
                     if ($this->invalidTypes[$type] !== false) {
-                        $error .= ' Did you mean %s ?';
+                        $error .= '. Did you mean %s ?';
                         $data[] = $this->invalidTypes[$type];
                     }
 
                     $phpcsFile->addError($error, $typeToken, 'InvalidType', $data);
                 }
             }
         }
-
-        $endOfStatement = $phpcsFile->findNext(\T_SEMICOLON, ($stackPtr + 1));
-        if ($endOfStatement !== false) {
-            // Don't throw the same error multiple times for multi-property declarations.
-            return ($endOfStatement + 1);
-        }
     }
 
 

PHPCompatibility/Sniffs/FunctionDeclarations/NewParamTypeDeclarationsSniff.php

@@ -202,6 +202,11 @@ public function process(File $phpcsFile, $stackPtr)
                 continue;
             }
 
+            if (empty($param['property_visibility']) === false) {
+                // Constructor property promotion, these are examined by the NewTypedProperties sniff.
+                continue;
+            }
+
             if ($supportsPHP4 === true) {
                 $phpcsFile->addError(
                     'Type declarations were not present in PHP 4.4 or earlier.',

PHPCompatibility/Tests/Classes/NewTypedPropertiesUnitTest.inc

@@ -107,3 +107,21 @@ $anon = class() {
     // Intentional fatal error - duplicate types are not allowed in union types, but that's not the concern of the sniff.
     public int|string|INT $duplicateTypeInUnion;
 };
+
+// PHP 8.0 constructor property promotion.
+class ConstructorPropertyPromotionWithTypes {
+    public function __construct(
+        protected float|int $x,
+        public ?string &$y = 'test',
+        private mixed $z,
+        public callable $callMe,
+        callable $normalParamIgnore1,
+        ?mixed $normalParamIgnore2,
+    ) {}
+
+    // Ignore. Not a constructor, so no property promotion.
+    public function notAConstructor(float|int $x, callable $callMe) {}
+}
+
+// Ignore. Not a constructor, so no property promotion.
+function __construct(float|int $x, callable $callMe) {}

PHPCompatibility/Tests/Classes/NewTypedPropertiesUnitTest.php

@@ -88,24 +88,52 @@ public function dataNewTypedProperties()
             [102],
             [105],
             [108],
+            [114],
+            [115, true],
+            [116],
+            [117],
         ];
     }
 
 
     /**
      * Verify the sniff doesn't throw false positives for non-typed properties.
      *
+     * @dataProvider dataNoFalsePositives
+     *
+     * @param int $line The line number.
+     *
      * @return void
      */
-    public function testNoFalsePositivesNewTypedProperties()
+    public function testNoFalsePositivesNewTypedProperties($line)
     {
         $file = $this->sniffFile(__FILE__, '7.3');
+        $this->assertNoViolation($file, $line);
+    }
 
-        for ($line = 1; $line < 19; $line++) {
-            $this->assertNoViolation($file, $line);
+    /**
+     * Data provider.
+     *
+     * @see testNoFalsePositives()
+     *
+     * @return array
+     */
+    public function dataNoFalsePositives()
+    {
+        $cases = [];
+        // No errors expected on the first 19 lines.
+        for ($line = 1; $line <= 19; $line++) {
+            $cases[] = [$line];
         }
-    }
 
+        // Don't throw errors for normal constructor/function parameters.
+        $cases[] = [118];
+        $cases[] = [119];
+        $cases[] = [123];
+        $cases[] = [127];
+
+        return $cases;
+    }
 
     /**
      * Verify that invalid type declarations are flagged correctly.
@@ -138,6 +166,7 @@ public function dataInvalidPropertyType()
             [64, 'callable'],
             [65, 'boolean'],
             [66, 'integer'],
+            [117, 'callable'],
         ];
     }
 
@@ -185,6 +214,7 @@ public function dataNewTypedPropertyTypes()
             ['null', '7.4', 93, '8.0', false],
             ['false', '7.4', 96, '8.0', false],
             ['false', '7.4', 99, '8.0'],
+            ['mixed', '7.4', 116, '8.0', false],
         ];
     }
 
@@ -256,6 +286,7 @@ public function dataNewUnionTypes()
             ['object|ClassName', 102],
             ['iterable|array|Traversable', 105],
             ['int|string|INT', 108],
+            ['float|int', 114],
         ];
     }
 

PHPCompatibility/Tests/FunctionDeclarations/NewParamTypeDeclarationsUnitTest.inc

@@ -120,3 +120,15 @@ function pseudoTypeIterableAndArray(iterable|array|Traversable $var) {}
 
 // Intentional fatal error - duplicate types are not allowed in union types, but that's not the concern of the sniff.
 function duplicateTypeInUnion(int|string|INT $var) {}
+
+// PHP 8.0 constructor property promotion. Only the "normal" parameters should be examined.
+class ConstructorPropertyPromotionWithTypes {
+    public function __construct(
+        protected float|int $propertyIgnore1,
+        private mixed $propertyIgnore2,
+        public callable $propertyIgnore3,
+        callable $normalParam1,
+        float|int $normalParam2,
+        mixed $normalParam3,
+    ) {}
+}

PHPCompatibility/Tests/FunctionDeclarations/NewParamTypeDeclarationsUnitTest.php

@@ -118,6 +118,11 @@ public function dataNewTypeDeclaration()
             ['array', '5.0', 119, '8.0'],
             ['int', '5.6', 122, '8.0'], // Expected x2.
             ['string', '5.6', 122, '8.0'],
+
+            ['callable', '5.3', 130, '5.4'],
+            ['float', '5.6', 131, '8.0'],
+            ['int', '5.6', 131, '8.0'],
+            ['mixed', '7.4', 132, '8.0', false],
         ];
     }
 
@@ -290,6 +295,7 @@ public function dataNewUnionTypes()
             ['object|ClassName', 116],
             ['iterable|array|Traversable', 119],
             ['int|string|INT', 122],
+            ['float|int', 131],
         ];
     }
 
@@ -469,6 +475,9 @@ public function dataTypeDeclaration()
             [116],
             [119],
             [122],
+            [130],
+            [131],
+            [132],
         ];
     }
 
@@ -502,6 +511,9 @@ public function dataNoFalsePositives()
             [49],
             [82],
             [91],
+            [127],
+            [128],
+            [129],
         ];
     }