Skip to content

tests: test parameters type detection #7240

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.

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

Open
wants to merge 3 commits into
base: 4.1
Choose a base branch
from

Conversation

vincentchalamon
Copy link
Contributor

No description provided.

@vincentchalamon vincentchalamon requested a review from soyuka June 23, 2025 07:47
@vincentchalamon vincentchalamon self-assigned this Jun 23, 2025
@vincentchalamon vincentchalamon force-pushed the fix/parameter-type-detection branch 4 times, most recently from c524727 to 3f4932a Compare June 23, 2025 09:05
@vincentchalamon vincentchalamon force-pushed the fix/parameter-type-detection branch from 3f4932a to 4f58677 Compare June 23, 2025 09:10
@vincentchalamon vincentchalamon marked this pull request as ready for review June 23, 2025 09:14

// Allow null in case of optional parameter
if (isset($schema['type']) && 'boolean' === $schema['type']) {
$assertions[] = new Expression(expression: 'value in [null, 0, 1, "0", "1", false, true, "false", "true"]');
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a huge fan of using an expression, I'd prefer to use Assert\IsTrue or Assert\IsFalse or why not even Assert\Type. This leads to the same issue as above that is that parameters are always strings. Suggestion: let's add a new castToNativeType option on parameters, if its true we can cast the string to the native type inside the ApiPlatform\State\Provider\ParameterProvider (there's a function in type info for that IIRC). Then, if castToNativeType is true we can safely add an Assert\Type constraint.

Copy link
Contributor Author

@vincentchalamon vincentchalamon Jun 26, 2025

Choose a reason for hiding this comment

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

@soyuka does this commit suit your proposition?

@vincentchalamon vincentchalamon force-pushed the fix/parameter-type-detection branch from b34e067 to f7a4639 Compare June 26, 2025 15:40
@vincentchalamon vincentchalamon force-pushed the fix/parameter-type-detection branch from f7a4639 to f1c3bbb Compare June 26, 2025 15:42
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