Enforcing string types for argument names call() scalar method

documentation/upgrading.md

@@ -5,6 +5,29 @@ Please follow the instructions for your specific version to ensure a smooth upgr
 
 ---
 
+## Upgrading from 0.27.x to 0.28.x
+
+### 1) Enforcing string types for argument names `call()` scalar method
+
+Before:
+```php
+ref('integers')->call(lit('explode'), ['separator' => ','], refAlias: 'string', returnType: type_list(type_integer()))
+ref('integers')->call(lit('explode'), ['separator' => ','])
+ref('integers')->call(lit('explode'), [','])
+ref('integers')->call(lit('count'))
+```
+
+After:
+
+```php
+ref('integers')->call(lit('explode'), arguments: ['separator' => ','], refAlias: 'string', returnType: type_list(type_integer()))
+ref('integers')->call(lit('explode'), arguments: ['separator' => ','], refAlias: 'string')
+ref('integers')->call(lit('explode'), ['separator' => ',']) // will throw invalid argument exception
+ref('integers')->call(lit('explode'), [',']) // will throw invalid argument exception
+ref('integers')->call(lit('explode')) // will return null or error in strict mode
+ref('integers')->call(lit('count')) // will work same as before
+```
+
 ## Upgrading from 0.26.x to 0.27.x
 
 ### 1) Force `EntryFactory $entryFactory` to be required on `array_to_row` & `array_to_row(s)`

examples/topics/transformations/call/code.php

@@ -23,7 +23,7 @@
     )
     ->withEntry(
         'integers',
-        ref('integers')->call(lit('explode'), ['separator' => ','], refAlias: 'string', returnType: type_list(type_integer()))
+        ref('integers')->call(lit('explode'), arguments: ['separator' => ','], refAlias: 'string', returnType: type_list(type_integer()))
     )
     ->write(to_stream(__DIR__ . '/output.txt', truncate: false))
     ->run();

src/core/etl/src/Flow/ETL/DSL/functions.php

@@ -1373,13 +1373,16 @@ function count(?EntryReference $function = null) : Count
 /**
  * Calls a user-defined function with the given parameters.
  *
- * @param callable|ScalarFunction $callable
- * @param array<mixed> $parameters
+ * @param array<string, mixed> $parameters
  * @param null|Type<mixed> $return_type
  */
 #[DocumentationDSL(module: Module::CORE, type: DSLType::SCALAR_FUNCTION)]
 function call(ScalarFunction|callable $callable, array $parameters = [], ?Type $return_type = null) : CallUserFunc
 {
+    if ([] !== $parameters && \array_is_list($parameters)) {
+        throw new InvalidArgumentException('call arguments cannot be a list');
+    }
+
     return new CallUserFunc($callable, $parameters, $return_type);
 }
 

src/core/etl/src/Flow/ETL/Function/CallUserFunc.php

@@ -17,8 +17,7 @@ final class CallUserFunc extends ScalarFunctionChain
     private $callable;
 
     /**
-     * @param callable|ScalarFunction $callable
-     * @param array<mixed> $parameters
+     * @param array<array-key, mixed> $parameters
      * @param null|Type<mixed> $returnType
      */
     public function __construct(ScalarFunction|callable $callable, private readonly array $parameters, private readonly ?Type $returnType = null)
@@ -40,13 +39,17 @@ public function eval(Row $row, FlowContext $context) : mixed
             $parameters[$key] = (new Parameter($parameter))->eval($row, $context);
         }
 
-        if ($this->returnType) {
-            return new ScalarResult(
-                \call_user_func($callable, ...$parameters),
-                $this->returnType
-            );
+        try {
+            if ($this->returnType) {
+                return new ScalarResult(
+                    \call_user_func($callable, ...$parameters),
+                    $this->returnType
+                );
+            }
+
+            return \call_user_func($callable, ...$parameters);
+        } catch (\ArgumentCountError $e) {
+            return $context->functions()->invalidResult(new InvalidArgumentException($e->getMessage(), $e->getCode(), $e));
         }
-
-        return \call_user_func($callable, ...$parameters);
     }
 }

src/core/etl/src/Flow/ETL/Function/ScalarFunctionChain.php

@@ -152,12 +152,26 @@ public function binaryLength() : BinaryLength
     }
 
     /**
-     * @param array<array-key, mixed> $arguments
-     * @param Type<mixed> $returnType
+     * @param array<string, mixed> $arguments
+     * @param null|Type<mixed> $returnType
      */
-    public function call(ScalarFunction|callable $callable, array $arguments = [], string|int $refAlias = 0, ?Type $returnType = null) : CallUserFunc
+    public function call(ScalarFunction|callable $callable, array $arguments = [], ?string $refAlias = null, ?Type $returnType = null) : CallUserFunc
     {
-        return new CallUserFunc($callable, array_merge($arguments, [$refAlias => $this]), $returnType);
+        if ([] !== $arguments && \array_is_list($arguments)) {
+            throw new InvalidArgumentException('call arguments cannot be a list');
+        }
+
+        if ($refAlias === null && [] !== $arguments) {
+            throw new InvalidArgumentException('refAlias cannot be null when named arguments are passed');
+        }
+
+        if ($refAlias === null && [] === $arguments) {
+            $arguments = [$this];
+        } else {
+            $arguments[$refAlias] = $this;
+        }
+
+        return new CallUserFunc($callable, $arguments, $returnType);
     }
 
     public function capitalize() : Capitalize

src/core/etl/tests/Flow/ETL/Tests/Integration/Function/CallUserFuncTest.php

@@ -23,7 +23,7 @@ public function test_call() : void
             )
             ->withEntry(
                 'integers',
-                ref('integers')->call(lit('explode'), ['separator' => ','], refAlias: 'string', returnType: type_list(type_integer()))
+                ref('integers')->call(lit('explode'), refAlias: 'string', arguments: ['separator' => ','], returnType: type_list(type_integer()))
             )
             ->write(to_memory($memory = new ArrayMemory()))
             ->run();

src/core/etl/tests/Flow/ETL/Tests/Unit/Function/CallUserFuncTest.php

@@ -6,11 +6,12 @@
 
 use function Flow\ETL\DSL\{call, flow_context, list_entry, lit, ref, row, string_entry};
 use function Flow\Types\DSL\{type_integer, type_list};
+use Flow\ETL\Exception\InvalidArgumentException;
 use Flow\ETL\Function\ScalarFunction\ScalarResult;
-use Flow\ETL\Tests\FlowTestCase;
 use Flow\ETL\Tests\Unit\Function\Fixtures\CallUserFunc\StaticCalculator;
+use PHPUnit\Framework\TestCase;
 
-final class CallUserFuncTest extends FlowTestCase
+final class CallUserFuncTest extends TestCase
 {
     public function test_call_user_func_as_dsl() : void
     {
@@ -31,6 +32,41 @@ public function test_call_user_func_with_native_function() : void
         );
     }
 
+    public function test_call_user_func_with_native_function_and_no_arguments() : void
+    {
+        $row = row(
+            list_entry('list', [1, 2, 3], type_list(type_integer())),
+        );
+
+        self::assertNull(
+            ref('list')
+                ->call(lit('time'))
+                ->eval($row, flow_context())
+        );
+    }
+
+    public function test_call_user_func_with_non_callable_function() : void
+    {
+        $row = row(
+            list_entry('list', [1, 2, 3], type_list(type_integer())),
+        );
+
+        self::assertNull(
+            ref('list')
+                ->call(lit('unknown'), refAlias: 'whatever')
+                ->eval($row, flow_context())
+        );
+    }
+
+    public function test_call_user_func_with_non_string_argument_keys() : void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('call arguments cannot be a list');
+
+        ref('list')
+            ->call(lit('explode'), arguments: [',']); // @phpstan-ignore argument.type
+    }
+
     public function test_call_user_func_with_object_method() : void
     {
         $row = row(
@@ -56,7 +92,7 @@ public function test_call_user_func_with_ref_alias_and_optional_arguments() : vo
         self::assertSame(
             ['1', '2', '3'],
             ref('item_ids')
-                ->call(lit('explode'), ['separator' => ','], refAlias: 'string')
+                ->call(lit('explode'), arguments: ['separator' => ','], refAlias: 'string')
                 ->eval($row, flow_context())
         );
     }
@@ -70,7 +106,7 @@ public function test_call_user_func_with_ref_alias_and_optional_arguments_and_re
         self::assertEquals(
             new ScalarResult([1, 2, 3], type_list(type_integer())),
             ref('item_ids')
-                ->call(lit('explode'), ['separator' => ','], refAlias: 'string', returnType: type_list(type_integer()))
+                ->call(lit('explode'), refAlias: 'string', arguments: ['separator' => ','], returnType: type_list(type_integer()))
                 ->eval($row, flow_context())
         );
     }
@@ -88,4 +124,13 @@ public function test_call_user_func_with_static_method() : void
                 ->eval($row, flow_context())
         );
     }
+
+    public function test_call_user_func_with_without_ref_alias_and_arguments() : void
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage('refAlias cannot be null when named arguments are passed');
+
+        ref('item_ids')
+            ->call(lit('explode'), arguments: ['separator' => ',']);
+    }
 }