Re: Introduce the abiltiy to use the first-call-callable syntax on non-static methods, statically

On Mon, Jan 23, 2023 at 4:39 PM Ollie Read <php@ollie.codes> wrote: > > You are absolutely correct. I guess the solution would be to handle it differently in this > case. > > Creating a closure from a static method would be fine, as it creates a static closure, but when > attempting to create a static closure from a non-static method, it would instead return a closure > that errors if it isn't bound to an appropriate object. You'd most likely want to restrict > this to public methods only, which would help with the security issues. > > There's already a check there that throws an error, so we can already tell the difference > there, but the tricky part will be in the returned closure.. Perhaps something like > "BindingClosure" that throws the static error when attempting to call it unbound, or > better yet, a more descriptive error about it requiring binding. > > Would that be feasible? > > On Sun, Jan 22, 2023, at 8:36 PM, Larry Garfield wrote: > > On Sun, Jan 22, 2023, at 11:45 AM, Ollie Read wrote: > > > Hello all, > > > > > > I've created a feature request issue on GitHub (here: > > > https://github.com/php/php-src/issues/10414), but I > > > have been advised > > > that it's best to post here. > > > > > > What I would like to introduce/suggest, is the ability to create a > > > closure from a method using the first-class-callable syntax (eg: > > > MyClass::aMethod(...)), for a non-static method, statically. > > > > > > Currently, the following code causes an error. > > > > > > ``` > > > class Test { > > > public function test(): string { return 'test'; } > > > } > > > > > > $closure = Test::test(...); > > > ``` > > > > > > I understand why the error is thrown, but, and I'm unsure of the > > > specifics regarding this, I think we could delay the error until the > > > closure was called. The reason for this, is that closures can be bound, > > > so if you followed on from the code above, you could do the following: > > > > > > ``` > > > $closure->bindTo(new Test); > > > $closure(); > > > ``` > > > > > > The above would bind the closure in $closure to the scope of an object, > > > which in this case, is the class that the method belongs to. > > > > > > The best example I can think, for this, would be when filter a > > > collection of instances. If you were using a collection library, you > > > would currently have something like the following: > > > > > > ``` > > > $collection->filter(function (Str $string) { > > > return !$string->empty(); > > > }); > > > ``` > > > > > > Whereas it would be much nicer to have the following: > > > > > > ``` > > > $collection->filter(Str::empty(...)); > > > ``` > > > > > > In this situation, the collection library would be responsible for > > > binding the closure to the value it is iterating. > > > > So you'd implement this yourself elsewhere? > > > > class Str { > > public function empty(): bool { ... } > > } > > > > I don't see in this example how this is any better than what is already currently > > possible: > > > > class Str { > > public static function empty(Str $s): bool { ... } > > } > > > > $collection->filter(Str::empty(...)); > > > > > I have limited experience with PHPs source, and C in general, but my > > > understanding would be that if we were creating a closure, we would > > > skip the check for the static method. The code responsible for handling > > > the closure call would most require some additional functionality to > > > check if it was bound to a valid instance, returning an error if it > > > isn't, and then returning an error if it isn't bound at all and the > > > method isn't static. > > > > > > The more I think about it, the more I think this may require a new type > > > of Closure, or at least a runtime applied interface, to help developers > > > determine whether a closure was created using first-class-callable > > > syntax. > > > > This is, I think, the important part here, and would be a prerequisite. Right now > > there's no way (as far as I know) to differentiate a closure that is callable from one that > > would be callable if it were bound to an object. That's generally not a huge deal in practice > > as unbound closures are not often used, but what you're suggesting would make them much more > > likely. Also, a static closure cannot be bound, so you cannot just blindly bind whatever callable > > you're passed to $this, in your example. (Besides, blindly binding a closure to $this sounds > > like a great security hole.) > > > > So for some variant of this to work, I think you'd first need to think through how to > > (easily and without dipping into reflection) determine if a closure object is bindable (static or > > not) and if it's already bound. Once that's figured out, then we can see what, if any, > > short-hand way to make a not-yet-bound closure makes sense. (Which could be FCC syntax or not, I > > don't know.) > > > > --Larry Garfield > > > > -- > > PHP Internals - PHP Runtime Development Mailing List > > To unsubscribe, visit: https://www.php.net/unsub.php > > > > > > --- > Best Regards, > *Ollie Read* FWIW, I think we can "throw out" any automatic binding, that just complicates things. In my mind, Test::Func(...) should be treated the same as ['Test', 'Func'] or 'Test::Func' until it is called and if some fancy framework wants to do something special with the closure, it can do so. FWIW, I didn't even know this syntax checked anything until today. I can think of a number of cases where delaying the error to actual execution is beneficial. 1. Just because the method/object doesn't exist, doesn't mean it won't exist by the time it is called -- this is PHP after all. 2. I can have a file with 100 million of these things without triggering any autoloading. 3. PHPStorm currently doesn't trigger this syntax as an error.