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

Why "Class needs to implement __isset to properly work here"? #912

Closed
Revisor opened this issue May 8, 2018 · 7 comments
Closed

Why "Class needs to implement __isset to properly work here"? #912

Revisor opened this issue May 8, 2018 · 7 comments

Comments

@Revisor
Copy link

Revisor commented May 8, 2018

Hello,
I haven't found anything about this inspection in the docs or in the issues.

Can you please explain, why

if (isset($object->property))

shows the following error?

Class needs to implement __isset to properly work here

Why does the $object's class need to implement __isset()?

@kiler129
Copy link

kiler129 commented May 9, 2018

If the property doesn’t exists in the class and it’s accessed via magic getter the only way for interpreter to know if the property exists is to call __isset.

@kalessil
Copy link
Owner

kalessil commented May 9, 2018

As @kiler129 says, if a property does not exists or the property is private/property the __isset is needed.

It's also possible that the codebase creating properties dynamically, like e.g. stdClass used as DTO (Data Transfer objects). Here we can not really fix this and just recommending using less dynamics and write regular DTO instead (they have all fields declared implicitely).

@kalessil kalessil closed this as completed May 9, 2018
@kalessil kalessil self-assigned this May 9, 2018
@kalessil kalessil added this to the C-3.0.2 milestone May 9, 2018
@Revisor
Copy link
Author

Revisor commented May 9, 2018

So if I understand correctly, the condition

if (isset($object->property))

is a problem if

  1. The class implements __get() but not __isset()
  2. Or the property is private.

On the other hand it is not a problem if the property is public (or undeclared) and the class doesn't implement __get().

Is that correct?

@kalessil
Copy link
Owner

kalessil commented May 9, 2018

  • The class implements __get() but not __isset(): Correct

  • Or the property is private: Correct (same applies to protected properties)

  • Or the property is private: correct

  • class doesn't implement __get(): correct, but in general if you are using any of __get/__set/__isset , you are expected to have all 3 of them (one of our inspections suppose to complain if something is missing).

@Revisor
Copy link
Author

Revisor commented May 9, 2018

Thank you for the explanation, now I understand the inspection and the problem it points to.

@kalessil
Copy link
Owner

kalessil commented May 9, 2018

You are welcome. I'll reopen the issue in order to cover the topic in our documentation as well.

@kalessil
Copy link
Owner

Done: https://github.com/kalessil/phpinspectionsea/blob/master/docs/probable-bugs.md#emptyisset-results-correctness

@Revisor @kiler129 : feedback on the added docs is appreciated.

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

3 participants