Merge pull request #518 from veewee/improved-bounds-calculations

Fix invalid array bounds during static analysis
phpro · May 21, 2024 · 66eb962 · 66eb962
2 parents 03d3123 + c2b02c4
commit 66eb962
Show file tree

Hide file tree

Showing 11 changed files with 53 additions and 36 deletions.
diff --git a/src/Phpro/SoapClient/CodeGenerator/TypeEnhancer/Calculator/ArrayBoundsCalculator.php b/src/Phpro/SoapClient/CodeGenerator/TypeEnhancer/Calculator/ArrayBoundsCalculator.php
@@ -7,16 +7,22 @@
 
 final class ArrayBoundsCalculator
 {
+    /**
+     * All lists will start from index 0.
+     *
+     * The maximum amount of items in the list will be maxOccurs - 1, since arrays are zero-index based.
+     * Edge cases like -1 (unbounded) and 0 (empty) are handled as well.
+     *
+     * These bounds don't take into account minOccurs, since minOccurs still starts from 0.
+     */
     public function __invoke(TypeMeta $meta): string
     {
-        $min = $meta->minOccurs()
-            ->map(fn (int $min): string => $min === -1 ? 'min' : (string) $min)
-            ->unwrapOr('min');
+        $max = $meta->maxOccurs()->unwrapOr(-1);
 
-        $max = $meta->maxOccurs()
-            ->map(fn (int $max): string => $max === -1 ? 'max' : (string) $max)
-            ->unwrapOr('max');
-
-        return 'int<'.$min.','.$max.'>';
+        return match (true) {
+            $max < 0 => 'int<0,max>',
+            $max === 0 => 'never',
+            default => 'int<0,'.($max - 1).'>'
+        };
     }
 }
diff --git a/src/Phpro/SoapClient/CodeGenerator/TypeEnhancer/MetaTypeEnhancer.php b/src/Phpro/SoapClient/CodeGenerator/TypeEnhancer/MetaTypeEnhancer.php
@@ -30,7 +30,10 @@ public function asDocBlockType(string $type): string
 
         $isArray = $this->meta->isList()->unwrapOr(false);
         if ($isArray) {
-            $type = 'array<'.(new ArrayBoundsCalculator())($this->meta).', '.$type.'>';
+            $nonEmpty = $this->meta->minOccurs()->unwrapOr(0) > 0;
+            $arrayType = $nonEmpty ? 'non-empty-array' : 'array';
+
+            $type = $arrayType.'<'.(new ArrayBoundsCalculator())($this->meta).', '.$type.'>';
         }
 
         $isNullable = (new IsConsideredNullableType())($this->meta);

diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/ConstructorAssemblerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/ConstructorAssemblerTest.php
@@ -174,7 +174,7 @@ class MyType
     /**
      * Constructor
      *
-     * @param array<int<min,max>, string> \$prop1
+     * @param array<int<0,max>, string> \$prop1
      */
     public function __construct(array \$prop1)
     {

diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/FluentSetterAssemblerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/FluentSetterAssemblerTest.php
@@ -218,7 +218,7 @@ function it_assembles_a_fluent_setter_with_advanced_types()
 class MyType
 {
     /**
-     * @param array<int<min,max>, string> \$prop1
+     * @param array<int<0,max>, string> \$prop1
      * @return \$this
      */
     public function setProp1(array \$prop1) : static

diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/GetterAssemblerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/GetterAssemblerTest.php
@@ -276,7 +276,7 @@ function it_assembles_a_property_with_advanced_types()
 class MyType
 {
     /**
-     * @return array<int<min,max>, string>
+     * @return array<int<0,max>, string>
      */
     public function getProp1() : array
     {

diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/ImmutableSetterAssemblerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/ImmutableSetterAssemblerTest.php
@@ -265,7 +265,7 @@ function it_assembles_a_fluent_setter_with_advanced_types()
 class MyType
 {
     /**
-     * @param array<int<min,max>, string> \$prop1
+     * @param array<int<0,max>, string> \$prop1
      * @return static
      */
     public function withProp1(array \$prop1) : static

diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/IteratorAssemblerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/IteratorAssemblerTest.php
@@ -57,15 +57,15 @@ function it_assembles_a_type()
 use IteratorAggregate;
 
 /**
- * @phpstan-implements \IteratorAggregate<int<1,2>, string>
- * @psalm-implements \IteratorAggregate<int<1,2>, string>
+ * @phpstan-implements \IteratorAggregate<int<0,1>, string>
+ * @psalm-implements \IteratorAggregate<int<0,1>, string>
  */
 class MyType implements IteratorAggregate
 {
     /**
      * @return \ArrayIterator|string[]
-     * @phpstan-return \ArrayIterator<int<1,2>, string>
-     * @psalm-return \ArrayIterator<int<1,2>, string>
+     * @phpstan-return \ArrayIterator<int<0,1>, string>
+     * @psalm-return \ArrayIterator<int<0,1>, string>
      */
     public function getIterator() : \ArrayIterator
     {
@@ -98,15 +98,15 @@ function it_assembles_a_type_with_no_occurs_information()
 use IteratorAggregate;
 
 /**
- * @phpstan-implements \IteratorAggregate<int<min,max>, string>
- * @psalm-implements \IteratorAggregate<int<min,max>, string>
+ * @phpstan-implements \IteratorAggregate<int<0,max>, string>
+ * @psalm-implements \IteratorAggregate<int<0,max>, string>
  */
 class MyType implements IteratorAggregate
 {
     /**
      * @return \ArrayIterator|string[]
-     * @phpstan-return \ArrayIterator<int<min,max>, string>
-     * @psalm-return \ArrayIterator<int<min,max>, string>
+     * @phpstan-return \ArrayIterator<int<0,max>, string>
+     * @psalm-return \ArrayIterator<int<0,max>, string>
      */
     public function getIterator() : \ArrayIterator
     {

diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/PropertyAssemblerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/PropertyAssemblerTest.php
@@ -222,7 +222,7 @@ function it_assembles_properties_with_advanced_types()
 class MyType
 {
     /**
-     * @var array<int<min,max>, string>
+     * @var array<int<0,max>, string>
      */
     private array \$prop1;
 }

diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/SetterAssemblerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/Assembler/SetterAssemblerTest.php
@@ -181,7 +181,7 @@ function it_assembles_a_setter_with_advanced_types()
 class MyType
 {
     /**
-     * @param array<int<min,max>, string> \$prop1
+     * @param array<int<0,max>, string> \$prop1
      */
     public function setProp1(array \$prop1) : void
     {

diff --git a/...oTest/SoapClient/Unit/CodeGenerator/TypeEnhancer/Calculator/ArrayBoundsCalculatorTest.php b/...oTest/SoapClient/Unit/CodeGenerator/TypeEnhancer/Calculator/ArrayBoundsCalculatorTest.php
@@ -27,23 +27,31 @@ public function provideExpectations()
     {
         yield 'simpleType' => [
             new TypeMeta(),
-            'int<min,max>',
+            'int<0,max>',
         ];
         yield 'array' => [
             (new TypeMeta())->withIsList(true),
-            'int<min,max>',
+            'int<0,max>',
         ];
         yield 'min' => [
             (new TypeMeta())->withIsList(true)->withMinOccurs(1),
-            'int<1,max>',
+            'int<0,max>',
         ];
         yield 'max' => [
             (new TypeMeta())->withIsList(true)->withMaxOccurs(3),
-            'int<min,3>',
+            'int<0,2>',
         ];
         yield 'min-max' => [
             (new TypeMeta())->withIsList(true)->withMinOccurs(1)->withMaxOccurs(3),
-            'int<1,3>',
+            'int<0,2>',
+        ];
+        yield 'max-1' => [
+            (new TypeMeta())->withIsList(true)->withMaxOccurs(1),
+            'int<0,0>',
+        ];
+        yield 'max-0' => [
+            (new TypeMeta())->withIsList(true)->withMaxOccurs(0),
+            'never',
         ];
     }
 }
diff --git a/test/PhproTest/SoapClient/Unit/CodeGenerator/TypeEnhancer/MetaTypeEnhancerTest.php b/test/PhproTest/SoapClient/Unit/CodeGenerator/TypeEnhancer/MetaTypeEnhancerTest.php
@@ -36,19 +36,19 @@ public function provideExpectations()
         yield 'array' => [
             (new TypeMeta())->withIsList(true),
             'simple',
-            'array<int<min,max>, simple>',
+            'array<int<0,max>, simple>',
             'array',
         ];
         yield 'min' => [
             (new TypeMeta())->withIsList(true)->withMinOccurs(1),
             'simple',
-            'array<int<1,max>, simple>',
+            'non-empty-array<int<0,max>, simple>',
             'array',
         ];
         yield 'max' => [
             (new TypeMeta())->withIsList(true)->withMaxOccurs(3),
             'simple',
-            'array<int<min,3>, simple>',
+            'array<int<0,2>, simple>',
             'array',
         ];
         yield 'nullable' => [
@@ -60,7 +60,7 @@ public function provideExpectations()
         yield 'nullable-array' => [
             (new TypeMeta())->withIsList(true)->withIsNullable(true),
             'simple',
-            'null | array<int<min,max>, simple>',
+            'null | array<int<0,max>, simple>',
             '?array',
         ];
         yield 'enum' => [
@@ -72,13 +72,13 @@ public function provideExpectations()
         yield 'enum-list' => [
             (new TypeMeta())->withEnums(['a', 'b'])->withIsList(true),
             'string',
-            "array<int<min,max>, 'a' | 'b'>",
+            "array<int<0,max>, 'a' | 'b'>",
             'array',
         ];
         yield 'nullable-enum-list' => [
             (new TypeMeta())->withEnums(['a', 'b'])->withIsList(true)->withIsNullable(true),
             'string',
-            "null | array<int<min,max>, 'a' | 'b'>",
+            "null | array<int<0,max>, 'a' | 'b'>",
             '?array',
         ];
         yield 'nullable-enum' => [
@@ -130,7 +130,7 @@ public function provideExpectations()
                     ['type' => 'int', 'isList' => true, 'namespace' => 'xx'],
                 ]),
             'unionType',
-            "array<int<min,max>, string | list<int>>",
+            "array<int<0,max>, string | list<int>>",
             'array',
         ];
         yield 'nullable-array-of-union-with-list' => [
@@ -143,7 +143,7 @@ public function provideExpectations()
                     ['type' => 'int', 'isList' => true, 'namespace' => 'xx'],
                 ]),
             'unionType',
-            "null | array<int<min,max>, string | list<int>>",
+            "null | array<int<0,max>, string | list<int>>",
             '?array',
         ];
     }