Skip to content

Report array&callable pass as wrong param#5573

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

Report array&callable pass as wrong param#5573
phpstan-bot wants to merge 6 commits intophpstan:2.1.xfrom
phpstan-bot:create-pull-request/patch-k8eciuw

Conversation

@phpstan-bot
Copy link
Copy Markdown
Collaborator

Summary

When a parameter, return type, or property was declared with an array&callable intersection type (including the callable-array shorthand), MissingTypehintCheck did not report the missing iterable value type for the array part. This meant users were not warned about the imprecise array<mixed> in these intersections, and downstream argument type checks couldn't catch mismatches (e.g., passing array&callable to array<int>).

Changes

  • Modified src/Rules/MissingTypehintCheck.php: In the special-case block for array&callable intersections (lines 76–96), added a check on each array inner type's getIterableValueType(). If the value type is implicit mixed, it is now added to the missing-typehint list. The non-array parts (callable) are still traversed for their own inner type checks (e.g., callable parameter/return arrays).
  • Added test case doBaz() with @param callable&array in tests/PHPStan/Rules/Methods/data/bug-14549.php
  • Updated test expectations in tests/PHPStan/Rules/Methods/MissingMethodParameterTypehintRuleTest.php:
    • doFoo() (callable-array) now reports missingType.iterableValue for its array part
    • doIntersection() (array&callable(array): array) now reports a third missingType.iterableValue for the outer array
    • doBaz() (callable&array) reports both missingType.iterableValue and missingType.callable

Root cause

MissingTypehintCheck::getIterableTypesWithMissingValueTypehint() had a special-case block (lines 76–92) for intersection types that are both callable and array. This block extracted the non-array parts and traversed only those, completely skipping the array parts. The array parts were never checked for missing value types, so callable-array / callable&array silently passed the check even though their array part has implicit mixed values.

The fix adds the missing value type check for each array inner type within the existing special-case block, before the continue that skips it.

Analogous cases probed

Since the fix is in the shared MissingTypehintCheck class, all consumers are automatically fixed:

  • Method parameters (MissingMethodParameterTypehintRule) — tested and verified
  • Function parameters (MissingFunctionParameterTypehintRule) — shares the same check, tests pass
  • Method return types (MissingMethodReturnTypehintRule) — shares the same check, tests pass
  • Function return types (MissingFunctionReturnTypehintRule) — shares the same check, tests pass
  • Properties (MissingPropertyTypehintRule) — shares the same check, tests pass
  • Class constants (MissingClassConstantTypehintRule) — shares the same check, tests pass
  • @var tags, @phpstan-assert, self-out types — all use the same getIterableTypesWithMissingValueTypehint() method

Non-array callable intersections (e.g., Traversable&callable) don't hit this code path (the condition requires isArray()->yes()), and they are already handled correctly by the general iterable check below.

Test

  • testBug14549 in MissingMethodParameterTypehintRuleTest verifies:
    • callable-array reports missing iterable value type
    • array&callable(array): array reports missing value type for all three arrays (outer + callable param + callable return)
    • callable&array reports both missing iterable value type and missing callable signature
  • NSRT test for bug-14549 verifies type inference for callable array offsets remains correct

Fixes phpstan/phpstan#14549

… intersections in `MissingTypehintCheck`

- In `MissingTypehintCheck::getIterableTypesWithMissingValueTypehint()`, the
  special-case block for `array&callable` intersection types was skipping the
  array part entirely, never checking it for missing value types
- Added a check for the array inner type's iterable value type before skipping
  it, so `callable-array`, `callable&array`, and `array&callable(...)` now
  correctly report `missingType.iterableValue` when the array has no value type
- Updated test expectations for `doIntersection()` (gains a third error for the
  outer array) and `doFoo()` (now reports missing iterable value type)
- Added new test case `doBaz()` with explicit `callable&array` parameter
- All consumers of `MissingTypehintCheck` (method/function parameters, return
  types, properties, constants, @var tags, @phpstan-assert) are automatically
  fixed since they share the same check
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

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

No, this behavior is on purpose.

We don't want to report missingTypehint check for array&callable since we can infer it.

But we should still report passing array&callable when passed to a function expecting array.

For instance we already report passing array&callable to array{string, string} (because it could be array{object|string, string}).

phpstan-bot and others added 2 commits April 30, 2026 09:20
…ngTypehintCheck

The missing typehint suppression for array&callable is intentional because
PHPStan can infer the value type (object|non-falsy-string). Reverts the
MissingTypehintCheck changes and their test expectations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rray&callable

When an array&callable intersection is checked for acceptance, the raw
ArrayType(mixed, mixed) component was used, causing array<int> to
incorrectly accept array&callable. Now the array component uses the
narrowed key/value types (int<0,1>, object|non-falsy-string) so the
acceptance check correctly rejects incompatible array types.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@VincentLanglet VincentLanglet changed the title Report missing iterable value type for array part of array&callable intersections in MissingTypehintCheck Report array&callable pass as wrong param Apr 30, 2026
Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

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

Use another file bug-14549-bis.php for your tests

And add test with callable&array passed to a method expecting constant array

  • array{string, string} => Error
  • array{object|string, string} => No error
  • array{object|string, string, string} => Error

…d constant array test cases

Test callable&array passed to methods expecting constant array types:
- array{string, string} => Error (object at index 0 is not string)
- array{object|string, string} => No error (matches callable-array shape)
- array{object|string, string, string} => Error (callable-array has 2 elements, not 3)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Done. Here's what I changed:

  1. Created tests/PHPStan/Rules/Methods/data/bug-14549-bis.php — a separate test file for CallMethodsRuleTest with methods accepting array<int>, array{string, string}, array{object|string, string}, and array{object|string, string, string}, called from both callable-array and callable&array typed parameters.

  2. Removed call-related methods from bug-14549.php — the call(), doCallWithCallableArray(), and doCallWithCallableAndArray() methods that were only used by CallMethodsRuleTest are now in the separate file.

  3. Updated CallMethodsRuleTest — now uses bug-14549-bis.php and verifies the constant array test cases:

    • array{string, string}Error (callable-array has object at index 0)
    • array{object|string, string}No error (matches callable-array shape)
    • array{object|string, string, string}Error (callable-array has 2 elements, not 3)
  4. Updated MissingMethodParameterTypehintRuleTest — removed expectations for methods that moved to the new file.

Copy link
Copy Markdown
Contributor

@VincentLanglet VincentLanglet left a comment

Choose a reason for hiding this comment

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

I can reproduce a similar issue with array<mixed>&hasOffsetValue(1,1) which seems acceptedBy array<string>.

Maybe IntersectionType::isAccepted implementation is wrong.
I wonder if lazyMaxMin is the right check

When looking for A&B isAcceptedBy C
If C does not accept A, A&B cannot be accepted.

What about a strategy

  • If a NO => Return No
  • Else returns the Max (Maybe & Yes)

What would be the impact of such change ?

@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

I processed this review but have nothing to report.

…d of special-casing callable&array

Replace the callable&array-specific narrowing logic with a general
approach: after the standard lazyMaxMin check, if it returns Yes,
verify with isSuperTypeOf that the accepting type is actually a
supertype of the full intersection. This catches cases where
MixedType's accepts-everything behavior causes false acceptances
(e.g. array<int> falsely accepting array<mixed>&hasOffsetValue
or array<mixed>&callable intersections).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@phpstan-bot
Copy link
Copy Markdown
Collaborator Author

Done. Here's a summary of what I changed:

The problem: The reviewer pointed out that the PR's approach of special-casing callable&array in IntersectionType::isAcceptedBy() was too narrow. The same issue exists for other intersection types like array<mixed>&hasOffsetValue(1,1) — they were incorrectly accepted by array<string> because MixedType::isAcceptedBy() always returns Yes, and lazyMaxMin short-circuits on that.

Root cause: MixedType implements CompoundType and its isAcceptedBy() always returns Yes (since mixed is a supertype of everything). When lazyMaxMin iterates intersection components individually, it short-circuits on the first Yes from array<mixed>, ignoring that other components (like hasOffsetValue(1,1) or callable) constrain the intersection in ways that make it incompatible.

The fix: Replace the callable&array-specific narrowing with a general guard: after lazyMaxMin produces a Yes result, verify it with $acceptingType->isSuperTypeOf($this). The isSuperTypeOf chain (which goes through IntersectionType::isSubTypeOf) correctly considers all components together and returns No when the intersection as a whole is not a subtype. If isSuperTypeOf says No, we override the false Yes.

This approach:

  • Fixes the original callable&array vs array<int> case
  • Also fixes the reviewer's array<mixed>&hasOffsetValue(1,1) vs array<string> case
  • Preserves $strictTypes coercion behavior (since accepts is still the primary check)
  • Preserves error reasons from accepts (the isSuperTypeOf guard only overrides on definitive No)
  • All 11,971 tests pass and make phpstan reports no errors

Comment on lines +50 to +55
/**
* @param callable&array $task
*/
public function doBaz(array $task): void
{
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove this

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done. Removed the doBaz() method from bug-14549.php and its corresponding test expectation in MissingMethodParameterTypehintRuleTest.php. All 11,971 tests pass and make phpstan reports no errors.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@VincentLanglet
Copy link
Copy Markdown
Contributor

This seems to work but I wonder if I and the bot are not making a mistake by confusing accepts and isSuperTypeOf...

And there is multiple integration test failure I'll need to check

I'll need to take more time on it, just wanted to know if it's inspire you @staabm ?
Maybe you see a better direction for this PR.

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.

2 participants