diff --git a/src/Drop.php b/src/Drop.php index cb0c644..90a33ff 100644 --- a/src/Drop.php +++ b/src/Drop.php @@ -5,6 +5,7 @@ use Keepsuit\Liquid\Concerns\ContextAware; use Keepsuit\Liquid\Contracts\IsContextAware; use Keepsuit\Liquid\Exceptions\UndefinedDropMethodException; +use Keepsuit\Liquid\Support\DropMemberType; use Keepsuit\Liquid\Support\DropMetadata; use Keepsuit\Liquid\Support\Str; @@ -47,44 +48,28 @@ public function __toString(): string public function __get(string $name): mixed { - $invokableMethods = $this->getMetadata()->invokableMethods; - $cacheableMethods = $this->getMetadata()->cacheableMethods; - - $possibleNames = array_unique([ - $name, - Str::camel($name), - Str::snake($name), - ]); - - foreach ($possibleNames as $propertyName) { - if (in_array($propertyName, $this->getMetadata()->properties)) { - return $this->{$propertyName}; - } - } + $metadata = $this->getMetadata(); + $resolution = $metadata->resolveStaticMember($name); - foreach ($possibleNames as $methodName) { - if (! in_array($methodName, $invokableMethods)) { - continue; + if ($resolution !== null) { + if ($resolution->type === DropMemberType::Property) { + return $this->{$resolution->name}; } - $isCacheable = in_array($methodName, $cacheableMethods); - - if ($isCacheable && isset($this->cache[$methodName])) { - return $this->cache[$methodName]; + if ($resolution->cacheable && array_key_exists($resolution->name, $this->cache)) { + return $this->cache[$resolution->name]; } - if (method_exists($this, $methodName)) { - $result = $this->{$methodName}(); + $result = $this->{$resolution->name}(); - if ($isCacheable) { - $this->cache[$methodName] = $result; - } - - return $result; + if ($resolution->cacheable) { + $this->cache[$resolution->name] = $result; } + + return $result; } - foreach ($possibleNames as $methodName) { + foreach ($metadata->possibleNames($name) as $methodName) { try { return $this->liquidMethodMissing($methodName); } catch (UndefinedDropMethodException) { diff --git a/src/Drops/SelfDrop.php b/src/Drops/SelfDrop.php index 52d938c..a356b8e 100644 --- a/src/Drops/SelfDrop.php +++ b/src/Drops/SelfDrop.php @@ -3,6 +3,7 @@ namespace Keepsuit\Liquid\Drops; use Keepsuit\Liquid\Render\RenderContext; +use Keepsuit\Liquid\Support\MissingValue; /** * Proxy object that resolves property lookups through the current render context scope chain. @@ -17,16 +18,16 @@ public function __construct( public function __get(string $name): mixed { - $variables = $this->context->findVariables($name); + $variable = $this->context->findVariable($name); - return $variables[0] ?? null; + return $variable instanceof MissingValue ? null : $variable; } public function __isset(string $name): bool { - $variables = $this->context->findVariables($name); + $variable = $this->context->findVariable($name); - return $variables !== []; + return ! $variable instanceof MissingValue; } public function __toString(): string diff --git a/src/Nodes/BodyNode.php b/src/Nodes/BodyNode.php index 60aa104..3cfde82 100644 --- a/src/Nodes/BodyNode.php +++ b/src/Nodes/BodyNode.php @@ -3,6 +3,7 @@ namespace Keepsuit\Liquid\Nodes; use Keepsuit\Liquid\Contracts\CanBeStreamed; +use Keepsuit\Liquid\Contracts\Disableable; use Keepsuit\Liquid\Exceptions\LiquidException; use Keepsuit\Liquid\Exceptions\UndefinedDropMethodException; use Keepsuit\Liquid\Exceptions\UndefinedFilterException; @@ -52,12 +53,19 @@ public function render(RenderContext $context): string $output = ''; foreach ($this->children as $node) { + // Text is the majority of children and cannot fail or interrupt. + if ($node instanceof Text) { + $output .= $node->value; + + continue; + } + try { - if ($node instanceof Tag) { + if ($node instanceof Disableable && $node instanceof Tag) { $node->ensureTagIsEnabled($context); } - $output .= $this->renderChild($context, $node); + $output .= $node->render($context); } catch (UndefinedVariableException|UndefinedDropMethodException|UndefinedFilterException $exception) { $context->handleError($exception, $node->lineNumber); } catch (\Throwable $exception) { @@ -84,12 +92,26 @@ public function stream(RenderContext $context): \Generator $context->resourceLimits->incrementRenderScore(count($this->children)); foreach ($this->children as $node) { + // Text is the majority of children and cannot fail or interrupt. + if ($node instanceof Text) { + $context->resourceLimits->incrementWriteScore($node->value); + yield $node->value; + + continue; + } + try { - if ($node instanceof Tag) { + if ($node instanceof Disableable && $node instanceof Tag) { $node->ensureTagIsEnabled($context); } - foreach ($this->streamChild($context, $node) as $output) { + if ($node instanceof CanBeStreamed) { + foreach ($node->stream($context) as $output) { + $context->resourceLimits->incrementWriteScore($output); + yield $output; + } + } else { + $output = $node->render($context); $context->resourceLimits->incrementWriteScore($output); yield $output; } @@ -107,25 +129,6 @@ public function stream(RenderContext $context): \Generator } } - protected function renderChild(RenderContext $context, Node $node): string - { - return $node->render($context); - } - - /** - * @return \Generator - */ - public function streamChild(RenderContext $context, Node $node): \Generator - { - if ($node instanceof CanBeStreamed) { - yield from $node->stream($context); - - return; - } - - yield $node->render($context); - } - public function blank(): bool { foreach ($this->children as $node) { diff --git a/src/Nodes/Variable.php b/src/Nodes/Variable.php index fe7c16e..9fd98ac 100644 --- a/src/Nodes/Variable.php +++ b/src/Nodes/Variable.php @@ -84,9 +84,19 @@ public function evaluate(RenderContext $context): mixed } foreach ($this->filters as [$filterName, $filterArgs, $filterNamedArgs]) { - $filterArgs = $this->evaluateFilterExpressions($context, $filterArgs ?? []); - $filterNamedArgs = $this->evaluateFilterExpressions($context, $filterNamedArgs ?? []); - $output = $context->applyFilter($filterName, $output, [...$filterArgs, ...$filterNamedArgs]); + if ($filterArgs === [] && $filterNamedArgs === []) { + $output = $context->applyFilter($filterName, $output); + + continue; + } + + $filterArgs = $this->evaluateFilterExpressions($context, $filterArgs); + + if ($filterNamedArgs !== []) { + $filterArgs = [...$filterArgs, ...$this->evaluateFilterExpressions($context, $filterNamedArgs)]; + } + + $output = $context->applyFilter($filterName, $output, $filterArgs); } return $output; diff --git a/src/Nodes/VariableLookup.php b/src/Nodes/VariableLookup.php index 4e7d2b8..9d4d6d3 100644 --- a/src/Nodes/VariableLookup.php +++ b/src/Nodes/VariableLookup.php @@ -16,24 +16,11 @@ class VariableLookup implements CanBeEvaluated, HasParseTreeVisitorChildren private const LOOKUP_REGEX = '{\.([\w\-]+)|\["([\w\-]+)"\]|\[\'([\w\-]+)\'\]|\[(\d+)\]}'; - /** - * @var int[] - */ - public readonly array $lookupFilters; - public function __construct( public readonly string $name, - /** @var string[] */ + /** @var array */ public readonly array $lookups = [], - ) { - $lookupFilters = []; - foreach ($this->lookups as $i => $lookup) { - if (in_array($lookup, self::FILTER_METHODS, true)) { - $lookupFilters[] = $i; - } - } - $this->lookupFilters = $lookupFilters; - } + ) {} /** * Parses `a.b[0]["c"]` into a name plus its lookups. @@ -88,49 +75,86 @@ public function parseTreeVisitorChildren(): array public function evaluate(RenderContext $context): mixed { - $name = $context->evaluate($this->name); - assert(is_string($name)); - $variables = $context->findVariables($name); + $variable = $context->findVariable($this->name); + + if ($variable instanceof MissingValue) { + return $this->undefined($context); + } if ($this->lookups === []) { - if ($context->options->strictVariables && $variables === []) { - return new UndefinedVariable($this->toString()); - } + return $variable; + } + + $result = $this->walkLookups($context, $variable); - return $variables[0] ?? null; + if (! $result instanceof MissingValue) { + return $result; } - foreach ($variables as $object) { - $object = $context->evaluate($object); + // The name resolved but the lookup chain broke on the innermost value: an + // outer scope may still hold one the chain resolves against. + foreach ($context->findVariables($this->name) as $candidate) { + // Skip the value already walked above: re-walking it would repeat any + // side effects the broken chain triggered on the way. + if ($candidate === $variable) { + continue; + } + + $result = $this->walkLookups($context, $candidate); - if ($object instanceof \Generator) { - $object = iterator_to_array($object, preserve_keys: false); + if (! $result instanceof MissingValue) { + return $result; } + } + + return $this->undefined($context); + } + + protected function undefined(RenderContext $context): ?UndefinedVariable + { + return $context->options->strictVariables ? new UndefinedVariable($this->toString()) : null; + } + + /** + * Walks the lookup chain against $object, returning MissingValue if it breaks. + */ + protected function walkLookups(RenderContext $context, mixed $object): mixed + { + if ($object instanceof CanBeEvaluated) { + $object = $context->evaluate($object); + } - foreach ($this->lookups as $i => $lookup) { - $key = $context->evaluate($lookup) ?? ''; + if ($object instanceof \Generator) { + $object = iterator_to_array($object, preserve_keys: false); + } - assert(is_string($key) || is_int($key)); + foreach ($this->lookups as $lookup) { + $key = $lookup instanceof VariableLookup ? $context->evaluate($lookup) : $lookup; - $nextObject = $context->evaluate($context->internalContextLookup($object, $key)); + if (! (is_string($key) || is_int($key))) { + return new MissingValue; + } - if ($nextObject instanceof MissingValue && is_iterable($object) && in_array($i, $this->lookupFilters, true)) { - $nextObject = $context->applyFilter($lookup, $object); - } + $nextObject = $context->internalContextLookup($object, $key); - if ($nextObject instanceof MissingValue) { - continue 2; - } + if ($nextObject instanceof CanBeEvaluated) { + $nextObject = $context->evaluate($nextObject); + } - $object = $nextObject; - if ($object instanceof IsContextAware) { - $object->setContext($context); + if ($nextObject instanceof MissingValue) { + if (is_iterable($object) && is_string($lookup) && in_array($lookup, self::FILTER_METHODS, true)) { + $nextObject = $context->applyFilter($lookup, $object); + } else { + return $nextObject; } } - return $object; + $object = $nextObject; + if ($object instanceof IsContextAware) { + $object->setContext($context); + } } - return $context->options->strictVariables ? new UndefinedVariable($this->toString()) : null; + return $object; } } diff --git a/src/Parse/ExpressionParser.php b/src/Parse/ExpressionParser.php index f3816f0..43a113b 100644 --- a/src/Parse/ExpressionParser.php +++ b/src/Parse/ExpressionParser.php @@ -52,7 +52,7 @@ public function parseExpression(): mixed protected function parseVariable(): VariableLookup { $name = $this->tokenStream->consume(TokenType::Identifier)->data; - $lookups = $this->parseVariableLookups(); + $lookups = $this->parseVariableLookups($name); return new VariableLookup( name: $name, @@ -63,7 +63,7 @@ protected function parseVariable(): VariableLookup /** * @throws SyntaxException */ - protected function parseVariableLookups(): array + protected function parseVariableLookups(string $variableName): array { $lookups = []; @@ -74,7 +74,17 @@ protected function parseVariableLookups(): array continue; } if ($this->tokenStream->consumeOrFalse(TokenType::OpenSquare)) { - $lookups[] = $this->tokenStream->expression(); + $expression = $this->tokenStream->expression(); + $lookups[] = match (true) { + is_string($expression), is_int($expression), $expression instanceof VariableLookup => $expression, + default => throw new SyntaxException(sprintf('Invalid variable lookup: %s[%s]', $variableName, match (true) { + $expression instanceof Literal => $expression->value, + $expression instanceof RangeLookup => $expression->toString(), + is_bool($expression) => $expression ? 'true' : 'false', + $expression === null => 'nil', + default => (string) $expression + })), + }; $this->tokenStream->consume(TokenType::CloseSquare); continue; diff --git a/src/Render/RenderContext.php b/src/Render/RenderContext.php index 65cf862..e2bdac7 100644 --- a/src/Render/RenderContext.php +++ b/src/Render/RenderContext.php @@ -144,8 +144,8 @@ public function stack(Closure $closure) public function evaluate(mixed $value): mixed { - if ($value instanceof CanBeEvaluated) { - return $this->evaluate($value->evaluate($this)); + while ($value instanceof CanBeEvaluated) { + $value = $value->evaluate($this); } return $value; @@ -177,40 +177,76 @@ public function has(string $key): bool return $this->get($key) !== null; } - public function findVariables(string $key): array + /** + * Resolves $key against the scope chain and returns the innermost value. + * + * @return mixed the value, or MissingValue when the key is undefined everywhere + */ + public function findVariable(string $key): mixed { - $variables = []; + // Deliberately not written as a loop over [...$this->scopes, $this->data, ...]: + // building that list would allocate an array on every variable reference. + foreach ($this->scopes as $scope) { + if (array_key_exists($key, $scope)) { + return $this->resolveVariable($scope[$key]); + } + } - // Check the variable in all scopes + env data + static variables - $scopeCount = count($this->scopes); - for ($index = 0; $index < $scopeCount + 2; $index++) { - $scope = match (true) { - $index < $scopeCount => $this->scopes[$index], - $index === $scopeCount => $this->data, - default => $this->sharedState->staticVariables, - }; + if (array_key_exists($key, $this->data)) { + return $this->resolveVariable($this->data[$key]); + } - $value = $this->internalContextLookup($scope, $key); + if (array_key_exists($key, $this->sharedState->staticVariables)) { + return $this->resolveVariable($this->sharedState->staticVariables[$key]); + } + + // Fall back to the implicit self drop only when no value was found anywhere. + return $key === 'self' ? $this->getSelfDrop() : $this->missingValue; + } + + /** + * Every value $key resolves to, innermost scope first. + * + * Only useful to callers that need to fall back to an outer scope when the + * innermost value does not satisfy them; prefer findVariable() otherwise. + * + * @return list + */ + public function findVariables(string $key): array + { + $variables = []; - if (! $value instanceof MissingValue) { - $variables[] = $value; + foreach ([...$this->scopes, $this->data, $this->sharedState->staticVariables] as $scope) { + if (array_key_exists($key, $scope)) { + $variables[] = $this->resolveVariable($scope[$key]); } } - // Inject the implicit self drop only when no value (including explicit null) was found. - // An explicit `self = nil` leaves [null] in $variables, so the fallback is skipped, - // correctly distinguishing defined-null from undefined. + // Fall back to the implicit self drop only when no value was found anywhere. if ($variables === [] && $key === 'self') { return [$this->getSelfDrop()]; } - foreach ($variables as $variable) { - if ($variable instanceof IsContextAware) { - $variable->setContext($this); - } + return $variables; + } + + /** + * Normalizes a value pulled out of a scope and binds it to this context. + */ + protected function resolveVariable(mixed $value): mixed + { + // Only objects can need either step, and scalars dominate the hot path. + if (! is_object($value)) { + return $value; } - return $variables; + $value = $this->normalizeValue($value); + + if ($value instanceof IsContextAware) { + $value->setContext($this); + } + + return $value; } public function getSelfDrop(): SelfDrop @@ -222,17 +258,23 @@ public function internalContextLookup(mixed $scope, int|string $key): mixed { try { $value = match (true) { + is_array($scope) => match (true) { + array_key_exists($key, $scope) => $scope[$key], + default => $this->missingValue, + }, $scope instanceof Drop => $scope->{$key}, - is_array($scope) && array_key_exists($key, $scope) => $scope[$key], - is_object($scope) && $this->objectHasProperty($scope, (string) $key) => $scope->{$key}, - is_object($scope) && $this->objectHasStaticProperty($scope, (string) $key) => $scope::$$key, + is_object($scope) => match (true) { + $this->objectHasProperty($scope, (string) $key) => $scope->{$key}, + $this->objectHasStaticProperty($scope, (string) $key) => $scope::$$key, + default => $this->missingValue, + }, default => $this->missingValue, }; } catch (UndefinedDropMethodException) { return $this->missingValue; } - return $this->normalizeValue($value); + return is_object($value) ? $this->normalizeValue($value) : $value; } protected function objectHasProperty(object $object, string $property): bool @@ -264,7 +306,16 @@ protected function objectHasStaticProperty(object $object, string $property): bo public function normalizeValue(mixed $value): mixed { - if (is_object($value) && isset($this->sharedState->computedObjectsCache[$value])) { + // Only objects can need normalization, and scalars dominate the hot path. + if (! is_object($value)) { + return $value; + } + + if ($value instanceof MissingValue) { + return $value; + } + + if (isset($this->sharedState->computedObjectsCache[$value])) { return $this->sharedState->computedObjectsCache[$value]; } @@ -332,7 +383,7 @@ public function popInterrupt(): ?Interrupt public function hasInterrupt(): bool { - return count($this->interrupts) > 0; + return $this->interrupts !== []; } /** diff --git a/src/Support/DropMemberResolution.php b/src/Support/DropMemberResolution.php new file mode 100644 index 0000000..6b678fe --- /dev/null +++ b/src/Support/DropMemberResolution.php @@ -0,0 +1,25 @@ +> */ + private array $possibleNames = []; + + /** @var array + */ + private array $staticResolution = []; + public function __construct( /** @var list */ public readonly array $invokableMethods = [], @@ -34,6 +41,46 @@ public function __construct( public readonly array $dynamicProperties = [], ) {} + /** + * @return list + */ + public function possibleNames(string $name): array + { + return $this->possibleNames[$name] ??= array_values(array_unique([ + $name, + Str::camel($name), + Str::snake($name), + ])); + } + + public function resolveStaticMember(string $name): ?DropMemberResolution + { + if (array_key_exists($name, $this->staticResolution)) { + return $this->staticResolution[$name]; + } + + $possibleNames = $this->possibleNames($name); + + foreach ($possibleNames as $propertyName) { + if (in_array($propertyName, $this->properties, true)) { + return $this->staticResolution[$name] = DropMemberResolution::property($propertyName); + } + } + + foreach ($possibleNames as $methodName) { + if (in_array($methodName, $this->invokableMethods, true)) { + return $this->staticResolution[$name] = DropMemberResolution::method( + $methodName, + in_array($methodName, $this->cacheableMethods, true) + ); + } + } + + $this->staticResolution[$name] = null; + + return null; + } + public static function init(Drop $drop): DropMetadata { if (isset(self::$cache[get_class($drop)])) { diff --git a/tests/Integration/ContextTest.php b/tests/Integration/ContextTest.php index 6cbb088..64aef6f 100644 --- a/tests/Integration/ContextTest.php +++ b/tests/Integration/ContextTest.php @@ -114,6 +114,29 @@ 'strict' => true, ]); +test('lookup falls back to outer scope when the inner value has no such key', function (bool $strict) { + $context = new RenderContext(options: new RenderContextOptions(strictVariables: $strict)); + $context->set('product', ['title' => 'outer']); + + $context->stack(function () use ($context, $strict) { + // The inner `product` shadows the outer one but cannot resolve `.title`, + // so resolution has to continue into the outer scope. + $context->set('product', ['handle' => 'inner']); + + expect($context->get('product.handle'))->toBe('inner'); + expect($context->get('product.title'))->toBe('outer'); + + if ($strict) { + expect($context->get('product.missing'))->toBeInstanceOf(UndefinedVariable::class); + } else { + expect($context->get('product.missing'))->toBeNull(); + } + }); +})->with([ + 'default' => false, + 'strict' => true, +]); + test('add item in inner scope', function (bool $strict) { $context = new RenderContext(options: new RenderContextOptions(strictVariables: $strict)); $context->stack(function () use ($context) { @@ -735,3 +758,50 @@ function () use (&$global) { 'default' => false, 'strict' => true, ]); + +test('lazy variable resolution returns the innermost scope', function (bool $strict) { + $context = new RenderContext(options: new RenderContextOptions(strictVariables: $strict)); + $context->set('test', 'outer'); + + $context->stack(function () use ($context) { + $context->set('test', 'inner'); + + expect($context->get('test'))->toBe('inner'); + }); + + expect($context->get('test'))->toBe('outer'); +})->with([ + 'default' => false, + 'strict' => true, +]); + +test('lazy variable resolution distinguishes defined-null from undefined', function (bool $strict) { + $context = new RenderContext(options: new RenderContextOptions(strictVariables: $strict)); + $context->set('definedNull', null); + + expect($context->get('definedNull'))->toBe(null); + + if ($strict) { + expect($context->get('neverSet'))->toBeInstanceOf(UndefinedVariable::class); + } else { + expect($context->get('neverSet'))->toBe(null); + } +})->with([ + 'default' => false, + 'strict' => true, +]); + +test('lazy variable resolution falls back when a lookup path misses', function (bool $strict) { + $context = new RenderContext(options: new RenderContextOptions(strictVariables: $strict)); + $context->set('test', ['b' => 'outer']); + + $context->stack(function () use ($context) { + $context->set('test', ['c' => 'inner']); + + expect($context->get('test.c'))->toBe('inner'); + expect($context->get('test.b'))->toBe('outer'); + }); +})->with([ + 'default' => false, + 'strict' => true, +]); diff --git a/tests/Integration/DropTest.php b/tests/Integration/DropTest.php index 70ea8f4..2bac54f 100644 --- a/tests/Integration/DropTest.php +++ b/tests/Integration/DropTest.php @@ -144,13 +144,14 @@ ->properties->toBe([]); expect(invade(new CachableDrop)->getMetadata()) - ->invokableMethods->toBe(['notCached', 'cached']) - ->cacheableMethods->toBe(['cached']) + ->invokableMethods->toBe(['notCached', 'cached', 'cachedNull', 'cachedNullCalls']) + ->cacheableMethods->toBe(['cached', 'cachedNull']) ->properties->toBe([]); }); it('can cache drop method calls', function () { $drop = new CachableDrop; + $anotherDrop = new CachableDrop; expect($drop) ->notCached->toBe(0) @@ -158,9 +159,36 @@ expect($drop) ->cached->toBe(0) + ->cached->toBe(0) + ->and($anotherDrop) ->cached->toBe(0); }); +it('can cache null drop method results', function () { + $drop = new CachableDrop; + + expect($drop) + ->cachedNull->toBeNull() + ->cachedNull->toBeNull() + ->cachedNullCalls->toBe(1); +}); + +it('does not cache dynamic drop method results', function () { + $drop = new class extends \Keepsuit\Liquid\Drop + { + private int $calls = 0; + + protected function liquidMethodMissing(string $name): mixed + { + return sprintf('%s:%d', $name, ++$this->calls); + } + }; + + expect($drop) + ->unknownValue->toBe('unknownValue:1') + ->unknownValue->toBe('unknownValue:2'); +}); + it('can access drop data with snake and camel cases', function () { $drop = new ProductDrop; diff --git a/tests/Integration/VariableTest.php b/tests/Integration/VariableTest.php index e53e419..d1a080c 100644 --- a/tests/Integration/VariableTest.php +++ b/tests/Integration/VariableTest.php @@ -1,5 +1,6 @@ setRethrowErrors(false) + ->setStrictVariables(true) + ->build(); + + expect(fn () => parseTemplate('{{ a[empty] }}', $environment)) + ->toThrow(\Keepsuit\Liquid\Exceptions\SyntaxException::class, 'Invalid variable lookup: a[empty]'); +}); + function generator(): Generator { yield '1'; diff --git a/tests/Stubs/CachableDrop.php b/tests/Stubs/CachableDrop.php index 76a9788..e4f576e 100644 --- a/tests/Stubs/CachableDrop.php +++ b/tests/Stubs/CachableDrop.php @@ -11,6 +11,8 @@ class CachableDrop extends Drop protected int $cachedCounter = 0; + protected int $cachedNullCounter = 0; + public function notCached(): int { return $this->notCachedCounter++; @@ -21,4 +23,17 @@ public function cached(): int { return $this->cachedCounter++; } + + #[Cache] + public function cachedNull(): mixed + { + $this->cachedNullCounter++; + + return null; + } + + public function cachedNullCalls(): int + { + return $this->cachedNullCounter; + } }