diff --git a/CLAUDE.md b/CLAUDE.md index 10279d2f0d..e33c66a1c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -348,6 +348,12 @@ Bugs occur when impure points are missed (e.g. inherited constructors of anonymo `FunctionCallParametersCheck` (`src/Rules/FunctionCallParametersCheck.php`) validates arguments passed to functions/methods. For by-reference parameters, it checks whether the argument is a valid lvalue (variable, array dim fetch, property fetch). It also allows function/method calls that return by reference (`&getString()`, `&staticGetString()`, `&refFunction()`), using `returnsByReference()` on the resolved reflection. The class is manually instantiated in ~20 test files, so adding a constructor parameter requires updating all of them. The `Scope` interface provides `getMethodReflection()` for method calls, while `ReflectionProvider` (injected into the class) is needed for resolving function reflections. +### FunctionCallParametersCheck: spread argument expansion with optional named keys + +When spreading a constant array into a function/method call (`foo(...$array)`), `FunctionCallParametersCheck::check()` (lines 139-213) expands each array position into an individual argument. For each position, it checks whether the key is optional (`getOptionalKeys()`), extracts the value type, and determines the key name. Optional keys (array positions that might not exist) are normally skipped to avoid asserting they're always present. + +However, when the optional key has a string name (named argument), skipping it causes a fallback path (lines 195-203) that loses the key-to-type correspondence. The fallback uses `getIterableValueType()` which unions ALL value types, then passes this as a single generic unpacked argument. This causes false positives when different keys have different value types — e.g., `non-empty-array{width?: int, bgColor?: string}` spread into `Foo(int|null $width, string|null $bgColor)` reports "int|string given" for `$width` because the fallback unions `int` and `string`. The fix: only skip optional keys when they don't have a named key (`$keyArgumentName === null`), so named optional keys are still expanded as individual named arguments with their correct per-key types. + ### Testing patterns - **Rule tests**: Extend `RuleTestCase`, implement `getRule()`, call `$this->analyse([__DIR__ . '/data/my-test.php'], [...expected errors...])`. Expected errors are `[message, line]` pairs. Test data files live in `tests/PHPStan/Rules/*/data/`. diff --git a/src/Rules/FunctionCallParametersCheck.php b/src/Rules/FunctionCallParametersCheck.php index 61c80b7199..74ceec9c00 100644 --- a/src/Rules/FunctionCallParametersCheck.php +++ b/src/Rules/FunctionCallParametersCheck.php @@ -179,7 +179,7 @@ public function check( $keyArgumentName = $commonKey; $hasNamedArguments = true; } - if ($isOptionalKey) { + if ($isOptionalKey && $keyArgumentName === null) { continue; } diff --git a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php index 89557169d0..3232e63282 100644 --- a/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php +++ b/tests/PHPStan/Rules/Classes/InstantiationRuleTest.php @@ -564,6 +564,12 @@ public function testNamedArgumentsPhpversion(): void $this->analyse([__DIR__ . '/data/named-arguments-phpversion.php'], []); } + #[RequiresPhp('>= 8.0')] + public function testBug14097(): void + { + $this->analyse([__DIR__ . '/data/bug-14097.php'], []); + } + public function testNewStaticWithConsistentConstructor(): void { $this->analyse([__DIR__ . '/data/instantiation-new-static-consistent-constructor.php'], [ diff --git a/tests/PHPStan/Rules/Classes/data/bug-14097.php b/tests/PHPStan/Rules/Classes/data/bug-14097.php new file mode 100644 index 0000000000..bd92988c0c --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/bug-14097.php @@ -0,0 +1,37 @@ +\d+)/', $path, $matches)) { + $manipulations['width'] = (int)$matches['w']; + $path = str_replace($matches[0], '', $path); + } + + // Background color (when padding images) + if (preg_match('/_rgb(?(?:[0-9a-fA-F]{3}){1,2})/', $path, $matches)) { + $manipulations['bgColor'] = $matches['rgb']; + $path = str_replace($matches[0], '', $path); + } + + if (!empty($manipulations)) { + return new Manipulations(...$manipulations); + } + + return new Manipulations(...[]); + } +}