Skip to content

Add property types and return types in constraints #18362

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

Merged

Conversation

alexandre-daubois
Copy link
Member

Forgot a "few" ones in constraints 😄

@alexandre-daubois alexandre-daubois requested a review from xabbuh as a code owner June 1, 2023 11:52
@carsonbot carsonbot added this to the 6.2 milestone Jun 1, 2023
@@ -69,7 +69,7 @@ An Invoice entity::
* @var InvoiceSubjectInterface
Copy link
Contributor

Choose a reason for hiding this comment

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

Can be removed then

@@ -112,7 +114,7 @@ that a date must at least be the next day:
class Order
{
#[Assert\GreaterThan('today')]
protected $deliveryDate;
protected \DateTime $deliveryDate;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
protected \DateTime $deliveryDate;
protected \DateTimeInterface $deliveryDate;

?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated!

@@ -169,7 +173,7 @@ dates. If you want to fix the timezone, append it to the date string:
class Order
{
#[Assert\GreaterThan('today UTC')]
protected $deliveryDate;
protected \DateTime $deliveryDate;
Copy link
Contributor

Choose a reason for hiding this comment

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

same for the others?

@alexandre-daubois alexandre-daubois force-pushed the missing-types-in-constraints branch from b6c7f68 to 056a3bc Compare June 1, 2023 12:17
@OskarStark OskarStark requested a review from javiereguiluz June 1, 2023 12:21
@alexandre-daubois alexandre-daubois force-pushed the missing-types-in-constraints branch from 056a3bc to 2339f19 Compare June 1, 2023 12:22
@javiereguiluz
Copy link
Member

Alex, thanks a million for the colossal effort of adding so many types 🙇🙇🙇

@javiereguiluz javiereguiluz merged commit 844af5d into symfony:6.2 Jun 5, 2023
javiereguiluz added a commit that referenced this pull request Jul 23, 2024
…pe´ constraint is used (xabbuh)

This PR was merged into the 6.4 branch.

Discussion
----------

[Validator] drop type-hints for properties where the ´Type´ constraint is used

partially revert #18362 as having the native type-hints prevents the `Type` constraint to be applied in a meaningful way (if the types don't match, PHP will error before)

Commits
-------

27a06be drop type-hints for properties where the Type constraint is used
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants