Skip to content

[TypeInfo] Fix isSatisfiedBy not traversing type tree #59844

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Mar 24, 2025
Merged

[TypeInfo] Fix isSatisfiedBy not traversing type tree #59844

merged 1 commit into from
Mar 24, 2025

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Feb 24, 2025

Q A
Branch? 7.2
Bug fix? yes
New feature? no
Deprecations? no
Issues
License MIT

Previously, Type::isSatisfiedBy was not traversing the type tree, which means that:

$specification = static fn (Type $type): bool => $type instanceof ObjectType;

return Type::collection(Type::object(Foo::class))->isSatisfiedBy($specification);

was unexpectedly returning false.
This PR fixes it.

@carsonbot carsonbot added this to the 7.2 milestone Feb 24, 2025
@mtarld mtarld force-pushed the fix/is-satisfied-by branch 2 times, most recently from e46905c to cad3275 Compare February 24, 2025 09:40
@mtarld mtarld mentioned this pull request Feb 24, 2025
Merged
@stof
Copy link
Member

stof commented Feb 24, 2025

a collection of object is not satisfying the object type. An array is not an object. This PR is the one introducing an unexpected behavior while the current code works fine.

-1 for this PR.

@mtarld
Copy link
Contributor Author

mtarld commented Feb 24, 2025

Sorry, the example was not clear enough, Type::collection(Type::object(Foo::class)) means that Foo is the collection. I should have written the following:

Type::collection(type: Type::object(\Iterator::class), value: Type::int())->isSatisfiedBy($specification);

Which will check the whole type, and the Iterator type, but not the int type.

@mtarld mtarld force-pushed the fix/is-satisfied-by branch from cad3275 to 8df764a Compare March 3, 2025 11:20
@mtarld mtarld requested a review from stof March 6, 2025 07:23
@fabpot
Copy link
Member

fabpot commented Mar 24, 2025

Thank you @mtarld.

@fabpot fabpot merged commit bc82b67 into symfony:7.2 Mar 24, 2025
10 of 11 checks passed
@mtarld mtarld deleted the fix/is-satisfied-by branch March 24, 2025 09:19
@fabpot fabpot mentioned this pull request Mar 28, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@chalasr chalasr chalasr approved these changes

@stof stof Awaiting requested review from stof

Assignees
No one assigned
Labels
Bug Status: Reviewed TypeInfo
Projects
None yet
Milestone
7.2
Development

Successfully merging this pull request may close these issues.
6 participants
@mtarld @stof @fabpot @nicolas-grekas @chalasr @carsonbot