Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Validator] AddedStrictTypes as class constraint with not nullable typed properties#36494

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

@HeahDude
Copy link
Contributor

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
Tickets~
LicenseMIT
Doc PRTODO

After I commented#36352 (comment), I started to work on a proper constraint instead. Following#35532,#36492 and#36352, this PR enables the following short example:

  • Before:
    /** * @Assert\Cascade */class User{/**     * @Assert\NotNull     * @Assert\Regex("some pattern")     */publicstring$username;/**     * @Assert\NotNull     * @AssertEmail     */publicstring$email;/**     * @AssertEmail     */public ?string$secondaryEmail;/**     * @Assert\NotNull     */publicAddress$address;public ?Address$secondaryAddress;}
  • After:
    /** * @Assert\Cascade * @Assert\StrictTypes */class User{/**     * @Assert\Regex("some pattern")     */publicstring$username;/**     * @AssertEmail     */publicstring$email;/**     * @AssertEmail     */public ?string$secondaryEmail;publicAddress$address;public ?Address$secondaryAddress;}

Until#36352 is merged or close, only the second commit should be reviewed here.


Side note: unlikeAutoMapping, this feature require PHP 7.4, but do not rely on complexity or other components to achieve a simple non nullable check.
This remains explicit and works for simple VO which do not require PHP annotations anymore, nor ORM mapping.

ro0NL reacted with confused emoji
@HeahDudeHeahDudeforce-pushed thevalidator/strict_types branch from333ca17 to9cf7cbdCompareApril 19, 2020 14:31
@nicolas-grekasnicolas-grekas added this to thenext milestoneApr 19, 2020
@ro0NL
Copy link
Contributor

how canpublic string $username; ever be null in the first place?

@HeahDude
Copy link
ContributorAuthor

@ro0NL when hydrating objects from external source/input, e.g a submitted form or a deserialized payload to be validated.

@ro0NL
Copy link
Contributor

AFAIK you'll hitUncaught TypeError: Typed property Class::$prop must be string, null used in general. The only reason this works is duePropertyMetadata::getPropertyValue i figured.

Perhaps the ship has sailed, and this complements our PHP74 support in general. But IMHO one should validate the raw payload, before hydrating a non-nullable property.

@HeahDude
Copy link
ContributorAuthor

Not in the context of the form given#36492.
But I agree for validating the raw payload as a preference, however this is still a shortcut for not null properties that could be read as null for partially hydrated objects or when relying onhttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Validator/ValidatorInterface.php#L55 to validate the raw payload.

This allows to decouple model from hydrators, and explicitly declare not nullable props with the type instead of a not null constraint.
The naming is bit confusing though. What about@Assert\NotNullableTypes?

@HeahDude
Copy link
ContributorAuthor

Or@Assert\Initialized?

@ro0NL
Copy link
Contributor

does that meanAssert\Initialized still translates toNotNull constraints, based on the fact we also getnull from properties in uninitialized state? Doesnt that trigger side effects if the value is trulynull?

In general i likeAssert\Initialized as a class constraint, to ensure all properties areinitialized.

However, IMHO it's developer concern. De facto, objects shouldnt be in uninitialized state after construct.

@fabpot
Copy link
Member

I'm not sure I like this StrictTypes constraint. What do you think@xabbuh?

@fabpot
Copy link
Member

Closing for now as the PR is stale. Feel free to reopen when you have time. Thank you.

@fabpotfabpot closed thisSep 7, 2020
@nicolas-grekasnicolas-grekas modified the milestones:next,5.2Oct 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

5 participants

@HeahDude@ro0NL@fabpot@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp