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

Tolerate $this usage in anonymous functions #543

Closed
wants to merge 3 commits into from
Closed

Tolerate $this usage in anonymous functions #543

wants to merge 3 commits into from

Conversation

alWerewolf
Copy link

@alWerewolf alWerewolf commented Nov 30, 2017

Alternative solution for issue #536.
For $this var usage it gives warning

$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.

And hints developer to check class of $this before usage.
Class check can be added in form

if ($this instanceof [class])" {
    /*use $this*/
}

Note: if class check added no type checking is done in language server it just hides the warning.
There is a leaf in code marked by comment:

//else we should warn here that invoker can bind $this to object of any class. Should we?

Question in comment is for community.

Alternative solution for issue 536.
Gives warning "$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check." for $this var usage.
And hints developer to check class of $this before usage.
Class check can be added in form "if ($this instanceof [class])" {/*use $this*/}.
Note: if class check added no type checking is done its in language server it just hides the warning.
@felixfbecker
Copy link
Owner

I've never seen anyone use if $this instanceof so I don't think anyone would actually use that. I don't think it belongs into the language server.

@alWerewolf
Copy link
Author

$this can be bound to ANY object by caller of Closure(anonymous function) so we must warn about it.
It's all about developer must know for sure how his function is called and which object is bound.
Class check for $this var must ensure expected results.
Anyone can suggest their way to do class check. I think there are many ways.
I've choosen $this instanceof because of simplicity and it's favor vs deprecated is_a.
I thought of example where you pass your anonymous function to "blackbox".
You never know for sure how it's called so checking $this var will help from getting into trouble.

@felixfbecker
Copy link
Owner

felixfbecker commented Dec 3, 2017

I decided for disabling the warnings in these cases instead. This is similar to what e.g. TypeScript does - this is any by default, so no checks. Only in cases where it is known to be a certain type (or type hinted) should we do warnings. In the future, we can give the user the option to type hint $this, but until then we should force the user to write code to circumvent the warning.

Thank you for your PR!

@alWerewolf alWerewolf deleted the tolerate-this-usage branch February 18, 2018 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants