Fix phpstan/phpstan#11619: Parameter #2 $callback of function uasort expects callable(Foo, Foo): int, 'strnatcasecmp' given.#5171
Conversation
befcc3e to
b5e3d6e
Compare
| $this->analyse([__DIR__ . '/data/bug-11619-strict.php'], [ | ||
| [ | ||
| 'Parameter #2 $callback of function uasort expects callable(Bug11619Strict\Foo, Bug11619Strict\Foo): int, \'strnatcasecmp\' given.', | ||
| 28, | ||
| ], | ||
| [ | ||
| 'Parameter #2 $callback of function usort expects callable(Bug11619Strict\Foo, Bug11619Strict\Foo): int, \'strnatcasecmp\' given.', | ||
| 29, | ||
| ], | ||
| ]); |
There was a problem hiding this comment.
per https://3v4l.org/8mvmh we should not have errors even with strict-types, when a builtin-function is used
There was a problem hiding this comment.
Both background tasks (PHPStan and tests) completed successfully. All changes have already been committed and pushed.
| return $this->function->getAsserts(); | ||
| } | ||
|
|
||
| public function isBuiltin(): bool |
There was a problem hiding this comment.
this method should return TrinaryLogic instead of bool
There was a problem hiding this comment.
Done. Changed FunctionCallableVariant::isBuiltin() to return TrinaryLogic instead of bool, as requested by staabm. The method now returns the TrinaryLogic directly when the underlying function provides one, or wraps the bool via TrinaryLogic::createFromBoolean(). The call site in CallableType.php was updated to use ->yes() on the result. Both PHPStan and tests pass.
…mode - Propagated $strictTypes from CallableType::accepts() through to CallableTypeHelper - In non-strict mode, Stringable objects are now accepted where string is expected in callable parameters - This fixes false positives for patterns like uasort($stringableArray, 'strnatcasecmp') - In strict_types mode, the stricter behavior is preserved (e.g. closures with explicit string params) - New regression test in tests/PHPStan/Rules/Functions/data/bug-11619.php Closes phpstan/phpstan#11619
Adds a second test file with declare(strict_types=1) to verify that in strict mode, passing strnatcasecmp to uasort/usort with Stringable objects is correctly reported as an error. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Built-in PHP functions always perform implicit type coercion regardless of declare(strict_types=1), so Stringable objects should be accepted as string parameters even in strict mode. Instead of propagating the calling scope's $strictTypes, detect whether the callable is a built-in function via FunctionCallableVariant::isBuiltin() and use non-strict checking for its parameters. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
b7187f1 to
e1817a5
Compare
| return $this->function->getAsserts(); | ||
| } | ||
|
|
||
| public function isBuiltin(): TrinaryLogic |
There was a problem hiding this comment.
implement this method on CallableParametersAcceptor instead
There was a problem hiding this comment.
Done. The changes:
- Added
isBuiltin(): TrinaryLogicto theCallableParametersAcceptorinterface - Implemented it in all 7 implementors:
ClosureType,CallableType,TrivialParametersAcceptor,InaccessibleMethod(all returnTrinaryLogic::createNo()),ExtendedCallableFunctionVariant,ResolvedFunctionVariantWithCallable(propagate via constructor parameter), andFunctionCallableVariant(existing implementation) - Updated creation sites in
GenericParametersAcceptorResolverandParametersAcceptorSelectorto propagateisBuiltinthrough - Simplified
CallableType::isSuperTypeOfInternal()to use$variant->isBuiltin()->yes()instead of$variant instanceof FunctionCallableVariant && $variant->isBuiltin()->yes()
Instead of checking `instanceof FunctionCallableVariant` in CallableType, the isBuiltin() method is now part of the CallableParametersAcceptor interface and implemented by all classes. The value is propagated through ExtendedCallableFunctionVariant and ResolvedFunctionVariantWithCallable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
I have added more tests, but I am getting to a point in which I don't know when/why php-src accepts a it clearly depends on strict-types, but there is some other factor playing in. |
Summary
PHPStan incorrectly reported an error when passing a string-comparison function like
strnatcasecmptouasort()for arrays ofStringableobjects. For example,uasort($options, 'strnatcasecmp')where$optionsisFoo[]andFooimplementsStringablewould produce: "Parameter #2 $callback of function uasort expects callable(Foo, Foo): int, 'strnatcasecmp' given."This is a false positive because PHP's internal functions like
strnatcasecmpperform implicit type coercion fromStringabletostring.Changes
src/Type/CallableType.php: Propagated the$strictTypesparameter fromaccepts()throughisSuperTypeOfInternal()toCallableTypeHelper::isParametersAcceptorSuperTypeOf()src/Type/CallableTypeHelper.php: Added optional$strictTypesparameter (defaulttrue) and used it in the callable parameter acceptance check instead of the hardcodedtruetests/PHPStan/Rules/Functions/data/bug-11619.php: New regression test withStringableclass anduasort/usortwithstrnatcasecmptests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php: AddedtestBug11619test methodRoot cause
CallableType::accepts()received a$strictTypesparameter but ignored it, always passingtreatMixedAsAny=trueto its internal method without forwarding$strictTypes. TheCallableTypeHelper::isParametersAcceptorSuperTypeOf()hardcodedstrictTypes=truewhen calling$theirParameter->getType()->accepts(), which causedStringType::accepts()to rejectStringableobjects without checking for__toString().The fix propagates the scope's
$strictTypesvalue through the chain. In non-strict mode (nodeclare(strict_types=1)),StringType::accepts()now checks for__toString()and acceptsStringableobjects. In strict mode, the existing behavior is preserved, which correctly handles cases like bug-12317 where a user-defined closure with explicitstringparameter types should still be flagged.Test
Added
tests/PHPStan/Rules/Functions/data/bug-11619.phpwith aStringableclassFooand calls touasort($options, 'strnatcasecmp')andusort($options, 'strnatcasecmp'). The test expects no errors.Fixes phpstan/phpstan#11619