Skip to content

Conversation

@nicosp
Copy link

@nicosp nicosp commented Dec 16, 2025

Allows object to use the __get magic function to fetch attributes.

This is very convenient when using Entities or other objects.

Ideally we can also use __isset to be more precise with defined checks but I am not sure.

@stof
Copy link
Member

stof commented Dec 16, 2025

Adding this without any tests is a no-go to me.

And AFAIK, we already support dynamic properties (but it relies on __isset to be implemented properly, not just to implement __get).

@nicosp
Copy link
Author

nicosp commented Dec 17, 2025

@stof I wanted to make sure the implementation is finalized before doing any tests.

The issue with __isset is that there is no difference between a null value and a "not defined" value. Would you consider adding an interface just for this? ie.

interface TwigDynamicAttributes (or a better name) {
  public function hasAttribute(string $name): bool;
}

This can be easily added to entities or other objects with dynamic properties.

@fabpot
Copy link
Contributor

fabpot commented Dec 17, 2025

IIRC, this has been proposed several times in the past and using __isset() was the reason why we rejected the previous attempts.

@nicosp
Copy link
Author

nicosp commented Dec 17, 2025

@fabpot Unfortunately __isset does not really distinguish between null and undefined, forcing us to use null coalescing and therefore defeating the reason we use strict variables in the first place.

@stof
Copy link
Member

stof commented Dec 17, 2025

@nicosp it depends how you implement the method actually.

@nicosp
Copy link
Author

nicosp commented Dec 17, 2025

@stof If you implement it correctly (per PHP spec) you will have to consider nulls as not set. I thought about using ArrayAccess::offsetExists but even then you have to follow this. See: https://bugs.php.net/bug.php?id=41727

Which is why I suggested the interface which would allow the implementation to be correct per PHP spec and still have Twig strict variables work properly.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants