Implement reportInsideIsset flag

shipmonk-rnd · Jul 10, 2024 · cd60f87 · cd60f87
1 parent 375ac13
commit cd60f87
Show file tree

Hide file tree

Showing 6 changed files with 43 additions and 10 deletions.
diff --git a/README.md b/README.md
@@ -89,6 +89,7 @@ parameters:
         forbidUnsafeArrayKey:
             enabled: true
             reportMixed: true
+            reportInsideIsset: true
         forbidVariableTypeOverwriting:
             enabled: true
         forbidUnsetClassField:
@@ -619,7 +620,8 @@ parameters:
 - PHP casts `null`, `float` and `bool` to some nearest int/string
   - You should rather do that intentionally and explicitly
   - Those types are the main difference to default PHPStan behaviour which allows using them as array keys
-- You can exclude reporting `mixed` keys via configuration
+- You can exclude reporting `mixed` keys via `reportMixed` configuration
+- You can exclude reporting `isset($array[$invalid])` and `$array[$invalid] ?? null` via `reportInsideIsset` configuration
 
 ```php
 $taxRates = [ // denied, float key gets casted to int (causing $taxRates to contain one item)
@@ -633,6 +635,7 @@ parameters:
     shipmonkRules:
         forbidUnsafeArrayKey:
             reportMixed: false # defaults to true
+            reportInsideIsset: false # defaults to true
 ```
 
 

diff --git a/rules.neon b/rules.neon
@@ -68,6 +68,7 @@ parameters:
         forbidUnsafeArrayKey:
             enabled: true
             reportMixed: true
+            reportInsideIsset: true
         forbidVariableTypeOverwriting:
             enabled: true
         forbidUnsetClassField:
@@ -183,6 +184,7 @@ parametersSchema:
         forbidUnsafeArrayKey: structure([
             enabled: bool()
             reportMixed: bool()
+            reportInsideIsset: bool()
         ])
         forbidVariableTypeOverwriting: structure([
             enabled: bool()
@@ -382,6 +384,7 @@ services:
         class: ShipMonk\PHPStan\Rule\ForbidUnsafeArrayKeyRule
         arguments:
             reportMixed: %shipmonkRules.forbidUnsafeArrayKey.reportMixed%
+            reportInsideIsset: %shipmonkRules.forbidUnsafeArrayKey.reportInsideIsset%
     -
         class: ShipMonk\PHPStan\Rule\ForbidVariableTypeOverwritingRule
     -

diff --git a/src/Rule/ForbidUnsafeArrayKeyRule.php b/src/Rule/ForbidUnsafeArrayKeyRule.php
@@ -24,9 +24,15 @@ class ForbidUnsafeArrayKeyRule implements Rule
 
     private bool $reportMixed;
 
-    public function __construct(bool $reportMixed)
+    private bool $reportInsideIsset;
+
+    public function __construct(
+        bool $reportMixed,
+        bool $reportInsideIsset
+    )
     {
         $this->reportMixed = $reportMixed;
+        $this->reportInsideIsset = $reportInsideIsset;
     }
 
     public function getNodeType(): string
@@ -59,6 +65,10 @@ public function processNode(
             && $node->dim !== null
             && !$scope->getType($node->var)->isArray()->no()
         ) {
+            if (!$this->reportInsideIsset && $scope->isUndefinedExpressionAllowed($node)) {
+                return [];
+            }
+
             $dimType = $scope->getType($node->dim);
 
             if (!$this->isArrayKey($dimType)) {

diff --git a/tests/Rule/ForbidUnsafeArrayKeyRuleTest.php b/tests/Rule/ForbidUnsafeArrayKeyRuleTest.php
@@ -15,27 +15,36 @@ class ForbidUnsafeArrayKeyRuleTest extends RuleTestCase
 
     private ?bool $checkMixed = null;
 
+    private ?bool $checkInsideIsset = null;
+
     protected function getRule(): Rule
     {
         if ($this->checkMixed === null) {
             throw new LogicException('Property checkMixed must be set');
         }
 
+        if ($this->checkInsideIsset === null) {
+            throw new LogicException('Property checkInsideIsset must be set');
+        }
+
         return new ForbidUnsafeArrayKeyRule(
             $this->checkMixed,
+            $this->checkInsideIsset,
         );
     }
 
-    public function testMixedDisabled(): void
+    public function testStrict(): void
     {
-        $this->checkMixed = false;
-        $this->analyseFile(__DIR__ . '/data/ForbidUnsafeArrayKeyRule/mixed-disabled.php');
+        $this->checkMixed = true;
+        $this->checkInsideIsset = true;
+        $this->analyseFile(__DIR__ . '/data/ForbidUnsafeArrayKeyRule/default.php');
     }
 
-    public function testMixedEnabled(): void
+    public function testLessStrict(): void
     {
-        $this->checkMixed = true;
-        $this->analyseFile(__DIR__ . '/data/ForbidUnsafeArrayKeyRule/mixed-enabled.php');
+        $this->checkMixed = false;
+        $this->checkInsideIsset = false;
+        $this->analyseFile(__DIR__ . '/data/ForbidUnsafeArrayKeyRule/less-strict.php');
     }
 
 }
diff --git a/...orbidUnsafeArrayKeyRule/mixed-enabled.php → ...data/ForbidUnsafeArrayKeyRule/default.php b/...orbidUnsafeArrayKeyRule/mixed-enabled.php → ...data/ForbidUnsafeArrayKeyRule/default.php
@@ -1,6 +1,6 @@
 <?php declare(strict_types = 1);
 
-namespace ForbidUnsafeArrayKeyRule\MixedEnabled;
+namespace ForbidUnsafeArrayKeyRule\MixedDisabled;
 
 
 class ArrayKey
@@ -33,6 +33,10 @@ public function test(
         $array[$arrayKey] = '';
         $weakMap[$object] = '';
 
+        $a = $array[$float] ?? ''; // error: Array key must be integer or string, but float given.
+        $b = isset($array[$float]); // error: Array key must be integer or string, but float given.
+        $c = empty($array[$float]); // error: Array key must be integer or string, but float given.
+
         [
             $int => $int,
             $string => $string,

diff --git a/...rbidUnsafeArrayKeyRule/mixed-disabled.php → .../ForbidUnsafeArrayKeyRule/less-strict.php b/...rbidUnsafeArrayKeyRule/mixed-disabled.php → .../ForbidUnsafeArrayKeyRule/less-strict.php
@@ -1,6 +1,6 @@
 <?php declare(strict_types = 1);
 
-namespace ForbidUnsafeArrayKeyRule\MixedDisabled;
+namespace ForbidUnsafeArrayKeyRule\MixedEnabled;
 
 
 class ArrayKey
@@ -33,6 +33,10 @@ public function test(
         $array[$arrayKey] = '';
         $weakMap[$object] = '';
 
+        $a = $array[$float] ?? '';
+        $b = isset($array[$float]);
+        $c = empty($array[$float]);
+
         [
             $int => $int,
             $string => $string,