Merge pull request #1006 from PHPCSStandards/phpcs-4.0/feature/708-ruleset-improve-property-setting-type-handling

Ruleset: improve type handling of user-set properties

Author: jrfnl

src/Ruleset.php

@@ -1673,43 +1673,74 @@ public function setSniffProperty($sniffClass, $name, $settings)
             return;
         }
 
-        $value = $settings['value'];
+        $value = $this->getRealPropertyValue($settings['value']);
 
-        if (is_string($value) === true) {
-            $value = trim($value);
-        }
-
-        if ($value === '') {
-            $value = null;
-        }
-
-        // Special case for booleans.
-        if ($value === 'true') {
-            $value = true;
-        } else if ($value === 'false') {
-            $value = false;
-        } else if (substr($name, -2) === '[]') {
-            $name   = $propertyName;
+        // Handle properties set inline via phpcs:set.
+        if (substr($name, -2) === '[]') {
             $values = [];
-            if ($value !== null) {
+            if (is_string($value) === true) {
                 foreach (explode(',', $value) as $val) {
                     list($k, $v) = explode('=>', $val.'=>');
                     if ($v !== '') {
-                        $values[trim($k)] = trim($v);
+                        $values[trim($k)] = $v;
                     } else {
-                        $values[] = trim($k);
+                        $values[] = $k;
                     }
                 }
             }
 
-            $value = $values;
+            $value = $this->getRealPropertyValue($values);
         }
 
-        $sniffObject->$name = $value;
+        $sniffObject->$propertyName = $value;
 
     }//end setSniffProperty()
 
 
+    /**
+     * Transform a property value received via a ruleset or inline annotation to a typed value.
+     *
+     * @param string|array<int|string, string> $value The current property value.
+     *
+     * @return mixed
+     */
+    private function getRealPropertyValue($value)
+    {
+        if (is_array($value) === true) {
+            foreach ($value as $k => $v) {
+                $value[$k] = $this->getRealPropertyValue($v);
+            }
+
+            return $value;
+        }
+
+        if (is_string($value) === true) {
+            $value = trim($value);
+
+            if ($value === '') {
+                return null;
+            }
+
+            $valueLc = strtolower($value);
+
+            if ($valueLc === 'true') {
+                return true;
+            }
+
+            if ($valueLc === 'false') {
+                return false;
+            }
+
+            if ($valueLc === 'null') {
+                return null;
+            }
+        }//end if
+
+        return $value;
+
+    }//end getRealPropertyValue()
+
+
     /**
      * Gets the array of ignore patterns.
      *

src/Standards/Generic/Sniffs/PHP/ForbiddenFunctionsSniff.php

@@ -226,9 +226,7 @@ protected function addError($phpcsFile, $stackPtr, $function, $pattern=null)
             $pattern = strtolower($function);
         }
 
-        if ($this->forbiddenFunctions[$pattern] !== null
-            && $this->forbiddenFunctions[$pattern] !== 'null'
-        ) {
+        if ($this->forbiddenFunctions[$pattern] !== null) {
             $type  .= 'WithAlternative';
             $data[] = $this->forbiddenFunctions[$pattern];
             $error .= '; use %s() instead';

tests/Core/Ruleset/Fixtures/PropertyTypeHandlingInline.inc

@@ -12,6 +12,7 @@
 
 // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsNull null
 // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsNullCase NULL
+// phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsNullTrimmed   null
 
 // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsBooleanTrue true
 // phpcs:set TestStandard.SetProperty.PropertyTypeHandling expectsBooleanTrueCase True

tests/Core/Ruleset/Fixtures/TestStandard/Sniffs/SetProperty/PropertyTypeHandlingSniff.php

@@ -53,19 +53,26 @@ final class PropertyTypeHandlingSniff implements Sniff
     public $expectsFloatButAcceptsString;
 
     /**
-     * Used to verify that null gets set as a string.
+     * Used to verify that null gets set as a proper null value.
      *
      * @var null
      */
     public $expectsNull;
 
     /**
-     * Used to verify that null gets set as a string.
+     * Used to verify that null gets set as a proper null value.
      *
      * @var null
      */
     public $expectsNullCase;
 
+    /**
+     * Used to verify that null gets set as a proper null value.
+     *
+     * @var null
+     */
+    public $expectsNullTrimmed;
+
     /**
      * Used to verify that booleans get set as proper boolean values.
      *

tests/Core/Ruleset/PropertyTypeHandlingTest.php

@@ -19,6 +19,7 @@
  *
  * @covers \PHP_CodeSniffer\Ruleset::processRule
  * @covers \PHP_CodeSniffer\Ruleset::setSniffProperty
+ * @covers \PHP_CodeSniffer\Ruleset::getRealPropertyValue
  */
 final class PropertyTypeHandlingTest extends TestCase
 {
@@ -91,17 +92,17 @@ public static function dataTypeHandling()
             'string',
             '10',
             '1.5',
-            'null',
-            'true',
-            'false',
+            null,
+            true,
+            false,
         ];
         $expectedArrayKeysAndValues = [
             'string' => 'string',
             10       => '10',
             'float'  => '1.5',
-            'null'   => 'null',
-            'true'   => 'true',
-            'false'  => 'false',
+            'null'   => null,
+            'true'   => true,
+            'false'  => false,
         ];
 
         return [
@@ -125,21 +126,25 @@ public static function dataTypeHandling()
                 'propertyName' => 'expectsFloatButAcceptsString',
                 'expected'     => '12.345',
             ],
-            'Null value gets set as string'                  => [
+            'Null value gets set as null'                    => [
                 'propertyName' => 'expectsNull',
-                'expected'     => 'null',
+                'expected'     => null,
             ],
-            'Null (uppercase) value gets set as string'      => [
+            'Null (uppercase) value gets set as null'        => [
                 'propertyName' => 'expectsNullCase',
-                'expected'     => 'NULL',
+                'expected'     => null,
+            ],
+            'Null (with spaces) value gets set as null'      => [
+                'propertyName' => 'expectsNullTrimmed',
+                'expected'     => null,
             ],
             'True value gets set as boolean'                 => [
                 'propertyName' => 'expectsBooleanTrue',
                 'expected'     => true,
             ],
             'True (mixed case) value gets set as string'     => [
                 'propertyName' => 'expectsBooleanTrueCase',
-                'expected'     => 'True',
+                'expected'     => true,
             ],
             'True (with spaces) value gets set as boolean'   => [
                 'propertyName' => 'expectsBooleanTrueTrimmed',
@@ -151,7 +156,7 @@ public static function dataTypeHandling()
             ],
             'False (mixed case) value gets set as string'    => [
                 'propertyName' => 'expectsBooleanFalseCase',
-                'expected'     => 'fALSe',
+                'expected'     => false,
             ],
             'False (with spaces) value gets set as boolean'  => [
                 'propertyName' => 'expectsBooleanFalseTrimmed',

tests/Core/Ruleset/PropertyTypeHandlingTest.xml

@@ -12,6 +12,7 @@
 
             <property name="expectsNull" value="null"/>
             <property name="expectsNullCase" value="NULL"/>
+            <property name="expectsNullTrimmed" value="   null"/>
 
             <!-- Also tests that property names get cleaned of surrounding whitespace. -->
             <property name="  expectsBooleanTrue " value="true"/>