Skip to content

Guard against missing twig_callable attribute#531

Merged
bocharsky-bw merged 3 commits into
php-translation:masterfrom
kevinvergauwen:fix-missing-twig-callable-attribute
May 11, 2026
Merged

Guard against missing twig_callable attribute#531
bocharsky-bw merged 3 commits into
php-translation:masterfrom
kevinvergauwen:fix-missing-twig-callable-attribute

Conversation

@kevinvergauwen
Copy link
Copy Markdown
Contributor

PR added with a minimal compatibility fix for custom FilterExpression nodes that do not expose the "twig_callable" attribute.

The changes only add guard checks before accessing the attribute, without changing the existing behavior for regular Twig filter expressions.

Copy link
Copy Markdown
Member

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Could you please fix code styles here?

}

if (!$transNode instanceof FilterExpression) {
if (!$transNode instanceof FilterExpression || !$transNode->hasAttribute('twig_callable')) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be clear, do you think it's OK to throw this exception if we have a FilterExpression instance, but it does not have twig_callable attribute on it?

Copy link
Copy Markdown
Contributor Author

@kevinvergauwen kevinvergauwen May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style issues fixed. The reported violations were related to missing : void return types in existing code.

Good point regarding the additional guard logic.

The original exception behavior is now preserved for actual invalid desc chains, while custom FilterExpression implementations without the twig_callable attribute are skipped by returning the original node.

Copy link
Copy Markdown
Member

@bocharsky-bw bocharsky-bw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

@bocharsky-bw bocharsky-bw merged commit 93be2a0 into php-translation:master May 11, 2026
8 checks passed
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.

2 participants