Re: [PHP-DEV] RFC: Trait expects interface

2022-01-19 Thread Jordan LeDoux
On Wed, Jan 19, 2022 at 7:07 AM Chase Peeler  wrote:

>
> 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


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-19 Thread Chase Peeler
On Tue, Jan 18, 2022 at 3:55 PM Jordan LeDoux 
wrote:

> On Tue, Jan 18, 2022 at 11:13 AM Rowan Tommins 
> 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;
> }
> }
>

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
chasepee...@gmail.com


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-18 Thread Jordan LeDoux
On Tue, Jan 18, 2022 at 1:07 PM Rowan Tommins 
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


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-18 Thread Rowan Tommins

On 18/01/2022 20:55, Jordan LeDoux wrote:
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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-18 Thread Jordan LeDoux
On Tue, Jan 18, 2022 at 11:13 AM Rowan Tommins 
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


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-18 Thread Rowan Tommins

On 17/01/2022 18:03, Mike Schinkel wrote:


"...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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-17 Thread Mike Schinkel


> On Jan 7, 2022, at 1:56 PM, Rowan Tommins  wrote:
> 
> On 07/01/2022 13:47, Robert Korulczyk wrote:
>> 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]
> 
> -- 
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
> 



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-07 Thread Rowan Tommins

On 07/01/2022 13:47, Robert Korulczyk wrote:
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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-07 Thread Robert Korulczyk

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?


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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-07 Thread Robert Landers
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 
wrote:

> On 06/01/2022 23:53, Robert Korulczyk wrote:
> > 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
>
>
> 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]
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-07 Thread Rowan Tommins

On 06/01/2022 23:53, Robert Korulczyk wrote:
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



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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Robert Korulczyk




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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Rowan Tommins
On 6 January 2022 19:51:46 GMT, Robert Korulczyk  wrote:
>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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Robert Korulczyk

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:

1) Does the class declaration say that it implements an interface?
2) 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Rowan Tommins
On 6 January 2022 18:03:04 GMT, Chase Peeler  wrote:
>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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Chase Peeler
On Thu, Jan 6, 2022 at 12:37 PM Alexandru Pătrănescu 
wrote:

> On Thu, Jan 6, 2022 at 7:12 PM Rowan Tommins 
> 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 ", 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
chasepee...@gmail.com


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Rowan Tommins
On 6 January 2022 17:36:41 GMT, "Alexandru Pătrănescu"  
wrote:
>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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Alexandru Pătrănescu
On Thu, Jan 6, 2022 at 7:12 PM Rowan Tommins 
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


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Rowan Tommins
On 6 January 2022 16:25:04 GMT, Chase Peeler  wrote:
> 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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Chase Peeler
On Thu, Jan 6, 2022 at 11:20 AM Rowan Tommins 
wrote:

> On 6 January 2022 15:21:28 GMT, Robert Korulczyk 
> 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:
>
> 1) Does the class declaration say that it implements an interface?
> 2) 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 me
>
> Regards,
>
> --
> Rowan Tommins
> [IMSoP]
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

-- 
Chase Peeler
chasepee...@gmail.com


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Rowan Tommins
On 6 January 2022 15:21:28 GMT, Robert Korulczyk  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:

1) Does the class declaration say that it implements an interface?
2) 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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Robert Korulczyk
> 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 
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-06 Thread Chase Peeler
On Wed, Jan 5, 2022 at 6:05 PM Larry Garfield 
wrote:

> On Wed, Jan 5, 2022, at 2:35 PM, Chase Peeler 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:
>
> 1. "This trait expects to be used in a class that has these other methods
> on it".
> 2. "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
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>

-- 
Chase Peeler
chasepee...@gmail.com


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-05 Thread David Gebler
On Wed, Jan 5, 2022 at 11:05 PM Larry Garfield 
wrote:

> On Wed, Jan 5, 2022, at 2:35 PM, Chase Peeler 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
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: https://www.php.net/unsub.php
>
>


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-05 Thread Larry Garfield
On Wed, Jan 5, 2022, at 2:35 PM, Chase Peeler 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:

1. "This trait expects to be used in a class that has these other methods on 
it".
2. "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

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-05 Thread Chase Peeler
On Wed, Jan 5, 2022 at 2:17 PM David Gebler  wrote:

> On Tue, Jan 4, 2022 at 10:35 PM Kirill Nesmeyanov  wrote:
>
> > 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
chasepee...@gmail.com


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-05 Thread David Gebler
On Tue, Jan 4, 2022 at 10:35 PM Kirill Nesmeyanov  wrote:

> 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


Re: [PHP-DEV] RFC: Trait expects interface

2022-01-05 Thread Rowan Tommins

On 04/01/2022 22:35, Kirill Nesmeyanov wrote:

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.



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"?



On 04/01/2022 23:10, Bruce Weirdan wrote:

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]

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-04 Thread Kirill Nesmeyanov
>Среда, 5 января 2022, 4:17 +03:00 от Larry Garfield :
> 
>On Tue, Jan 4, 2022, at 4:35 PM, Kirill Nesmeyanov wrote:
>> 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?
>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
>
>--
>PHP Internals - PHP Runtime Development Mailing List
>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

 

Re: [PHP-DEV] RFC: Trait expects interface

2022-01-04 Thread Larry Garfield
On Tue, Jan 4, 2022, at 4:35 PM, Kirill Nesmeyanov wrote:
> 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?

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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-04 Thread Dusk
On Jan 4, 2022, at 14:35, Kirill Nesmeyanov  wrote:
> 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.

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?)
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php



Re: [PHP-DEV] RFC: Trait expects interface

2022-01-04 Thread Bruce Weirdan
On Wed, Jan 5, 2022 at 12:36 AM Kirill Nesmeyanov  wrote:
> 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.

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:weir...@gmail.com

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php