[ExpressionLanguage] Fix coalescing operator deep nested properties #47446
Conversation
…more than two levels.
|
Hey! I think @mytuny has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
| @@ -69,6 +69,10 @@ public function evaluate(array $functions, array $values) | |||
| { | |||
| switch ($this->attributes['type']) { | |||
| case self::PROPERTY_CALL: | |||
| if ($this->nodes['node'] instanceof GetAttrNode) { | |||
| $this->nodes['node']->attributes['is_null_coalesce'] = $this->attributes['is_null_coalesce']; | |||
There was a problem hiding this comment.
shouldn't this be $this->nodes['node']->attributes['is_null_coalesce'] = $this->nodes['node']->attributes['is_null_coalesce'] || $this->attributes['is_null_coalesce']; ?
There was a problem hiding this comment.
shouldn't this be
$this->nodes['node']->attributes['is_null_coalesce'] = $this->nodes['node']->attributes['is_null_coalesce'] || $this->attributes['is_null_coalesce'];?
@stof This will introduce a regression when we have a mixed chaining between different types of nodes (not only GetAttrNode).
There was a problem hiding this comment.
| $this->nodes['node']->attributes['is_null_coalesce'] = $this->attributes['is_null_coalesce']; | |
| $this->nodes['node']->attributes['is_null_coalesce'] = $this->attributes['is_null_coalesce'] ?? false; |
|
I looked at this a bit. diff --git a/src/Symfony/Component/ExpressionLanguage/Node/NullCoalesceNode.php b/src/Symfony/Component/ExpressionLanguage/Node/NullCoalesceNode.php
index 1cc5eb058e..c9c3038bb7 100644
--- a/src/Symfony/Component/ExpressionLanguage/Node/NullCoalesceNode.php
+++ b/src/Symfony/Component/ExpressionLanguage/Node/NullCoalesceNode.php
@@ -38,9 +38,7 @@ class NullCoalesceNode extends Node
public function evaluate(array $functions, array $values): mixed
{
- if ($this->nodes['expr1'] instanceof GetAttrNode) {
- $this->nodes['expr1']->attributes['is_null_coalesce'] = true;
- }
+ $this->addNullCoalesceAttributeToGetAttrNodes($this->nodes['expr1']);
return $this->nodes['expr1']->evaluate($functions, $values) ?? $this->nodes['expr2']->evaluate($functions, $values);
}
@@ -49,4 +47,17 @@ class NullCoalesceNode extends Node
{
return ['(', $this->nodes['expr1'], ') ?? (', $this->nodes['expr2'], ')'];
}
+
+ private function addNullCoalesceAttributeToGetAttrNodes(Node $node): void
+ {
+ if (!$node instanceof GetAttrNode) {
+ return;
+ }
+
+ $node->attributes['is_null_coalesce'] = true;
+
+ foreach ($node->nodes as $node) {
+ $this->addNullCoalesceAttributeToGetAttrNodes($node);
+ }
+ }
} |
|
Closing in favor of #52964 |
…yweb) This PR was merged into the 6.3 branch. Discussion ---------- [ExpressionLanguage] Fix null coalescing propagation | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | #47192 | License | MIT This PR replaces #47446 since the author didn't reply. I think my patch suggestion is better. Commits ------- 6c5ca23 [ExpressionLanguage] Fix null coalescing propagation
This is a fix for issue #47192 which consist of dealing with nested object's properties evalution problem occuring for levels 3 and above.