Hi internals,
When the constructor property promotion landed, the question of how it
interacts with attributes on promoted properties did not get fully
resolved. See https://wiki.php.net/rfc/constructor_promotion#attributes for
what the issue is.
The behavior that landed was to apply the attribute is applied to both the
parameter and the property. However, this was with the understanding that
we may have to adjust the behavior later, in particular depending on how
the whole "attribute target validation" turns out. The way it is now, using
a property-only attribute on a promoted parameter may result in spurious
validation errors.
I wanted to bring up this topic now to make sure it's not forgotten... I
still don't really know what the best behavior here is.
Regards,
Nikita
Hi internals,
When the constructor property promotion landed, the question of how it
interacts with attributes on promoted properties did not get fully
resolved. See https://wiki.php.net/rfc/constructor_promotion#attributes
for what the issue is.The behavior that landed was to apply the attribute is applied to both the
parameter and the property. However, this was with the understanding that
we may have to adjust the behavior later, in particular depending on how
the whole "attribute target validation" turns out. The way it is now, using
a property-only attribute on a promoted parameter may result in spurious
validation errors.I wanted to bring up this topic now to make sure it's not forgotten... I
still don't really know what the best behavior here is.
imho, we should pick the 80% use-case and advise to desugar the code if
other behavior is desired. For me the 80% case is that the attribute only
applies to the property, not to the parameter.
Regards,
Nikita
On Mon, 28 Sep 2020 at 13:56, Benjamin Eberlei kontakt@beberlei.de
wrote:
imho, we should pick the 80% use-case and advise to desugar the code if
other behavior is desired. For me the 80% case is that the attribute only
applies to the property, not to the parameter.
+1 for the desugaring advice in this case, however as a matter of
consistency I'd rather have it either apply to both the property and the
parameter, or just throw an exception if used on promoted properties,
whatever the 80% use-case is (source?)
Kind regards,
— Benjamin
On Mon, Sep 28, 2020 at 2:25 PM Benjamin Morel benjamin.morel@gmail.com
wrote:
On Mon, 28 Sep 2020 at 13:56, Benjamin Eberlei kontakt@beberlei.de
wrote:imho, we should pick the 80% use-case and advise to desugar the code if
other behavior is desired. For me the 80% case is that the attribute only
applies to the property, not to the parameter.+1 for the desugaring advice in this case, however as a matter of
consistency I'd rather have it either apply to both the property and the
parameter, or just throw an exception if used on promoted properties,
whatever the 80% use-case is (source?)
We should assume that there are very few attributes that make sense to
apply to both a property and an argument. I can't find any good example
where this makes sense.
Library developers shouldn't be forced to allow target argument only that
it works in the ctor promotion case.
I assume the 80% case is properties, because attributes did not have
docblock annotations yet, that means this use-case isn't even possible at
the moment. Yet annotations on properties are widespread (Doctrine ORM,
symfony validator, ...).
Kind regards,
— Benjamin
imho, we should pick the 80% use-case and advise to desugar the code if
other behavior is desired. For me the 80% case is that the attribute
only
applies to the property, not to the parameter.+1 for the desugaring advice in this case, however as a matter of
consistency I'd rather have it either apply to both the property and the
parameter, or just throw an exception if used on promoted properties,
whatever the 80% use-case is (source?)We should assume that there are very few attributes that make sense to
apply to both a property and an argument. I can't find any good example
where this makes sense.Library developers shouldn't be forced to allow target argument only that
it works in the ctor promotion case.I assume the 80% case is properties, because attributes did not have
docblock annotations yet, that means this use-case isn't even possible at
the moment. Yet annotations on properties are widespread (Doctrine ORM,
symfony validator, ...).
I'm 100% with Benjamin here, this is what will be the most useful to me
also.
On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:
I assume the 80% case is properties, because attributes did not have
docblock annotations yet, that means this use-case isn't even possible at
the moment. Yet annotations on properties are widespread (Doctrine ORM,
symfony validator, ...).I'm 100% with Benjamin here, this is what will be the most useful to me
also.
To be clear, I don't have a strong opinion against yours, I'm just pointing
out the fact that even though it might be useful, it might also be
confusing and create yet another WTF moment in PHP for developers. Sure, it
might make more sense to apply to the property. Sure, so far annotations
weren't possible on parameters. But is that obvious to the average
developer writing the attribute? A few years down the road, DI containers
may have broad support for annotating parameters for injection. Will it
still be obvious then that an attribute on a promoted property applies to
the property only?
I do agree that applying the attribute to both the property and the
parameter will probably never be useful, though. So, throwing an exception
and forcing the de-sugaring feels like the most sensible thing to do for me
in this case!
— Benjamin
On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel benjamin.morel@gmail.com
wrote:
On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:I assume the 80% case is properties, because attributes did not have
docblock annotations yet, that means this use-case isn't even possible at
the moment. Yet annotations on properties are widespread (Doctrine ORM,
symfony validator, ...).I'm 100% with Benjamin here, this is what will be the most useful to me
also.To be clear, I don't have a strong opinion against yours, I'm just
pointing out the fact that even though it might be useful, it might also be
confusing and create yet another WTF moment in PHP for developers. Sure, it
might make more sense to apply to the property. Sure, so far annotations
weren't possible on parameters. But is that obvious to the average
developer writing the attribute? A few years down the road, DI containers
may have broad support for annotating parameters for injection. Will it
still be obvious then that an attribute on a promoted property applies to
the property only?I do agree that applying the attribute to both the property and the
parameter will probably never be useful, though. So, throwing an exception
and forcing the de-sugaring feels like the most sensible thing to do for me
in this case!
I haven't checked if this is possible in the code doing the "desugering",
but what if we had an attribute on the constructor that could specify where
the attributes should apply to?
#[AttributePromotion(Attribute::TARGET_PROPERTY)]
public function __construct(#[Foo] public string $bar) {}
Then we could apply it to both by default, which is what is probably the
expected approach, and users could change it to apply only to properties,
which is what is the use-case that makes most sense.
— Benjamin
On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel benjamin.morel@gmail.com
wrote:On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:I assume the 80% case is properties, because attributes did not have
docblock annotations yet, that means this use-case isn't even possible at
the moment. Yet annotations on properties are widespread (Doctrine ORM,
symfony validator, ...).I'm 100% with Benjamin here, this is what will be the most useful to me
also.To be clear, I don't have a strong opinion against yours, I'm just
pointing out the fact that even though it might be useful, it might also be
confusing and create yet another WTF moment in PHP for developers. Sure, it
might make more sense to apply to the property. Sure, so far annotations
weren't possible on parameters. But is that obvious to the average
developer writing the attribute? A few years down the road, DI containers
may have broad support for annotating parameters for injection. Will it
still be obvious then that an attribute on a promoted property applies to
the property only?I do agree that applying the attribute to both the property and the
parameter will probably never be useful, though. So, throwing an exception
and forcing the de-sugaring feels like the most sensible thing to do for me
in this case!I haven't checked if this is possible in the code doing the "desugering",
but what if we had an attribute on the constructor that could specify where
the attributes should apply to?#[AttributePromotion(Attribute::TARGET_PROPERTY)]
public function __construct(#[Foo] public string $bar) {}Then we could apply it to both by default, which is what is probably the
expected approach, and users could change it to apply only to properties,
which is what is the use-case that makes most sense.— Benjamin
From a user experience POV, I'd almost expect the opposite.
If I mark the attribute as being for properties, it gets applied to the property.
If I mark the attribute as being for parameters, it gets applied to the parameter.
If I mark it for both, or don't restrict it at all, it applies to both.
That may not be how the logic is currently implemented but that's what as a user I'd find least-surprising.
--Larry Garfield
On Tue, Sep 29, 2020 at 3:45 PM Larry Garfield larry@garfieldtech.com
wrote:
On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel <benjamin.morel@gmail.com
wrote:
On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas <
nicolas.grekas+php@gmail.com>
wrote:I assume the 80% case is properties, because attributes did not have
docblock annotations yet, that means this use-case isn't even
possible at
the moment. Yet annotations on properties are widespread (Doctrine
ORM,
symfony validator, ...).I'm 100% with Benjamin here, this is what will be the most useful to
me
also.To be clear, I don't have a strong opinion against yours, I'm just
pointing out the fact that even though it might be useful, it might
also be
confusing and create yet another WTF moment in PHP for developers.
Sure, it
might make more sense to apply to the property. Sure, so far
annotations
weren't possible on parameters. But is that obvious to the average
developer writing the attribute? A few years down the road, DI
containers
may have broad support for annotating parameters for injection. Will it
still be obvious then that an attribute on a promoted property applies
to
the property only?I do agree that applying the attribute to both the property and the
parameter will probably never be useful, though. So, throwing an
exception
and forcing the de-sugaring feels like the most sensible thing to do
for me
in this case!I haven't checked if this is possible in the code doing the "desugering",
but what if we had an attribute on the constructor that could specify
where
the attributes should apply to?#[AttributePromotion(Attribute::TARGET_PROPERTY)]
public function __construct(#[Foo] public string $bar) {}Then we could apply it to both by default, which is what is probably the
expected approach, and users could change it to apply only to properties,
which is what is the use-case that makes most sense.— Benjamin
From a user experience POV, I'd almost expect the opposite.
If I mark the attribute as being for properties, it gets applied to the
property.If I mark the attribute as being for parameters, it gets applied to the
parameter.If I mark it for both, or don't restrict it at all, it applies to both.
That may not be how the logic is currently implemented but that's what as
a user I'd find least-surprising.
The problem with this approach is that it would require autoloading the
attributes when they are assigned to either the internal property or
parameter struct, but we have the design goal not to trigger autoloading
unless newInstance() or getArguments() is called. What you could do in
userland code is handle this case yourself and never newInstance()
attributes that don't apply to the right "thing" (parameter vs property).
but that would defer the problem to userland with some annoying piece of
code.
--Larry Garfield
--
To unsubscribe, visit: https://www.php.net/unsub.php
On Tue, Sep 29, 2020 at 3:57 PM Benjamin Eberlei kontakt@beberlei.de
wrote:
On Tue, Sep 29, 2020 at 3:45 PM Larry Garfield larry@garfieldtech.com
wrote:On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel <
benjamin.morel@gmail.comwrote:
On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas <
nicolas.grekas+php@gmail.com>
wrote:I assume the 80% case is properties, because attributes did not have
docblock annotations yet, that means this use-case isn't even
possible at
the moment. Yet annotations on properties are widespread (Doctrine
ORM,
symfony validator, ...).I'm 100% with Benjamin here, this is what will be the most useful to
me
also.To be clear, I don't have a strong opinion against yours, I'm just
pointing out the fact that even though it might be useful, it might
also be
confusing and create yet another WTF moment in PHP for developers.
Sure, it
might make more sense to apply to the property. Sure, so far
annotations
weren't possible on parameters. But is that obvious to the average
developer writing the attribute? A few years down the road, DI
containers
may have broad support for annotating parameters for injection. Will
it
still be obvious then that an attribute on a promoted property
applies
to
the property only?I do agree that applying the attribute to both the property and the
parameter will probably never be useful, though. So, throwing an
exception
and forcing the de-sugaring feels like the most sensible thing to do
for me
in this case!I haven't checked if this is possible in the code doing the
"desugering",
but what if we had an attribute on the constructor that could specify
where
the attributes should apply to?#[AttributePromotion(Attribute::TARGET_PROPERTY)]
public function __construct(#[Foo] public string $bar) {}Then we could apply it to both by default, which is what is probably
the
expected approach, and users could change it to apply only to
properties,
which is what is the use-case that makes most sense.— Benjamin
From a user experience POV, I'd almost expect the opposite.
If I mark the attribute as being for properties, it gets applied to the
property.If I mark the attribute as being for parameters, it gets applied to the
parameter.If I mark it for both, or don't restrict it at all, it applies to both.
That may not be how the logic is currently implemented but that's what as
a user I'd find least-surprising.The problem with this approach is that it would require autoloading the
attributes when they are assigned to either the internal property or
parameter struct, but we have the design goal not to trigger autoloading
unless newInstance() or getArguments() is called. What you could do in
userland code is handle this case yourself and never newInstance()
attributes that don't apply to the right "thing" (parameter vs property).
but that would defer the problem to userland with some annoying piece of
code.
So, as there seems to be resistance to applying the attribute to properties
only I only see a couple of options:
-
Forbid combining attributes and promotion.
-
Relax attribute validation for this case, as implemented in
https://github.com/php/php-src/pull/6244. I think if we otherwise stick to
the current behavior, this is what we should do to avoid false-positive
errors. -
Disambiguate what is desired somehow. __construct(#[PropAttr] public int
$x) vs __construct(public #[ParamAttr] int $x). This is ... a bit too much
magic :)
Nikita
So, as there seems to be resistance to applying the attribute to properties
only I only see a couple of options:
- Forbid combining attributes and promotion.
That means we can still decide to allow it later, right? Which gives us
more time to think this through in stead of deciding in a hurry.
Regards,
Dik Takken
Le mer. 30 sept. 2020 à 12:45, Nikita Popov nikita.ppv@gmail.com a écrit :
On Tue, Sep 29, 2020 at 3:57 PM Benjamin Eberlei kontakt@beberlei.de
wrote:On Tue, Sep 29, 2020 at 3:45 PM Larry Garfield larry@garfieldtech.com
wrote:On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel <
benjamin.morel@gmail.comwrote:
On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas <
nicolas.grekas+php@gmail.com>
wrote:I assume the 80% case is properties, because attributes did not
havedocblock annotations yet, that means this use-case isn't even
possible at
the moment. Yet annotations on properties are widespread
(Doctrine
ORM,
symfony validator, ...).I'm 100% with Benjamin here, this is what will be the most useful
to
me
also.To be clear, I don't have a strong opinion against yours, I'm just
pointing out the fact that even though it might be useful, it might
also be
confusing and create yet another WTF moment in PHP for developers.
Sure, it
might make more sense to apply to the property. Sure, so far
annotations
weren't possible on parameters. But is that obvious to the average
developer writing the attribute? A few years down the road, DI
containers
may have broad support for annotating parameters for injection.
Will
it
still be obvious then that an attribute on a promoted property
applies
to
the property only?I do agree that applying the attribute to both the property and the
parameter will probably never be useful, though. So, throwing an
exception
and forcing the de-sugaring feels like the most sensible thing to
do
for me
in this case!I haven't checked if this is possible in the code doing the
"desugering",
but what if we had an attribute on the constructor that could specify
where
the attributes should apply to?#[AttributePromotion(Attribute::TARGET_PROPERTY)]
public function __construct(#[Foo] public string $bar) {}Then we could apply it to both by default, which is what is probably
the
expected approach, and users could change it to apply only to
properties,
which is what is the use-case that makes most sense.— Benjamin
From a user experience POV, I'd almost expect the opposite.
If I mark the attribute as being for properties, it gets applied to the
property.If I mark the attribute as being for parameters, it gets applied to the
parameter.If I mark it for both, or don't restrict it at all, it applies to both.
That may not be how the logic is currently implemented but that's what
as
a user I'd find least-surprising.The problem with this approach is that it would require autoloading the
attributes when they are assigned to either the internal property or
parameter struct, but we have the design goal not to trigger
autoloading
unless newInstance() or getArguments() is called. What you could do in
userland code is handle this case yourself and never newInstance()
attributes that don't apply to the right "thing" (parameter vs property).
but that would defer the problem to userland with some annoying piece of
code.So, as there seems to be resistance to applying the attribute to properties
only I only see a couple of options:
- Forbid combining attributes and promotion.
That'd be quite deceptive to me.
- Relax attribute validation for this case, as implemented in
https://github.com/php/php-src/pull/6244. I think if we otherwise stick to
the current behavior, this is what we should do to avoid false-positive
errors.
This makes a lot of sense actually!
Just to be sure, "ignore the error" means that attributes that target
properties would be applied only to the property, and attributes that
target params only to the param, isn't it? Or does it mean that they would
still be applied to both, but not error would be raised?
- Disambiguate what is desired somehow. __construct(#[PropAttr] public int
$x) vs __construct(public #[ParamAttr] int $x). This is ... a bit too much
magic :)
a bit too much magic I agree :)
Nicolas
On Wed, Sep 30, 2020 at 2:13 PM Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:
Le mer. 30 sept. 2020 à 12:45, Nikita Popov nikita.ppv@gmail.com a
écrit :On Tue, Sep 29, 2020 at 3:57 PM Benjamin Eberlei kontakt@beberlei.de
wrote:On Tue, Sep 29, 2020 at 3:45 PM Larry Garfield larry@garfieldtech.com
wrote:On Mon, Sep 28, 2020 at 4:35 PM Benjamin Morel <
benjamin.morel@gmail.comwrote:
On Mon, 28 Sep 2020 at 15:17, Nicolas Grekas <
nicolas.grekas+php@gmail.com>
wrote:I assume the 80% case is properties, because attributes did not
havedocblock annotations yet, that means this use-case isn't even
possible at
the moment. Yet annotations on properties are widespread
(Doctrine
ORM,
symfony validator, ...).I'm 100% with Benjamin here, this is what will be the most
useful to
me
also.To be clear, I don't have a strong opinion against yours, I'm just
pointing out the fact that even though it might be useful, it
might
also be
confusing and create yet another WTF moment in PHP for developers.
Sure, it
might make more sense to apply to the property. Sure, so far
annotations
weren't possible on parameters. But is that obvious to the average
developer writing the attribute? A few years down the road, DI
containers
may have broad support for annotating parameters for injection.
Will
it
still be obvious then that an attribute on a promoted property
applies
to
the property only?I do agree that applying the attribute to both the property and
the
parameter will probably never be useful, though. So, throwing an
exception
and forcing the de-sugaring feels like the most sensible thing to
do
for me
in this case!I haven't checked if this is possible in the code doing the
"desugering",
but what if we had an attribute on the constructor that could
specify
where
the attributes should apply to?#[AttributePromotion(Attribute::TARGET_PROPERTY)]
public function __construct(#[Foo] public string $bar) {}Then we could apply it to both by default, which is what is probably
the
expected approach, and users could change it to apply only to
properties,
which is what is the use-case that makes most sense.— Benjamin
From a user experience POV, I'd almost expect the opposite.
If I mark the attribute as being for properties, it gets applied to
the
property.If I mark the attribute as being for parameters, it gets applied to
the
parameter.If I mark it for both, or don't restrict it at all, it applies to
both.That may not be how the logic is currently implemented but that's
what as
a user I'd find least-surprising.The problem with this approach is that it would require autoloading the
attributes when they are assigned to either the internal property or
parameter struct, but we have the design goal not to trigger
autoloading
unless newInstance() or getArguments() is called. What you could do in
userland code is handle this case yourself and never newInstance()
attributes that don't apply to the right "thing" (parameter vs
property).
but that would defer the problem to userland with some annoying piece of
code.So, as there seems to be resistance to applying the attribute to
properties
only I only see a couple of options:
- Forbid combining attributes and promotion.
That'd be quite deceptive to me.
- Relax attribute validation for this case, as implemented in
https://github.com/php/php-src/pull/6244. I think if we otherwise stick
to
the current behavior, this is what we should do to avoid false-positive
errors.This makes a lot of sense actually!
Just to be sure, "ignore the error" means that attributes that target
properties would be applied only to the property, and attributes that
target params only to the param, isn't it? Or does it mean that they would
still be applied to both, but not error would be raised?
Unfortunately they would still be applied to both, but without error. As
Benjamin mentioned before, there's no way to prevent applying them to both
without loading the attribute definition and checking the targets on it.
Nikita
Hi internals,
When the constructor property promotion landed, the question of how it
interacts with attributes on promoted properties did not get fully
resolved. See https://wiki.php.net/rfc/constructor_promotion#attributes
for what the issue is.The behavior that landed was to apply the attribute is applied to both the
parameter and the property. However, this was with the understanding that
we may have to adjust the behavior later, in particular depending on how
the whole "attribute target validation" turns out. The way it is now, using
a property-only attribute on a promoted parameter may result in spurious
validation errors.I wanted to bring up this topic now to make sure it's not forgotten... I
still don't really know what the best behavior here is.Regards,
Nikita
I want to bump this topic, we should resolve this one way or another soon.
My two proposals are:
https://github.com/php/php-src/pull/6244: Suppress target validation errors
for promoted properties (for property/parameter targets, the rest gets
validated as usual).
https://github.com/php/php-src/pull/6285: Forbid use of attributes on
promoted properties altogether, punting on making a decision here.
Regards,
Nikita
When the constructor property promotion landed, the question of how it
interacts with attributes on promoted properties did not get fully
resolved. See https://wiki.php.net/rfc/constructor_promotion#attributes
for
what the issue is.The behavior that landed was to apply the attribute is applied to both the
parameter and the property. However, this was with the understanding that
we may have to adjust the behavior later, in particular depending on how
the whole "attribute target validation" turns out. The way it is now, using
a property-only attribute on a promoted parameter may result in spurious
validation errors.I wanted to bring up this topic now to make sure it's not forgotten... I
still don't really know what the best behavior here is.
My opinion on constructor property promotion (CPP) is that it's something
for small value object classes and should probably be regarded as
code-smell on larger classes. At the same time, annotations belong with
more complex objects and not so much with small "struct-like" classes.
Given that position, I think we should err towards strictness in how
attributes are applied to CPP declarations. That is, we should require
them to be meaningfully applicable to both arguments and properties in
order to be used in a CPP context. If that's a problem for the consumer,
then they should avoid use of CPP.
-Sara
My opinion on constructor property promotion (CPP) is that it's something
for small value object classes and should probably be regarded as
code-smell on larger classes. At the same time, annotations belong with
more complex objects and not so much with small "struct-like" classes.Given that position, I think we should err towards strictness in how
attributes are applied to CPP declarations. That is, we should require
them to be meaningfully applicable to both arguments and properties in
order to be used in a CPP context. If that's a problem for the consumer,
then they should avoid use of CPP.-Sara
The current usage of annotations is quite often with small struct-like
classes though - I mainly use annotations (and will use attributes) for
data that needs to be validated, or entity-like classes that contain
data. Those classes are small and simple and would benefit greatly from
CPP and attributes used in combination, so it would be a pity to make
that impossible.
From my understanding suppressing the validation errors in this
particular case would be a good solution, or are there any serious
downsides to that?
From my understanding suppressing the validation errors in this
particular case would be a good solution, or are there any serious
downsides to that?
The downside presumably is that a library author could implement an
attribute with a property-only restriction, and therefore write code
handling parameter attributes with the assumption that that attribute won't
be present. If a consumer of the library makes use of constructor property
promotion, that assumption can be violated. I can't quite picture what that
code would look like, though, since it's always necessary to filter the
list of attributes to exclude those from other libraries.
Regards,
Rowan Tommins
[IMSoP]
On Tue, Oct 6, 2020 at 6:59 PM Rowan Tommins rowan.collins@gmail.com
wrote:
From my understanding suppressing the validation errors in this
particular case would be a good solution, or are there any serious
downsides to that?The downside presumably is that a library author could implement an
attribute with a property-only restriction, and therefore write code
handling parameter attributes with the assumption that that attribute won't
be present. If a consumer of the library makes use of constructor property
promotion, that assumption can be violated. I can't quite picture what that
code would look like, though, since it's always necessary to filter the
list of attributes to exclude those from other libraries.
Yeah, this is my own view as well. If an attribute appears in an unexpected
place, you'd just ignore it the same way you ignore all attributes you do
not "own". For manual implementations, you wouldn't even notice (because
for a property-only attribute, you'd only be looking at properties, duh)
and general attribute handling libraries can easily accommodate this case
(if it doesn't just happen automatically).
Regards,
Nikita
My opinion on constructor property promotion (CPP) is that it's something
for small value object classes and should probably be regarded as
code-smell on larger classes. At the same time, annotations belong with
more complex objects and not so much with small "struct-like" classes.Given that position, I think we should err towards strictness in how
attributes are applied to CPP declarations. That is, we should require
them to be meaningfully applicable to both arguments and properties in
order to be used in a CPP context. If that's a problem for the consumer,
then they should avoid use of CPP.-Sara
The current usage of annotations is quite often with small struct-like
classes though - I mainly use annotations (and will use attributes) for
data that needs to be validated, or entity-like classes that contain
data. Those classes are small and simple and would benefit greatly from
CPP and attributes used in combination, so it would be a pity to make
that impossible.From my understanding suppressing the validation errors in this
particular case would be a good solution, or are there any serious
downsides to that?
This is my argument as well. Attributes + CPP are a powerful combination.
Attributes are an important use-csae for data transfer objects, to
configure validation or serialization for example:
class ChangeFirstNameRequest
{
public function __construct(
#[Validate\IsPositive]
public int $id;
#[Validate\NotBlank]
#[Json("first_name")]
public string $firstName;
) {}
}
--
To unsubscribe, visit: https://www.php.net/unsub.php
Hi!
My opinion on constructor property promotion (CPP) is that it's something
for small value object classes and should probably be regarded as
code-smell on larger classes. At the same time, annotations belong with
more complex objects and not so much with small "struct-like" classes.
Just wanted to chime in on this:
I can foresee already the impact CPP will have on dependency injection:
sometimes you just have complex business logic with lots of (auto-wired)
dependencies and have to pull in n+1 classes into the constructor.
Currently 5+ deps create 3*(5+) codelines for not much value. My point
is that I see much more value outside of "value object classes" with
auto-wiring, reducing boilerplate a lot.
That said: I err on the side of forbidding it, if we've no clear
consensus how to do it!
thanks,
- Makrs