Hello internals!
Since «traits» are often an indicator of not very good code and many may not use them quite correctly, for example, as helpers, I suggest adding support for the expects keyword to indicate that the trait is part of the code decomposition taking into account ISP.
For example:
// Definition
trait LoggerTrait expects LoggerInterface
{
// ...
}
// Usage
class MyService
{
use LoggerTrait; // Fatal Error: Class MyService expects LoggerInterface to be implemented
}
class MyService2 implements LoggerInterface
{
use LoggerTrait; // OK
}
How relevant do you think this idea/proposal is? And what possible problems or solutions will this entail in the future?
Kirill Nesmeyanov
Since «traits» are often an indicator of not very good code and many may not use them quite correctly, for example, as helpers, I suggest adding support for the
expectskeyword to indicate that the trait is part of the code decomposition taking into account ISP.
Prior art: @psalm-require-extends and @psalm-require-implements Psalm
annotations: https://psalm.dev/docs/annotating_code/supported_annotations/#psalm-require-extends
--
Best regards,
Bruce Weirdan mailto:weirdan@gmail.com
Since «traits» are often an indicator of not very good code and many may not use them quite correctly, for example, as helpers, I suggest adding support for the
expectskeyword to indicate that the trait is part of the code decomposition taking into account ISP.
I like this idea, but I think it'd be even more useful if it could be used with classes as well as interfaces. This would allow traits to reject their use outside an intended class hierarchy.
(And, while we're at it: could it be useful for a trait to expect another trait?)
Hello internals!
Since «traits» are often an indicator of not very good code and many
may not use them quite correctly, for example, as helpers, I suggest
adding support for theexpectskeyword to indicate that the trait is
part of the code decomposition taking into account ISP.For example:
// Definition trait LoggerTrait expects LoggerInterface { // ... } // Usage class MyService { use LoggerTrait; // Fatal Error: Class MyService expects LoggerInterface to be implemented } class MyService2 implements LoggerInterface { use LoggerTrait; // OK }How relevant do you think this idea/proposal is? And what possible
problems or solutions will this entail in the future?
I can't say this has ever been an issue for me when using traits. I've never had a trait that needed to be used by a class with a given interface. What I usually have is a trait that is a full or mostly implementation of an interface. So this would be much more useful to me:
trait LoggerTrait implements LoggerInterface { ... }
(Perhaps some way to indicate that it mostly implements, and the rest are abstract methods? Or require the other methods to be explicitly abstract?)
I think it's subtly different, as it approaches the question from the other direction.
--Larry Garfield
Среда, 5 января 2022, 4:17 +03:00 от Larry Garfield larry@garfieldtech.com:
Hello internals!
Since «traits» are often an indicator of not very good code and many
may not use them quite correctly, for example, as helpers, I suggest
adding support for theexpectskeyword to indicate that the trait is
part of the code decomposition taking into account ISP.For example:
// Definition trait LoggerTrait expects LoggerInterface { // ... } // Usage class MyService { use LoggerTrait; // Fatal Error: Class MyService expects LoggerInterface to be implemented } class MyService2 implements LoggerInterface { use LoggerTrait; // OK }How relevant do you think this idea/proposal is? And what possible
problems or solutions will this entail in the future?
I can't say this has ever been an issue for me when using traits. I've never had a trait that needed to be used by a class with a given interface. What I usually have is a trait that is a full or mostly implementation of an interface. So this would be much more useful to me:trait LoggerTrait implements LoggerInterface { ... }
(Perhaps some way to indicate that it mostly implements, and the rest are abstract methods? Or require the other methods to be explicitly abstract?)
I think it's subtly different, as it approaches the question from the other direction.
--Larry Garfield
--
To unsubscribe, visit: https://www.php.net/unsub.php
I don't like this option, as now traits are a mechanism for injecting code into classes or other traits. However, they do not affect the type system in any way.
And if we add «trait implements», then each use XxxTrait will affect the type system. That is, instead of changing the child type by explicitly specifying which interfaces it implements, the type will be modified taking into account the mechanism that did not change this type initially.
I don’t know, it’s just a feeling that this is not correct. And I have no other arguments against the concept of «implementation Interface». I need to think about it…
Kirill Nesmeyanov
Hello internals!
Since «traits» are often an indicator of not very good code and many may not use them quite correctly, for example, as helpers, I suggest adding support for the
expectskeyword to indicate that the trait is part of the code decomposition taking into account ISP.
For example:
// Definition
trait LoggerTrait expects LoggerInterface { // ... }
// Usage
class MyService { use LoggerTrait; // Fatal Error: Class MyService expects LoggerInterface to be implemented }
class MyService2 implements LoggerInterface { use LoggerTrait; // OK }
How relevant do you think this idea/proposal is? And what possible problems or solutions will this entail in the future?
Kirill Nesmeyanov
Hello Kirill
I really like to have this feature, this is not just useful for implementing interfaces, but also to enforce where a trait can be used.
e.g: if ReadUserInputTrait should only be used in console commands, so putting a requirement to implement the console command interface, will ensure that the trait is not misused.
However, i personally prefer the HackLang syntax of doing this.
Which is:
trait Foo
{
require implements Bar;
require extends Baz;
}
This syntax also will result in no new keywords, which is always preferred, but not necessarily a requirement.
ref: https://docs.hhvm.com/hack/traits-and-interfaces/trait-and-interface-requirements
Regards,
Saif Eddin Gmati.
Since «traits» are often an indicator of not very good code and many may not use them quite correctly, for example, as helpers, I suggest adding support for the
expectskeyword to indicate that the trait is part of the code decomposition taking into account ISP.
Hi Kirill,
I'm a little confused what problem this is trying to solve - in what way
does using a trait in the "wrong" inheritance hierarchy constitute "abuse"?
Prior art: @psalm-require-extends and @psalm-require-implements Psalm
annotations:https://psalm.dev/docs/annotating_code/supported_annotations/#psalm-require-extends
... and on 05/01/2022 12:36, Saif Eddin Gmati wrote:
ref:https://docs.hhvm.com/hack/traits-and-interfaces/trait-and-interface-requirements
The examples in both of these cases appear to be using the requirements
primarily as short-hand for listing abstract methods in the trait.
For instance:
interface FooInterface {
public function doFoo();
}
trait SillyFooTrait {
require implements FooInterface;
public function doFooTwice() {
$this->doFoo();
$this->doFoo();
}
}
Is essentially equivalent to:
trait SillyFooTrait {
abstract public function doFoo();
public function doFooTwice() {
$this->doFoo();
$this->doFoo();
}
}
In other words, the requirement is there because it is actually a
requirement for the trait to work correctly, not because of some
perceived "correct" use of the trait. This doesn't seem to match your
reasoning for proposing the syntax, so maybe I'm missing something?
Regards,
--
Rowan Tommins
[IMSoP]
How relevant do you think this idea/proposal is? And what possible
problems or solutions will this entail in the future?
I'm not convinced there's a reasonable need for it. The very nature of
finding yourself in a situation where you want any class using a trait to
also require other things outside the trait kind of suggests you really
want to be using interfaces or abstract classes anyway.
There is a similar concept for what I think you're trying to achieve in
Java, though, which could also be useful in PHP if it was feasible within
the engine - the ability to provide a default method implementation on
interfaces themselves.
Short of that, we can already effectively get there with the tools we have;
an abstract class can use a trait and define abstract or concrete methods,
and in doing so can implement one or more interfaces. Obviously traits can
also declare abstract methods but I assume it's the identity/type aspect of
an interface you want here which isn't satisfied by that approach.
Also worth noting, although I can't say I'm familiar with the mechanics of
traits in PHP's implementation, my understanding has always been that
they're effectively compiler-level copy and paste in to a class so I'm not
sure what interfaces are implemented by the class (or conversely allowing a
trait to implement an interface) would be readily achievable.
That's my two cents, good luck with whatever you're trying to do anyway.
- David
--
Kirill Nesmeyanov
How relevant do you think this idea/proposal is? And what possible
problems or solutions will this entail in the future?I'm not convinced there's a reasonable need for it. The very nature of
finding yourself in a situation where you want any class using a trait to
also require other things outside the trait kind of suggests you really
want to be using interfaces or abstract classes anyway.There is a similar concept for what I think you're trying to achieve in
Java, though, which could also be useful in PHP if it was feasible within
the engine - the ability to provide a default method implementation on
interfaces themselves.Short of that, we can already effectively get there with the tools we have;
an abstract class can use a trait and define abstract or concrete methods,
and in doing so can implement one or more interfaces. Obviously traits can
also declare abstract methods but I assume it's the identity/type aspect of
an interface you want here which isn't satisfied by that approach.Also worth noting, although I can't say I'm familiar with the mechanics of
traits in PHP's implementation, my understanding has always been that
they're effectively compiler-level copy and paste in to a class so I'm not
sure what interfaces are implemented by the class (or conversely allowing a
trait to implement an interface) would be readily achievable.That's my two cents, good luck with whatever you're trying to do anyway.
- David
--
Kirill Nesmeyanov
First, I'm someone that mainly uses traits to implement the functionality
defined in an interface. I think that's one of the best uses for them.
However, I'm personally not a huge fan of overly restrictive things. For
instance, while there are definitely some use cases for them, I need a
REALLY good reason to justify making a property/method private instead of
protected, or making a class final.
As such, I think this would be better if it didn't throw a fatal error.
When you make it optional, however, I think you are left with something
that can be handled with an attribute just as well as a new keyword:
#[Expects('MyInterface')]
trait foo { }
However, there might be value in generating a notice/warning, and I think
that would require a keyword, correct? (Not that up to speed on
annotations). Another option might be two support two new keywords:
requires and expects. The former would throw an error if the interface
isn't implemented while the latter will throw a warning/notice/nothing.
Another option (and I haven't thought about this one enough to decide if I
like it) would be to have the expected interface automatically implemented
in the using class. This would allow the trait to be written under the
assumption it has access to the methods defined in the interface, and will
then throw an error if any of the methods are not implemented in the using
class:
interface foo {
function a();
function b();
}
trait bar expects foo {
function c(){
return $this->a() + $this->b();
}
}
class baz {
use foo;
}
The above would throw an error since a() and b() are never implemented and
baz is implementing the foo interface. You can currently get the same
behavior if you define a() and b() as abstract in the trait. However, this
doesn't give you the added benefit of utilizing the interface automatically
within the type system. The more I think about it, the less I like this
idea, since it doesn't require that much additional work to make the code
clearer by explicitly implementing the interface on the class if you want
it implemented. However, I'll go ahead and leave it here because it might
help generate some other ideas.
--
Chase Peeler
chasepeeler@gmail.com
First, I'm someone that mainly uses traits to implement the functionality
defined in an interface. I think that's one of the best uses for them.
However, I'm personally not a huge fan of overly restrictive things. For
instance, while there are definitely some use cases for them, I need a
REALLY good reason to justify making a property/method private instead of
protected, or making a class final.
I am much the same.
As such, I think this would be better if it didn't throw a fatal error.
When you make it optional, however, I think you are left with something
that can be handled with an attribute just as well as a new keyword:
#[Expects('MyInterface')]
trait foo { }However, there might be value in generating a notice/warning, and I think
that would require a keyword, correct? (Not that up to speed on
annotations). Another option might be two support two new keywords:
requires and expects. The former would throw an error if the interface
isn't implemented while the latter will throw a warning/notice/nothing.Another option (and I haven't thought about this one enough to decide if I
like it) would be to have the expected interface automatically implemented
in the using class. This would allow the trait to be written under the
assumption it has access to the methods defined in the interface, and will
then throw an error if any of the methods are not implemented in the using
class:interface foo {
function a();
function b();
}trait bar expects foo {
function c(){
return $this->a() + $this->b();
}
}class baz {
use foo;
}The above would throw an error since a() and b() are never implemented and
baz is implementing the foo interface. You can currently get the same
behavior if you define a() and b() as abstract in the trait. However, this
doesn't give you the added benefit of utilizing the interface automatically
within the type system. The more I think about it, the less I like this
idea, since it doesn't require that much additional work to make the code
clearer by explicitly implementing the interface on the class if you want
it implemented. However, I'll go ahead and leave it here because it might
help generate some other ideas.
I... still don't see any use in this annotation.
Stepping back and ignoring the syntax for a moment, there's two different things here:
- "This trait expects to be used in a class that has these other methods on it".
- "This trait mostly/fully fulfills interface X, because it has methods a, b, and c."
For point 1, we already have that. It's called abstract methods in traits. This is a solved problem that requires no further resolution. At best it would be a shorthand to copying a few methods from an interface into the trait and sticking "abstract" in front of them. I really don't see a need for that.
For point 2, that's mainly useful as a way to signal to other developers "hey, this trait has all but one method of the LoggerInterface, that's how you'd use it", and to signal static analyzers and refactoring tools the same thing so that they can be auto-updated if you tweak the interface. I can see a use for point 2, and it would make my life a bit easier, but it's overall not high priority.
--Larry Garfield
On Wed, Jan 5, 2022 at 11:05 PM Larry Garfield larry@garfieldtech.com
wrote:
For point 2, that's mainly useful as a way to signal to other developers
"hey, this trait has all but one method of the LoggerInterface, that's how
you'd use it", and to signal static analyzers and refactoring tools the
same thing so that they can be auto-updated if you tweak the interface. I
can see a use for point 2, and it would make my life a bit easier, but it's
overall not high priority.
I think existing constructs provide 99% of this benefit anyway.
interface I { public function hello(); }
abstract class A implements I { abstract public function hello(); }
trait T { public function hello() { echo 'hello'; } }
class C extends A { use T; }
// or alternatively
class D implements I { use T; }
// or
abstract class E implements I { use T; }
class F extends E { ... }
$c = new C;
$c->hello(); // $c is instanceof I
$d = new D;
$d->hello(); // $d is instanceof I
$f = new F;
$f->hello(); // $f is instanceof I
It's not quite as neat as directly having a way of annotating T to say it
implements I, but it does the job insofar as your IDE and tools should be
able to immediately pick up a change in interface I is not fulfilled by
anything relying on trait T to be of type I, either directly or by
inheritance.
The remaining 1% of benefit could probably be achieved just by naming
convention:
interface LoggerInterface { ... }
trait LoggerInterface_FileLogger { ... }
But I still think both goals would be better achieved with something like:
interface LoggerInterface {
default public function error(string $message) {
$this->logger->log('error', $message);
}
}
in the manner of Java...no idea how easy or not it would be for someone a
little more experienced than me working on PHP core to do this though.
- David
--Larry Garfield
--
To unsubscribe, visit: https://www.php.net/unsub.php
On Wed, Jan 5, 2022 at 6:05 PM Larry Garfield larry@garfieldtech.com
wrote:
First, I'm someone that mainly uses traits to implement the functionality
defined in an interface. I think that's one of the best uses for them.
However, I'm personally not a huge fan of overly restrictive things. For
instance, while there are definitely some use cases for them, I need a
REALLY good reason to justify making a property/method private instead of
protected, or making a class final.I am much the same.
As such, I think this would be better if it didn't throw a fatal error.
When you make it optional, however, I think you are left with something
that can be handled with an attribute just as well as a new keyword:
#[Expects('MyInterface')]
trait foo { }However, there might be value in generating a notice/warning, and I think
that would require a keyword, correct? (Not that up to speed on
annotations). Another option might be two support two new keywords:
requires and expects. The former would throw an error if the interface
isn't implemented while the latter will throw a warning/notice/nothing.Another option (and I haven't thought about this one enough to decide if
I
like it) would be to have the expected interface automatically
implemented
in the using class. This would allow the trait to be written under the
assumption it has access to the methods defined in the interface, and
will
then throw an error if any of the methods are not implemented in the
using
class:interface foo {
function a();
function b();
}trait bar expects foo {
function c(){
return $this->a() + $this->b();
}
}class baz {
use foo;
}The above would throw an error since a() and b() are never implemented
and
baz is implementing the foo interface. You can currently get the same
behavior if you define a() and b() as abstract in the trait. However,
this
doesn't give you the added benefit of utilizing the interface
automatically
within the type system. The more I think about it, the less I like this
idea, since it doesn't require that much additional work to make the code
clearer by explicitly implementing the interface on the class if you want
it implemented. However, I'll go ahead and leave it here because it might
help generate some other ideas.I... still don't see any use in this annotation.
Stepping back and ignoring the syntax for a moment, there's two different
things here:
- "This trait expects to be used in a class that has these other methods
on it".- "This trait mostly/fully fulfills interface X, because it has methods
a, b, and c."For point 1, we already have that. It's called abstract methods in
traits. This is a solved problem that requires no further resolution. At
best it would be a shorthand to copying a few methods from an interface
into the trait and sticking "abstract" in front of them. I really don't
see a need for that.
Agreed. It would also allow IDEs and what not to determine the trait has
access to the interface methods that you didn't explicitly define as
abstract, but I still agree that it would just be shorthand that isn't
really needed.
For point 2, that's mainly useful as a way to signal to other developers
"hey, this trait has all but one method of the LoggerInterface, that's how
you'd use it", and to signal static analyzers and refactoring tools the
same thing so that they can be auto-updated if you tweak the interface. I
can see a use for point 2, and it would make my life a bit easier, but it's
overall not high priority.
I also agree.
I was focusing more on fleshing out the idea in general. I think the idea
is interesting enough that it's worth discussing even if it doesn't seem
like it's worth pursuing in its current form.
--Larry Garfield
--
To unsubscribe, visit: https://www.php.net/unsub.php
--
Chase Peeler
chasepeeler@gmail.com
For point 1, we already have that. It's called abstract methods in traits. This is a solved problem that requires no further resolution. At best
it would be a shorthand to copying a few methods from an interface into the trait and sticking "abstract" in front of them. I really don't see a need
for that.
You can say exactly the same about traits in general - you don't need traits at all, since you can copy method implementation into multiple classes,
in the same way as you're proposing to copy abstract methods declarations. But traits are implemented and they're useful - they reduce the amount of
code duplication, which means less work and a smaller surface for errors. It is the same case for this proposal - if you don't need to copy&paste
method declarations from an interface, that means less code duplication and possible errors. It should also make it easier for SCA tools to understand
the code since they no longer need to know the whole project to understand that method from trait is implementation of method from interface (which is
really tricky right now since it depends on context - trait can at the same implement and not implement the interface, because it depends on class
where it is used). And finally, you could use {@inheritdoc} in some meaningful way in traits - right now there is no straightforward way to inherit
phpdoc from the interface, and often this is what you want to do.
--
Regards,
Robert Korulczyk
It should also make it easier for SCA tools to understand
the code since they no longer need to know the whole project to understand that method from trait is implementation of method from interface (which is
really tricky right now since it depends on context - trait can at the same implement and not implement the interface, because it depends on class
where it is used).
Your other points make sense, but I don't think this one does - there are no implicit interfaces in PHP*, so all any tool cares about is:
- Does the class declaration say that it implements an interface?
- Does it actually contain the methods needed to do so, through any combination of direct implementation, inheritance, and trait usage?
Knowing that a particular trait could be used to implement a particular interface without further code doesn't really tell the tool anything - it still has to resolve the list of methods on the class itself, and test those against the "implements" clause. This is particularly true if the class uses the "as" and "insteadOf" clauses when including the trait, which nullify any promises the trait could make.
- other than "Stringable", whose purpose and implementation continue to baffle me
Regards,
--
Rowan Tommins
[IMSoP]
On Thu, Jan 6, 2022 at 11:20 AM Rowan Tommins rowan.collins@gmail.com
wrote:
On 6 January 2022 15:21:28 GMT, Robert Korulczyk robert@korulczyk.pl
wrote:It should also make it easier for SCA tools to understand
the code since they no longer need to know the whole project to
understand that method from trait is implementation of method from
interface (which is
really tricky right now since it depends on context - trait can at the
same implement and not implement the interface, because it depends on class
where it is used).Your other points make sense, but I don't think this one does - there are
no implicit interfaces in PHP*, so all any tool cares about is:
- Does the class declaration say that it implements an interface?
- Does it actually contain the methods needed to do so, through any
combination of direct implementation, inheritance, and trait usage?Knowing that a particular trait could be used to implement a particular
interface without further code doesn't really tell the tool anything - it
still has to resolve the list of methods on the class itself, and test
those against the "implements" clause. This is particularly true if the
class uses the "as" and "insteadOf" clauses when including the trait, which
nullify any promises the trait could make.I think the advantage would come within the trait. If I say the trait
expects an interface with two methods, then the tool can act as if the
interfaces methods exist in the trait even if they aren't explicitly
defined. As others have pointed out, though, you can get the same behavior
from declaring the methods not implemented in the trait as abstract.
- other than "Stringable", whose purpose and implementation continue to
baffle meRegards,
--
Rowan Tommins
[IMSoP]--
To unsubscribe, visit: https://www.php.net/unsub.php
--
Chase Peeler
chasepeeler@gmail.com
I think the advantage would come within the trait. If I say the trait
expects an interface with two methods, then the tool can act as if the
interfaces methods exist in the trait even if they aren't explicitly
defined. As others have pointed out, though, you can get the same behavior
from declaring the methods not implemented in the trait as abstract.
This is where I wonder if people are talking at cross purposes.
The only advantage I've understood of the proposal is to save on copy and paste of abstract methods from an interface into a trait, and everything else is just restating that in more complicated ways. So, yes, static analysers could use the information about requiring an interface in exactly the same way they can already use abstract method signatures, to validate the trait itself - but that wouldn't be any easier for the tool, and in fact would be extra logic for them to implement, so it seems an odd thing to bring up.
But some comments seem to be hinting at some separate advantage, to do with "correct usage" of the trait, which I haven't grasped. It's possible that the mention of static analysis relates to that in some way, and I'm just completely missing the point.
Regards,
--
Rowan Tommins
[IMSoP]
On Thu, Jan 6, 2022 at 7:12 PM Rowan Tommins rowan.collins@gmail.com
wrote:
But some comments seem to be hinting at some separate advantage, to do
with "correct usage" of the trait, which I haven't grasped. It's possible
that the mention of static analysis relates to that in some way, and I'm
just completely missing the point.
Yes, traits are a language construct that has in general more negative
implications than positive so it's good to keep an eye on their usage.
One of the okish usages is to define some implementation for an interface
that classes can use.
Limiting that trait to be used only by classes implementing the interface
is seen as a restriction placed on the trait that would not allow it to be
used in other places when it might have a negative impact.
Just as a note, on the composition vs inheritance line, I see traits
somewhere in the middle.
I'll always prefer to go for full composition, even if that means a bit
more boilerplate. The implementation can very well sit in a class and have
it as a dependency of the class that needs it.
Alex
Yes, traits are a language construct that has in general more negative
implications than positive so it's good to keep an eye on their usage.
One of the okish usages is to define some implementation for an interface
that classes can use.
Limiting that trait to be used only by classes implementing the interface
is seen as a restriction placed on the trait that would not allow it to be
used in other places when it might have a negative impact.
The examples I've generally seen of problematic trait use are more about what's in the trait itself, rather than where it's used. I can't really picture what would be the "negative impact" of using somebody else's well-designed trait (and if it's not well designed, it presumably wouldn't make use of the new syntax). Can you give any example scenarios that you think mandating an interface would help avoid?
Regards,
--
Rowan Tommins
[IMSoP]
On Thu, Jan 6, 2022 at 12:37 PM Alexandru Pătrănescu drealecs@gmail.com
wrote:
On Thu, Jan 6, 2022 at 7:12 PM Rowan Tommins rowan.collins@gmail.com
wrote:But some comments seem to be hinting at some separate advantage, to do
with "correct usage" of the trait, which I haven't grasped. It's possible
that the mention of static analysis relates to that in some way, and I'm
just completely missing the point.Yes, traits are a language construct that has in general more negative
implications than positive so it's good to keep an eye on their usage.
One of the okish usages is to define some implementation for an interface
that classes can use.
Limiting that trait to be used only by classes implementing the interface
is seen as a restriction placed on the trait that would not allow it to be
used in other places when it might have a negative impact.
And that's what I don't like about this. I'm totally OK with allowing
developers to do things the "wrong" way. It might end up that it's the
"right" way for their use-case.
Just as a note, on the composition vs inheritance line, I see traits
somewhere in the middle.
I'll always prefer to go for full composition, even if that means a bit
more boilerplate. The implementation can very well sit in a class and have
it as a dependency of the class that needs it.Alex
but that wouldn't be any easier for the tool, and in fact would be
extra logic for them to implement, so it seems an odd thing to bring up.
I think most people see the tools as black boxes to make THEIR lives
easier. They aren't concerned about whether it's added complexity to the
tool itself. If PhpStorm allows me to reference
$this->someMethodFromAnInterface() without defining
someMethodFromAnInterface() as an abstract method in the trait because I
include "expects <interface>", then that means things are "simpler" for me
as a developer. I don't really think this makes things that much simpler
though.
--
Chase Peeler
chasepeeler@gmail.com
I think most people see the tools as black boxes to make THEIR lives
easier. They aren't concerned about whether it's added complexity to the
tool itself.
Just to be clear, my comments were a response to an earlier message that explicitly talked about it being easier for the tools:
It should also make it easier for SCA tools to understand the code
I understand that that's not your opinion, and mostly agree with what you've said, but you're answering a different question.
Regards,
--
Rowan Tommins
[IMSoP]
Your other points make sense, but I don't think this one does - there are no implicit interfaces in PHP*, so all any tool cares about is:
- Does the class declaration say that it implements an interface?
- Does it actually contain the methods needed to do so, through any combination of direct implementation, inheritance, and trait usage?
Knowing that a particular trait could be used to implement a particular interface without further code doesn't really tell the tool anything - it still has to resolve the list of methods on the class itself, and test those against the "implements" clause.
You're talking about classes that use traits, but I'm talking about traits themselves. If I open a class in editor, there is a straightforward way to
check if specific method is implementation of an interface - check interfaces implemented by class and if one of them have this method, then this
method is implementation of this interface, and editor can properly mark it and handle navigation between implementation in class and declaration in
interface. So even if I have a big project, you need to only load a subset of classes used by this project in order to find methods that are
implementations of interfaces in this particular class.
If I open trait in my editor, it is a lot more complicated. Editor needs to scan the whole project, find all classes that use this trait, find all
interfaces that are implemented by these classes, and then find matches. And you may still end up with confusing results, because if you have class
A that implements FooInterface and uses FooTrait, and also class B that uses FooTrait, but NOT implements FooInterface, then if you ask if
FooTrait::someMethod() is implementation of FooInterface::someMethod(), the answer is "yes and no".
Also, I'm not sure how it works now, but about 2 years ago I got rid of most of traits in one project, because navigation between trait and interface
worked so badly in PhpStorm, that it was easier to deal with code duplication than broken "implements" detection. At the same time navigation between
classes and interfaces worked without any problem.
--
Regards,
Robert Korulczyk
You're talking about classes that use traits, but I'm talking about traits themselves. If I open a class in editor, there is a straightforward way to
check if specific method is implementation of an interface - check interfaces implemented by class and if one of them have this method, then this
method is implementation of this interface, and editor can properly mark it and handle navigation between implementation in class and declaration in
interface.
Ah, I see what you're getting at now. I've never particularly had the need to jump between an interface and a trait, but then I very rarely use traits at all.
In your use cases, are these traits generally using the interface methods, or implementing them? For instance, you might have a trait that requires the LoggerInterface because it makes use of its methods, so you want to jump to their documentation when viewing those uses; or you might have a trait that helps implement the LoggerInterface, and you want to link each method to the definition in the interface that it implements. Is one of these situations more common than the other, in your experience?
Regards,
--
Rowan Tommins
[IMSoP]
In your use cases, are these traits generally using the interface methods, or implementing them?
It is both. However "implementing" case is more problematic, since "using" can be quite reliably handled by adding assert($this instanceof LoggerInterface) or type hints in trait methods - SCA tools should handle this and PHP will complain if interface is not implemented. But there is no
easy way to say "FooTrait::someMethod() is implementation of FooInterface::someMethod()" that PHP and SCA will understand. And I think this
proposal handles this quite well, since it does not introduce implicit "implements" (IMO introducing another way of "injecting" interfaces to classes
will just hurt readability) - you still needs to add interface to "implements" section in class definition, but traits have more tools to define its
purpose and ensure that classes have proper definition.
--
Regards,
Robert Korulczyk
But there is no easy way to say "
FooTrait::someMethod()is
implementation ofFooInterface::someMethod()" that PHP and SCA will
understand. And I think this proposal handles this quite well
I'm not convinced it does, actually. Consider the following trait:
trait PropertyCount {
public function count(): int {
return count(get_object_vars($this));
}
}
This trait CAN be used to implement the built-in Countable interface,
and it might be useful to label it as such; but does it really make
sense to say that classes MUST implement that interface?
Even if we put it as a requirement, we can't guarantee that the class
will actually use the trait's implementation of the interface, because
this would still be valid:
class Foo implements Countable {
private $whatever;
use PropertyCount {
count as getPropertyCount;
}
public function count(): int {
return 0;
}
}
It feels like this use case would work better with an annotation like
/** @can-implement Countable */ since it is really just documentation
about possible uses.
Regards,
--
Rowan Tommins
[IMSoP]
Hi everyone. I've been lurking for a couple of days, but fwiw, I think this
RFC makes more sense as a built-in annotation.
Something like
#[partialImplemenation(Countable::class)]
trait PropertyCount {
public function count(): int {
return count(get_object_vars($this));
}
}
Adding new syntax for this doesn't seem like it would provide any useful
abilities or optimizations in the language (I could be wrong) but are more
useful to developers and tooling. A built-in annotation would allow tooling
to introspect the engineer's intentions as well as allow devs to not care,
if they want to and/or know what they're doing. I'd actually suggest two
annotations: expectsImplementation and partialImplementation.
expectsImplementation would mean that it expects those methods from the
given class/interface/trait to be present where it is used, while
partialImplementation indicates that it implements some (or all) of a child
interface/class.
Robert Landers
Software Engineer
Utrecht NL
On Fri, Jan 7, 2022 at 10:01 AM Rowan Tommins rowan.collins@gmail.com
wrote:
But there is no easy way to say "
FooTrait::someMethod()is
implementation ofFooInterface::someMethod()" that PHP and SCA will
understand. And I think this proposal handles this quite wellI'm not convinced it does, actually. Consider the following trait:
trait PropertyCount {
public functioncount(): int {
return count(get_object_vars($this));
}
}This trait CAN be used to implement the built-in Countable interface,
and it might be useful to label it as such; but does it really make
sense to say that classes MUST implement that interface?Even if we put it as a requirement, we can't guarantee that the class
will actually use the trait's implementation of the interface, because
this would still be valid:class Foo implements Countable {
private $whatever;use PropertyCount { count as getPropertyCount; } public function `count()`: int { return 0; }}
It feels like this use case would work better with an annotation like
/** @can-implement Countable */ since it is really just documentation
about possible uses.Regards,
--
Rowan Tommins
[IMSoP]--
To unsubscribe, visit: https://www.php.net/unsub.php
I'm not convinced it does, actually. Consider the following trait:
trait PropertyCount {
public functioncount(): int {
return count(get_object_vars($this));
}
}This trait CAN be used to implement the built-in Countable interface, and it might be useful to label it as such; but does it really make sense to say
that classes MUST implement that interface?
IMO yes, it does. This simplifies rules for detecting implementation of interfaces in traits, so you don't get "yes and no" answers in that case.
I'm not really sure why would you not want this - what is the point of marking method as interface implementation, if it is not reflected by type
system in actual execution? If it does not make sense in case of your PropertyCount, then do not use this feature.
Even if we put it as a requirement, we can't guarantee that the class will actually use the trait's implementation of the interface, because this
would still be valid:class Foo implements Countable {
private $whatever;use PropertyCount {
count as getPropertyCount;
}public function
count(): int {
return 0;
}
}
It works the same for classes and inheritance - if you declare Foo::count() directly in class, I can always create FooBar extends Foo that
overwrites your implementation. But I never heard that someone complain about it, so I'm not sure why this is suddenly a problem for traits. Ensuring
that particular implementation is used is not the purpose of interfaces, it never was.
Also note that right now method signatures from trait can be changed by class, so PHP won't complain about this:
trait T {
public function t(string $t): void {
}
}
class C {
use T {
t as tt;
}
public function t(): string {
return 't';
}
}
This proposal gives you tools to ensure that class does not mess up with method signatures expected by trait - you can create pairs of trait-interface
and require interface in trait.
--
Regards,
Robert Korulczyk
I'm not really sure why would you not want this - what is the point of
marking method as interface implementation, if it is not reflected by
type system in actual execution?
It's really quite simple: I don't want traits to tell me how I "must"
use them, but am quite happy for them to document how I "can" use them.
Unless we allow a trait to automatically implement the interface (or
similar features, such as an interface with default implementation)
there is no direct impact on the actual type system. There are, as far
as I can see, three things the feature would provide:
- Documentation: the trait can tell me as a programmer that it contains
everything I need to implement a particular interface - Code generation: the trait can import the list of methods from an
interface as abstract methods which it requires - Policing: the trait can force me as a programmer to use it in a
certain way; this is the part I don't see the need for
Regards,
--
Rowan Tommins
[IMSoP]
I'm not really sure why would you not want this - what is the point of marking method as interface implementation, if it is not reflected by type system in actual execution?
It's really quite simple: I don't want traits to tell me how I "must" use them, but am quite happy for them to document how I "can" use them.
About any of these someone could say "I don't want to be told..."
"...I must implement an abstract method"
"...I must implement all the methods in an interface"
"...I must pass all arguments declared in a function"
"...I must pass arguments that are of the type that were type-hinted"
"...I cannot extend a final class"
"...I cannot access a private property outside the class"
"...I cannot change a readonly property after it has been initialized"
And yet specifying appropriate constraints for a specific use-case has its benefits. I'm sure you can see benefit to the above constraints so it is strange to me you cannot see the benefit of traits that would constrain their uses to requiring an interface.
If it were possible to specify that a trait required an interface then I don't envision anyone other than those who need bespoke traits would do so, and those are not likely traits you would want to use anyway because they could be changed by the developer when their use-case evolved.
#jmtcw #fwiw
-Mike
Unless we allow a trait to automatically implement the interface (or similar features, such as an interface with default implementation) there is no direct impact on the actual type system. There are, as far as I can see, three things the feature would provide:
- Documentation: the trait can tell me as a programmer that it contains everything I need to implement a particular interface
- Code generation: the trait can import the list of methods from an interface as abstract methods which it requires
- Policing: the trait can force me as a programmer to use it in a certain way; this is the part I don't see the need for
Regards,
--
Rowan Tommins
[IMSoP]--
To unsubscribe, visit: https://www.php.net/unsub.php
"...I must implement an abstract method" "...I must implement all the methods in an interface" "...I must pass all arguments declared in a function" "...I must pass arguments that are of the type that were type-hinted"
In all of these cases, failing to meet the requirements means the code
probably won't work correctly.
As I've said previously, there are legitimate cases where a trait uses
the methods from an interface, which is a similar scenario. That use
case is currently covered by including abstract methods in the trait,
and while "requires interface" could be a short-hand for that, it's been
made clear to me that that is not the use case people are talking about.
"...I cannot extend a final class" "...I cannot access a private property outside the class" "...I cannot change a readonly property after it has been initialized"
These are more relevant comparisons, because there could be valid uses
of the class which violate the restrictions; and particularly "final" is
often used out of principle rather than specific need. However, they do
allow the author of the class to make certain assumptions about how it
will behave - for example, if a property is private, you know exactly
which lines need changing if you want to rename it; if a class is final,
other code referencing it can make stronger assumptions than its method
signatures; and so on.
The difference with the "trait requires interface" proposal is that I
don't understand what advantage it gives the author of the trait. What
decisions can you make as a library developer because you've said "users
of this trait must implement the matching interface" as opposed to "...
can implement the matching interface"?
It's possible there is some advantage I'm missing, but so far nobody
seems to have presented it.
Regards,
--
Rowan Tommins
[IMSoP]
On Tue, Jan 18, 2022 at 11:13 AM Rowan Tommins rowan.collins@gmail.com
wrote:
The difference with the "trait requires interface" proposal is that I
don't understand what advantage it gives the author of the trait. What
decisions can you make as a library developer because you've said "users
of this trait must implement the matching interface" as opposed to "...
can implement the matching interface"?It's possible there is some advantage I'm missing, but so far nobody
seems to have presented it.
Well, the trait doesn't necessarily have to fulfill the entire interface,
first of all. As you mentioned, this can be worked around using abstracts
in the trait. However, what if you're dealing with return values within the
trait?
Suppose I have something like this:
trait ArithmeticTrait {
public function add(float $val): NumberInterface {
// Do math
return $this;
}
public function addmul(float $val): NumberInterface {
$added = $this->add($val);
if ($added->isPositive()) {
// Do stuff
}
return $this;
}
}
In this situation, the return value of the trait requires that $this
implements the NumberInterface, however there is no way for the trait
itself to enforce this.
Jordan
In this situation, the return value of the trait requires that $this
implements the NumberInterface, however there is no way for the trait
itself to enforce this.
Fair enough, that's a reasonable use case, thank you. I could quibble
and say that "self" or "static" would probably work in that particular
example, but I suspect that would just lead to an unproductive spiral of
example and counter-example.
I'm not yet convinced that this use case is frequent enough to pass the
high bar of adding a new language feature, but it at least reassures me
that this isn't just control for control's sake.
Regards,
--
Rowan Tommins
[IMSoP]
On Tue, Jan 18, 2022 at 1:07 PM Rowan Tommins rowan.collins@gmail.com
wrote:
Fair enough, that's a reasonable use case, thank you. I could quibble
and say that "self" or "static" would probably work in that particular
example, but I suspect that would just lead to an unproductive spiral of
example and counter-example.
Not even a separate counter example actually. Static analysis, including in
IDEs, will mis-type the return value as being the trait itself within the
trait. So for instance, in my example I used the add() method from the
trait in a separate method within the trait. If I return "self" or
"static", my IDE will think that within the trait it is returning itself,
even though traits can't be directly instantiated. (Or more accurately, it
won't be able to understand what other types it may satisfy.)
This is something I've had to do some real design gymnastics before to get
around. I encountered this in a situation where I needed to check between a
ComplexNumberInterface and a NumberInterface in the math library I
maintain, since they behave differently for the same operations (which was
an example of my motivation for operator overloads).
Jordan
On Tue, Jan 18, 2022 at 3:55 PM Jordan LeDoux jordan.ledoux@gmail.com
wrote:
On Tue, Jan 18, 2022 at 11:13 AM Rowan Tommins rowan.collins@gmail.com
wrote:The difference with the "trait requires interface" proposal is that I
don't understand what advantage it gives the author of the trait. What
decisions can you make as a library developer because you've said "users
of this trait must implement the matching interface" as opposed to "...
can implement the matching interface"?It's possible there is some advantage I'm missing, but so far nobody
seems to have presented it.Well, the trait doesn't necessarily have to fulfill the entire interface,
first of all. As you mentioned, this can be worked around using abstracts
in the trait. However, what if you're dealing with return values within the
trait?Suppose I have something like this:
trait ArithmeticTrait {
public function add(float $val): NumberInterface {
// Do mathreturn $this; } public function addmul(float $val): NumberInterface { $added = $this->add($val); if ($added->isPositive()) { // Do stuff } return $this; }}
This can still be handled with abstract methods
trait ArithmeticTrait {
public function add(float $val): NumberInterface {
$n = $this->getNumberInterface();
// Do math
return $n;
}
public function addmul(float $val): NumberInterface {
$n = $this->getNumberInterface();
$added = $this->add($val);
if ($added->isPositive()) {
// Do stuff
}
return $n;
}
abstract protected function getNumberInterface(): NumberInterface;
}
class Foo {
use ArithmeticTrait;
protected NumberInterface $n;
protected function getNumberInterface() : NumberInterface {
return $this->n;
}
}
In this situation, the return value of the trait requires that $this
implements the NumberInterface, however there is no way for the trait
itself to enforce this.Jordan
--
Chase Peeler
chasepeeler@gmail.com
This can still be handled with abstract methods
While it is technically possible (I also came up with architecture hacks),
it is... not a good solution.
It is technically possible for the __toString() method to increment one of
the properties on an object, that doesn't mean it's a good way to do it. It
is technically possible for the __set() method to mutate a database, but
that doesn't mean it's a good way to do it.
Jordan