Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The functionality of the __callStatic magic method is incomplete. #13813

Open
daddyofsky opened this issue Mar 26, 2024 · 15 comments
Open

The functionality of the __callStatic magic method is incomplete. #13813

daddyofsky opened this issue Mar 26, 2024 · 15 comments

Comments

@daddyofsky
Copy link

Description

The following code:

<?php
class MyClass
{
	public static function __callStatic($method, $args)
	{
		echo $method . "\n";
	}

	private function privateMethod() {}
	protected function protectedMethod() {}
	public function publicMethod() {}
}

MyClass::privateMethod();
MyClass::protectedMethod();
MyClass::publicMethod();

Resulted in this output:

privateMethod
protectedMethod
Fatal error: Uncaught Error: Non-static method MyClass::publicMethod() cannot be called statically in ...

But I expected this output instead:

privateMethod
protectedMethod
publicMethod

It seems that the __callStatic magic method is discriminating against public methods. ;)

PHP Version

PHP 8.x

Operating System

Any

@MorganLOCode
Copy link

__callStatic is used when inaccessible methods are invoked. MyClass::publicMethod() is public, hence accessible globally. __callStatic isn't being invoked.

@iluuu1994
Copy link
Member

Right. This is consistent with the fact that two functions (static and non-static) cannot coexist, if declared directly.

@daddyofsky
Copy link
Author

daddyofsky commented Mar 26, 2024

__callStatic is used when inaccessible methods are invoked. MyClass::publicMethod() is public, hence accessible globally. __callStatic isn't being invoked.

Not correct, please check manual.
__callStatic() is triggered when invoking inaccessible methods in a static context.

All dynamic methods are inaccessible in a static context.

@daddyofsky
Copy link
Author

daddyofsky commented Mar 26, 2024

Right. This is consistent with the fact that two functions (static and non-static) cannot coexist, if declared directly.

In static context, public methods are inaccessible.
This is not a problem with the function declaration, but rather an issue of whether it can be used statically or not. __callStatic is intended for use when calling methods that cannot be used statically, and this should be applied to public methods as well.

@iluuu1994
Copy link
Member

That's not how it currently works. Static and instance methods share the same symbol table. You can even call static methods as instance methods. As such, both -> and :: look up the method in the same symbol table. If found, and visibility checks succeed, the method is called (including the static-ness check). Otherwise, __call or __callStatic is called.

Your interpretation is completely reasonable, but not how it works. PHP is an implementation specified language, and any such change would require an RFC. See https://wiki.php.net/rfc/howto on how this process works.

@damianwadley
Copy link
Member

Reminder that :: is not a "static method call operator". It is the scope resolution operator, as in it resolves what name::name means according to the current scope.

MyClass::publicMethod resolves to "the publicMethod of MyClass", which exists and is accessible. And because it exists and is accessible, __call/__callStatic will not get involved. The call is static not because of the :: but because there is no instance of MyClass in the scope to use; if there was one then it would be used and the call would not be static.
https://3v4l.org/4JV2B

@daddyofsky
Copy link
Author

Reminder that :: is not a "static method call operator". It is the scope resolution operator, as in it resolves what name::name means according to the current scope.

MyClass::publicMethod resolves to "the publicMethod of MyClass", which exists and is accessible. And because it exists and is accessible, __call/__callStatic will not get involved. The call is static not because of the :: but because there is no instance of MyClass in the scope to use; if there was one then it would be used and the call would not be static. https://3v4l.org/4JV2B

I understand the explanation, but I believe the outcome should be the same. When a static call is made in a static context without an instance, it should invoke the __callStatic magic method if the corresponding method is not static. This would be the general expectation of those using magic methods.

@damianwadley
Copy link
Member

I understand the explanation, but I believe the outcome should be the same.

And that's quite reasonable. Dare I say it even makes sense, especially if you look at how other languages work (even though there are significant differences between what compiled and interpreted languages can do...). But it's not compatible with the philosophy of how PHP currently does methods and method calls, and changing that philosophy to fit this pattern could be a huge break to anyone relying on the current behaviors.

Which is why iluuu1994 marked this as Requires RFC: we're not saying "we cannot and will not do this" and most certainly not "you are wrong and this is bad", but instead we're saying "changing this is a big and complicated thing to talk about, and these GitHub Issues are just not a good enough medium for us to give it the conversation it deserves". Like, the first step in the RFC process is actually to (re)start the conversation on the internals mailing list - that being where all the super cool dev people hang out...

@daddyofsky
Copy link
Author

And that's quite reasonable. Dare I say it even makes sense, ...

Thank you for your kind response. I will prepare well and raise my voice again through the RFC process.

@iluuu1994 iluuu1994 added Feature and removed Bug labels Mar 27, 2024
@brzuchal
Copy link
Contributor

@daddyofsky just make your function static when they don't use class object instance by $this your problem is solved. We definitely should not change the behavior like you'd like to cause it can lead only to confusion and shade the code.
The current implementation works as expected, The outer scope has no private and protected method, and the public method exists so it is clear that no magic should run before the method is truly invoked, the fact that it is an error at runtime is caused by mixed expectations. Expecting the object method to be evaluated as a class method is conceptually wrong.

@daddyofsky
Copy link
Author

daddyofsky commented Mar 28, 2024

@brzuchal If there were no __callStatic magic method, of course, I would have used it statically.

Do you find it strange to use it like MyClass::privateMethod()? You might not like it, but you would think it's possible because there is __callStatic. I think the same. With __callStatic existing, why can't it work for public methods? That's what I'm questioning.

Moreover, using MyClass::privateMethod() like this doesn't actually employ it statically. Inside __callStatic, it creates a singleton or an instance of another class. Yes, that's correct. I want to use public methods in the same way, like Laravel's ORM.

@brzuchal
Copy link
Contributor

@daddyofsky

Do you find it strange to use it like MyClass::privateMethod()? You might not like it, but you would think it's possible because there is __callStatic.

I don't share your opinion on it. I think differently, the "privateMethod" is meant to be called from the inner scope of the class and nowhere else, so it doesn't exist for the outer - public scope. Seems logical that __callStatic is applied here.

I think the same. With __callStatic existing, why can't it work for public methods? That's what I'm questioning.

Because __callStatic was designed to be called whenever the method doesn't exist in callee scope and this is a clearly defined behavior since always.

@daddyofsky
Copy link
Author

@brzuchal

What you pointed out is correct. In the current behavior of __callStatic, it is as you say. However, there is one aspect that is being overlooked. That is, it's __callStatic not __call. This means that in the static scope, even public methods can be considered non-existent. I am interested in this aspect, and I think that with a slight change to the design of __callStatic, it could become an even more powerful magic method.

Perhaps the difference in opinions arose because the issue was initially registered as a bug. If that's the case, I apologize. This issue has already been changed from a bug to a feature. And we are discussing opinions for writing an RFC to change the functionality through the PHP mailing list. I'm not sure what the outcome will be, but I plan to do my best.

Thank you for your interest.

@brzuchal
Copy link
Contributor

brzuchal commented Mar 28, 2024

@daddyofsky

That is, it's __callStatic not __call. This means that in the static scope, even public methods can be considered non-existent.

AFAIK there is no such thing as "static scope", but if you find it in the PHP Manual I'd be happy to read about it.

Perhaps the difference in opinions arose because the issue was initially registered as a bug. If that's the case, I apologize. This issue has already been changed from a bug to a feature. And we are discussing opinions for writing an RFC to change the functionality through the PHP mailing list. I'm not sure what the outcome will be, but I plan to do my best.

No worries, I didn't consider this as a bug at all. I understand your reasoning, I only don't consider it correct.
IMO you mix different terms like method visibility and variable scope but they're two different things.

@daddyofsky
Copy link
Author

@brzuchal

__call() is triggered when invoking inaccessible methods in an object context.

__callStatic() is triggered when invoking inaccessible methods in a static context.

The manual uses the terms "object context" and "static context." Your mention of "variable scope" corresponds to "object context," and my mention of "static scope" refers to what is meant by "static context."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants