From 57766a1dc2e3013e5be18304edc05db96916dfde Mon Sep 17 00:00:00 2001 From: Simon Praetorius Date: Wed, 5 Jul 2023 22:43:27 +0200 Subject: [PATCH] [FEATURE] Array unpacking (spread operator) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change adds unpacking to Fluid’s array syntax, also known as the spread operator. Internally, it uses PHP’s own spread operator, so the behavior should be consistent between Fluid and PHP. Example: Result: {key1: 'value1', key2: 'value2', anotherKey: 'another value'} Note that this change does not cover dynamic ViewHelper arguments. This means that you can only use the spread operator in normal array contexts, but not for arguments in inline ViewHelper syntax. This is currently not possible because then ViewHelper arguments could only be validated at runtime rather than parsetime, which is where validation currently happens for performance reasons. Because of similarities between object accessor and array definition syntax, there is an edge case where Fluid wrongly chooses object accessor instead of array syntax. This only happens if the array syntax is used without any spaces at the beginning and only with a single spread operator. This case shouldn’t be relevant for real-world usage because it only creates a copy of the original array. This edge case results in null: {...input1} These variants work fine: { ...input1} { ...input1 } {...input1, ...input2} {key: value, ...input1} --- src/Core/Parser/Patterns.php | 89 +++++++------ src/Core/Parser/SyntaxTree/ArrayNode.php | 29 ++++- src/Core/Parser/TemplateParser.php | 8 +- .../Cases/Parsing/ArraySyntaxTest.php | 119 ++++++++++++++++++ tests/Unit/Core/Parser/TemplateParserTest.php | 31 +++++ 5 files changed, 233 insertions(+), 43 deletions(-) create mode 100644 tests/Functional/Cases/Parsing/ArraySyntaxTest.php diff --git a/src/Core/Parser/Patterns.php b/src/Core/Parser/Patterns.php index 391b99cf7..09095c41e 100644 --- a/src/Core/Parser/Patterns.php +++ b/src/Core/Parser/Patterns.php @@ -201,26 +201,34 @@ abstract class Patterns * THIS IS ALMOST THE SAME AS IN SCAN_PATTERN_SHORTHANDSYNTAX_OBJECTACCESSORS */ public static $SCAN_PATTERN_SHORTHANDSYNTAX_ARRAYS = '/^ - (?P # Start the recursive part of the regular expression - describing the array syntax - { # Each array needs to start with { - (?P # Start sub-match + (?P # Start the recursive part of the regular expression - describing the array syntax + { # Each array needs to start with { + (?P # Start sub-match (?: - \s*( - [a-zA-Z0-9\\-_]+ # Unquoted key - |"(?:\\\"|[^"])+" # Double quoted key, supporting more characters like dots and square brackets - |\'(?:\\\\\'|[^\'])+\' # Single quoted key, supporting more characters like dots and square brackets + (?: + \s*( + [a-zA-Z0-9\\-_]+ # Unquoted key + |"(?:\\\"|[^"])+" # Double quoted key, supporting more characters like dots and square brackets + |\'(?:\\\\\'|[^\'])+\' # Single quoted key, supporting more characters like dots and square brackets + ) + \s*[:=]\s* # Key|Value delimiter : or = + (?: # Possible value options: + "(?:\\\"|[^"])*" # Double quoted string + |\'(?:\\\\\'|[^\'])*\' # Single quoted string + |[a-zA-Z0-9\-_.]+ # variable identifiers + |(?P>Recursion) # Another sub-array + ) # END possible value options + \s*,?\s* # There might be a , to separate different parts of the array + ) + |(?: # Array unpacking (spread operator) + \s* + \.{3}\s*(?:(?=[^,{}\.]*[a-zA-Z])[a-zA-Z0-9_-]*) + (?:\\.[a-zA-Z0-9_-]+)* + \s*,?\s* # There might be a , to separate different parts of the array ) - \s*[:=]\s* # Key|Value delimiter : or = - (?: # Possible value options: - "(?:\\\"|[^"])*" # Double quoted string - |\'(?:\\\\\'|[^\'])*\' # Single quoted string - |[a-zA-Z0-9\-_.]+ # variable identifiers - |(?P>Recursion) # Another sub-array - ) # END possible value options - \s*,?\s* # There might be a , to separate different parts of the array - )* # The above cycle is repeated for all array elements - ) # End array sub-match - } # Each array ends with } + )* # The above cycle is repeated for all array elements + ) # End array sub-match + } # Each array ends with } )$/x'; /** @@ -229,25 +237,34 @@ abstract class Patterns * Note that this pattern can be used on strings with or without surrounding curly brackets. */ public static $SPLIT_PATTERN_SHORTHANDSYNTAX_ARRAY_PARTS = '/ - (?P # Start sub-match of one key and value pair - (?P # The arry key - [a-zA-Z0-9_-]+ # Unquoted - |"(?:\\\\"|[^"])+" # Double quoted - |\'(?:\\\\\'|[^\'])+\' # Single quoted - ) - \\s*[:=]\\s* # Key|Value delimiter : or = - (?: # BEGIN Possible value options - (?P # Quoted string - "(?:\\\\"|[^"])*" - |\'(?:\\\\\'|[^\'])*\' + (?P # Start sub-match of one key and value pair + (?: + (?P # The arry key + [a-zA-Z0-9_-]+ # Unquoted + |"(?:\\\\"|[^"])+" # Double quoted + |\'(?:\\\\\'|[^\'])+\' # Single quoted ) - |(?P - (?:(?=[^,{}\.]*[a-zA-Z])[a-zA-Z0-9_-]*) # variable identifiers must contain letters (otherwise they are hardcoded numbers) - (?:\\.[a-zA-Z0-9_-]+)* # but in sub key access only numbers are fine (foo.55) + \\s*[:=]\\s* # Key|Value delimiter : or = + (?: # BEGIN Possible value options + (?P # Quoted string + "(?:\\\\"|[^"])*" + |\'(?:\\\\\'|[^\'])*\' + ) + |(?P + (?:(?=[^,{}\.]*[a-zA-Z])[a-zA-Z0-9_-]*) # variable identifiers must contain letters (otherwise they are hardcoded numbers) + (?:\\.[a-zA-Z0-9_-]+)* # but in sub key access only numbers are fine (foo.55) + ) + |(?P[0-9]+(?:\\.[0-9]+)?) # A hardcoded Number (also possibly with decimals) + |\\{\\s*(?P(?:(?P>ArrayPart)\\s*,?\\s*)+)\\s*\\} # Another sub-array + ) # END possible value options + ) + |(?: + \.{3}\\s* # Array unpacking (spread operator) + (?P + (?:(?=[^,{}\.]*[a-zA-Z])[a-zA-Z0-9_-]*) + (?:\\.[a-zA-Z0-9_-]+)* ) - |(?P[0-9]+(?:\\.[0-9]+)?) # A hardcoded Number (also possibly with decimals) - |\\{\\s*(?P(?:(?P>ArrayPart)\\s*,?\\s*)+)\\s*\\} # Another sub-array - ) # END possible value options - )\\s*(?=\\z|,|\\}) # An array part sub-match ends with either a comma, a closing curly bracket or end of string + ) + )\\s*(?=\\z|,|\\}) # An array part sub-match ends with either a comma, a closing curly bracket or end of string /x'; } diff --git a/src/Core/Parser/SyntaxTree/ArrayNode.php b/src/Core/Parser/SyntaxTree/ArrayNode.php index 0cdfc6dfb..2fdde2152 100644 --- a/src/Core/Parser/SyntaxTree/ArrayNode.php +++ b/src/Core/Parser/SyntaxTree/ArrayNode.php @@ -18,6 +18,8 @@ */ class ArrayNode extends AbstractNode { + public const SPREAD_PREFIX = '__spread'; + /** * Constructor. * @@ -37,7 +39,15 @@ public function evaluate(RenderingContextInterface $renderingContext) { $arrayToBuild = []; foreach ($this->internalArray as $key => $value) { - $arrayToBuild[$key] = $value instanceof NodeInterface ? $value->evaluate($renderingContext) : $value; + if ($value instanceof NodeInterface) { + if (str_starts_with($key, self::SPREAD_PREFIX)) { + $arrayToBuild = [...$arrayToBuild, ...$value->evaluate($renderingContext)]; + } else { + $arrayToBuild[$key] = $value->evaluate($renderingContext); + } + } else { + $arrayToBuild[$key] = $value; + } } return $arrayToBuild; } @@ -53,11 +63,18 @@ public function convert(TemplateCompiler $templateCompiler): array if (!empty($converted['initialization'])) { $accumulatedInitializationPhpCode .= $converted['initialization']; } - $initializationPhpCode .= sprintf( - '\'%s\' => %s,' . chr(10), - $key, - $converted['execution'] - ); + if (str_starts_with($key, self::SPREAD_PREFIX)) { + $initializationPhpCode .= sprintf( + '...%s,' . chr(10), + $converted['execution'] + ); + } else { + $initializationPhpCode .= sprintf( + '\'%s\' => %s,' . chr(10), + $key, + $converted['execution'] + ); + } } elseif (is_numeric($value)) { // handle int, float, numeric strings $initializationPhpCode .= sprintf( diff --git a/src/Core/Parser/TemplateParser.php b/src/Core/Parser/TemplateParser.php index 18c30fa6e..651f16b92 100644 --- a/src/Core/Parser/TemplateParser.php +++ b/src/Core/Parser/TemplateParser.php @@ -761,9 +761,15 @@ protected function recursiveArrayHandler(ParsingState $state, $arrayText, ViewHe } $matches = []; $arrayToBuild = []; + $spreadVariableCounter = 0; if (preg_match_all(Patterns::$SPLIT_PATTERN_SHORTHANDSYNTAX_ARRAY_PARTS, $arrayText, $matches, PREG_SET_ORDER)) { foreach ($matches as $singleMatch) { - $arrayKey = $this->unquoteString($singleMatch['Key']); + if (array_key_exists('SpreadVariableIdentifier', $singleMatch)) { + $arrayKey = ArrayNode::SPREAD_PREFIX . $spreadVariableCounter++; + $singleMatch['VariableIdentifier'] = $singleMatch['SpreadVariableIdentifier']; + } else { + $arrayKey = $this->unquoteString($singleMatch['Key']); + } $assignInto = &$arrayToBuild; $isBoolean = false; $argumentDefinition = null; diff --git a/tests/Functional/Cases/Parsing/ArraySyntaxTest.php b/tests/Functional/Cases/Parsing/ArraySyntaxTest.php new file mode 100644 index 000000000..7ad943db2 --- /dev/null +++ b/tests/Functional/Cases/Parsing/ArraySyntaxTest.php @@ -0,0 +1,119 @@ + [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + ], + null, + ], + // Edge case: Fluid treats this expression as an object accessor instead of an array + 'single array spread with whitespace after' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + ], + null, + ], + 'single array spread with whitespace before' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + ], + ['abc' => 1, 'def' => 2], + ], + 'single array spread' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + ], + ['abc' => 1, 'def' => 2], + ], + 'multiple array spreads' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + 'input2' => ['ghi' => 3], + ], + ['abc' => 1, 'def' => 2, 'ghi' => 3], + ], + 'multiple array spreads mixed with other items' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + 'input2' => ['ghi' => 3], + ], + ['first' => 1, 'abc' => 1, 'def' => 2, 'middle' => 'middle value', 'ghi' => 3, 'last' => ['sub' => 1]], + ], + 'overwrite static value' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + ], + ['abc' => 1, 'def' => 2], + ], + 'overwrite spreaded value' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + ], + ['abc' => 10, 'def' => 2], + ], + 'overwrite spreaded value with spreaded value' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + 'input2' => ['abc' => 10], + ], + ['abc' => 10, 'def' => 2], + ], + 'whitespace variants' => [ + '', + [ + 'input1' => ['abc' => 1, 'def' => 2], + 'input2' => ['ghi' => 3], + ], + ['abc' => 1, 'def' => 2, 'ghi' => 3], + ] + ]; + } + + /** + * @test + * @dataProvider arraySyntaxDataProvider + */ + public function arraySyntax(string $source, array $variables, $expected): void + { + $view = new TemplateView(); + $view->getRenderingContext()->setCache(self::$cache); + $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($source); + $view->assignMultiple($variables); + $view->render(); + self::assertSame($view->getRenderingContext()->getVariableProvider()->get('result'), $expected); + + $view = new TemplateView(); + $view->getRenderingContext()->setCache(self::$cache); + $view->getRenderingContext()->getTemplatePaths()->setTemplateSource($source); + $view->assignMultiple($variables); + $view->render(); + self::assertSame($view->getRenderingContext()->getVariableProvider()->get('result'), $expected); + } +} diff --git a/tests/Unit/Core/Parser/TemplateParserTest.php b/tests/Unit/Core/Parser/TemplateParserTest.php index b4ff9c262..97c33e545 100644 --- a/tests/Unit/Core/Parser/TemplateParserTest.php +++ b/tests/Unit/Core/Parser/TemplateParserTest.php @@ -553,6 +553,37 @@ public static function dataProviderRecursiveArrayHandler(): \Generator ] ]; + yield 'Single array spread' => [ + 'string' => '...some.identifier', + 'expected' => [ + '__spread0' => new ObjectAccessorNode('some.identifier') + ] + ]; + + yield 'Multiple arrays spread' => [ + 'string' => '...some.identifier, ...other.identifier', + 'expected' => [ + '__spread0' => new ObjectAccessorNode('some.identifier'), + '__spread1' => new ObjectAccessorNode('other.identifier') + ] + ]; + + yield 'Mixed types and arrays spread' => [ + 'string' => 'number: 123, string: \'some.string\', identifier: some.identifier, ...some.identifier, array: {number: 123, string: \'some.string\', identifier: some.identifier}, ...other.identifier', + 'expected' => [ + 'number' => 123, + 'string' => new TextNode('some.string'), + 'identifier' => new ObjectAccessorNode('some.identifier'), + '__spread0' => new ObjectAccessorNode('some.identifier'), + 'array' => new ArrayNode([ + 'number' => 123, + 'string' => new TextNode('some.string'), + 'identifier' => new ObjectAccessorNode('some.identifier') + ]), + '__spread1' => new ObjectAccessorNode('other.identifier') + ] + ]; + $rootNode = new RootNode(); $rootNode->addChildNode(new ObjectAccessorNode('some.{index}')); yield 'variable identifier' => [