Skip to content

Fix phpstan/phpstan#11619: Parameter #2 $callback of function uasort expects callable(Foo, Foo): int, 'strnatcasecmp' given.#5171

Open
phpstan-bot wants to merge 7 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-mzyn37q
Open

Fix phpstan/phpstan#11619: Parameter #2 $callback of function uasort expects callable(Foo, Foo): int, 'strnatcasecmp' given.#5171
phpstan-bot wants to merge 7 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-mzyn37q

Conversation

@phpstan-bot
Copy link
Collaborator

Summary

PHPStan incorrectly reported an error when passing a string-comparison function like strnatcasecmp to uasort() for arrays of Stringable objects. For example, uasort($options, 'strnatcasecmp') where $options is Foo[] and Foo implements Stringable would 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 strnatcasecmp perform implicit type coercion from Stringable to string.

Changes

  • src/Type/CallableType.php: Propagated the $strictTypes parameter from accepts() through isSuperTypeOfInternal() to CallableTypeHelper::isParametersAcceptorSuperTypeOf()
  • src/Type/CallableTypeHelper.php: Added optional $strictTypes parameter (default true) and used it in the callable parameter acceptance check instead of the hardcoded true
  • tests/PHPStan/Rules/Functions/data/bug-11619.php: New regression test with Stringable class and uasort/usort with strnatcasecmp
  • tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php: Added testBug11619 test method

Root cause

CallableType::accepts() received a $strictTypes parameter but ignored it, always passing treatMixedAsAny=true to its internal method without forwarding $strictTypes. The CallableTypeHelper::isParametersAcceptorSuperTypeOf() hardcoded strictTypes=true when calling $theirParameter->getType()->accepts(), which caused StringType::accepts() to reject Stringable objects without checking for __toString().

The fix propagates the scope's $strictTypes value through the chain. In non-strict mode (no declare(strict_types=1)), StringType::accepts() now checks for __toString() and accepts Stringable objects. In strict mode, the existing behavior is preserved, which correctly handles cases like bug-12317 where a user-defined closure with explicit string parameter types should still be flagged.

Test

Added tests/PHPStan/Rules/Functions/data/bug-11619.php with a Stringable class Foo and calls to uasort($options, 'strnatcasecmp') and usort($options, 'strnatcasecmp'). The test expects no errors.

Fixes phpstan/phpstan#11619

@VincentLanglet VincentLanglet force-pushed the create-pull-request/patch-mzyn37q branch from befcc3e to b5e3d6e Compare March 26, 2026 07:17
@VincentLanglet VincentLanglet self-assigned this Mar 26, 2026
staabm
staabm previously approved these changes Mar 26, 2026
Comment on lines +2753 to +2762
$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,
],
]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per https://3v4l.org/8mvmh we should not have errors even with strict-types, when a builtin-function is used

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both background tasks (PHPStan and tests) completed successfully. All changes have already been committed and pushed.

@staabm staabm dismissed their stale review March 26, 2026 12:58

needs more work

return $this->function->getAsserts();
}

public function isBuiltin(): bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this method should return TrinaryLogic instead of bool

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

ondrejmirtes and others added 4 commits March 26, 2026 15:13
…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>
@staabm staabm force-pushed the create-pull-request/patch-mzyn37q branch from b7187f1 to e1817a5 Compare March 26, 2026 14:13
return $this->function->getAsserts();
}

public function isBuiltin(): TrinaryLogic
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implement this method on CallableParametersAcceptor instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. The changes:

  • Added isBuiltin(): TrinaryLogic to the CallableParametersAcceptor interface
  • Implemented it in all 7 implementors: ClosureType, CallableType, TrivialParametersAcceptor, InaccessibleMethod (all return TrinaryLogic::createNo()), ExtendedCallableFunctionVariant, ResolvedFunctionVariantWithCallable (propagate via constructor parameter), and FunctionCallableVariant (existing implementation)
  • Updated creation sites in GenericParametersAcceptorResolver and ParametersAcceptorSelector to propagate isBuiltin through
  • Simplified CallableType::isSuperTypeOfInternal() to use $variant->isBuiltin()->yes() instead of $variant instanceof FunctionCallableVariant && $variant->isBuiltin()->yes()

phpstan-bot and others added 3 commits March 26, 2026 14:26
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>
@staabm
Copy link
Contributor

staabm commented Mar 26, 2026

I have added more tests, but I am getting to a point in which I don't know when/why php-src accepts a Stringable object and when it only accepts a string but not Stringable.

it clearly depends on strict-types, but there is some other factor playing in.
I was not able to find in the php-src documentation how this is supposed to work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants