Skip to content

Commit c8eecf0

Browse files
committed
Fix helper argument handling in strict mode
Also updated assumeObjects errors to align with Handlebars.js.
1 parent cca677f commit c8eecf0

3 files changed

Lines changed: 68 additions & 35 deletions

File tree

src/Compiler.php

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ private function PartialStatement(PartialStatement $statement): string
408408
$p = $this->SubExpression($name);
409409
$this->context->usedDynPartial++;
410410
} elseif ($name instanceof PathExpression || $name instanceof StringLiteral || $name instanceof NumberLiteral) {
411-
$partialName = $name instanceof PathExpression ? $name->original : $this->getLiteralKeyName($name);
411+
$partialName = $this->resolvePartialName($name);
412412
$p = self::quote($partialName);
413413
$this->resolveAndCompilePartial($partialName);
414414
} else {
@@ -447,22 +447,23 @@ private function PartialBlockStatement(PartialBlockStatement $statement): string
447447
$body = $this->compileProgram($statement->program);
448448

449449
if ($name instanceof PathExpression || $name instanceof StringLiteral || $name instanceof NumberLiteral) {
450-
$partialName = $name instanceof PathExpression ? $name->original : $this->getLiteralKeyName($name);
450+
$partialName = $this->resolvePartialName($name);
451451
$p = self::quote($partialName);
452452
} else {
453453
$p = $this->compileExpression($name);
454454
$partialName = null;
455455
}
456456

457+
$found = false;
458+
457459
if ($partialName !== null) {
458460
$found = isset($this->context->usedPartial[$partialName]);
459461

460462
if (!$found && !str_starts_with($partialName, '@partial-block')) {
461-
$resolveName = $partialName;
462-
$cnt = $this->resolvePartial($resolveName);
463+
$cnt = $this->resolvePartial($partialName);
463464
if ($cnt !== null) {
464-
$this->context->usedPartial[$resolveName] = $cnt;
465-
$this->compilePartialTemplate($resolveName, $cnt);
465+
$this->context->usedPartial[$partialName] = $cnt;
466+
$this->compilePartialTemplate($partialName, $cnt);
466467
$found = true;
467468
}
468469
}
@@ -479,11 +480,12 @@ private function PartialBlockStatement(PartialBlockStatement $statement): string
479480

480481
// Capture $blockParams if we're inside a block-param scope so the partial block body can access them.
481482
$useVars = $this->blockParamsUseVars();
483+
$bodyClosure = self::templateClosure($body, useVars: $useVars);
482484
$fallbackParts = ($partialName !== null && !$found)
483-
? [self::getRuntimeFunc('inFallback', "\$cx, " . self::quote($partialName) . ', ' . self::templateClosure($body, useVars: $useVars))]
485+
? [self::getRuntimeFunc('inFallback', "\$cx, " . self::quote($partialName) . ', ' . $bodyClosure)]
484486
: [];
485487
$parts = [...$hoistedParts, ...$fallbackParts,
486-
self::getRuntimeFunc('in', "\$cx, '@partial-block$pid', " . self::templateClosure($body, useVars: $useVars)),
488+
self::getRuntimeFunc('in', "\$cx, '@partial-block$pid', " . $bodyClosure),
487489
self::getRuntimeFunc('p', "\$cx, $p, $vars, $pid, ''"),
488490
];
489491
return implode('.', $parts);
@@ -639,32 +641,22 @@ private function PathExpression(PathExpression $expression): string
639641
if (count($checks) > 1) {
640642
$cond = "($cond)";
641643
}
642-
$lenStart = "($cond ? count($base$p) : ";
643-
$lenEnd = ')';
644+
$lenStart = "$cond ? count($base$p) : ";
644645

645-
return "$base$n ?? $lenStart$miss$lenEnd";
646+
return "$base$n ?? ($lenStart$miss)";
646647
}
647648

648-
if ($this->context->options->assumeObjects) {
649-
$missCode = self::getRuntimeFunc('miss', self::quote($expression->original));
650-
$conditions = ["isset($base)"];
651-
$intermediateAccess = '';
652-
foreach (array_slice($stringParts, 0, -1) as $part) {
653-
$intermediateAccess .= '[' . self::quote($part) . ']';
654-
$conditions[] = "isset($base$intermediateAccess)";
655-
}
656-
$allConds = implode(' && ', $conditions);
657-
return "($allConds ? ($base$n ?? null) : $missCode)";
649+
// assumeObjects and strict mode for helper arguments both use nullCheck chains.
650+
// This mirrors HBS.js: both paths use bare nameLookup (no container.strict wrapping), so
651+
// only a null intermediate throws (JS TypeError), while a missing key on a valid array
652+
// returns null silently (JS undefined). nullCheck encodes those semantics and includes
653+
// the key name in the exception message.
654+
if ($this->context->options->assumeObjects || ($this->context->options->strict && $this->compilingHelperArgs)) {
655+
return self::buildCallChain('nullCheck', $base, $stringParts);
658656
}
659657

660-
if ($this->context->options->strict && !$this->compilingHelperArgs) {
661-
$escapedOriginal = self::quote($expression->original);
662-
$expr = $base;
663-
foreach ($stringParts as $part) {
664-
$escapedKey = self::quote($part);
665-
$expr = self::getRuntimeFunc('strictLookup', "$expr, $escapedKey, $escapedOriginal");
666-
}
667-
return $expr;
658+
if ($this->context->options->strict) {
659+
return self::buildCallChain('strictLookup', $base, $stringParts, self::quote($expression->original));
668660
}
669661

670662
return "$base$n ?? $miss";
@@ -762,11 +754,8 @@ private function resolveAndCompilePartial(string $name): void
762754
/**
763755
* Returns the resolved partial content, or null if it doesn't exist.
764756
*/
765-
private function resolvePartial(string &$name): ?string
757+
private function resolvePartial(string $name): ?string
766758
{
767-
if ($name === '@partial-block') {
768-
$name = "@partial-block{$this->context->usedPBlock}";
769-
}
770759
if (isset($this->context->partials[$name])) {
771760
return $this->context->partials[$name];
772761
}
@@ -900,6 +889,30 @@ private function buildBasePath(bool $data, int $depth): string
900889
return $base;
901890
}
902891

892+
/**
893+
* Resolve the name of a non-SubExpression partial reference.
894+
*/
895+
private function resolvePartialName(PathExpression|StringLiteral|NumberLiteral $name): string
896+
{
897+
return $name instanceof PathExpression ? $name->original : $this->getLiteralKeyName($name);
898+
}
899+
900+
/**
901+
* Build a left-associative chain of runtime function calls over the given parts.
902+
* e.g. buildCallChain('f', '$in', ['a','b']) → "LR::f(LR::f($in, 'a'), 'b')"
903+
* An optional $extraArg is appended to every call's argument list.
904+
* @param string[] $parts
905+
*/
906+
private static function buildCallChain(string $fn, string $base, array $parts, string $extraArg = ''): string
907+
{
908+
$extra = $extraArg !== '' ? ", $extraArg" : '';
909+
$expr = $base;
910+
foreach ($parts as $part) {
911+
$expr = self::getRuntimeFunc($fn, "$expr, " . self::quote($part) . $extra);
912+
}
913+
return $expr;
914+
}
915+
903916
/**
904917
* Build a chained array-access string for the given path parts.
905918
* e.g. ['foo', 'bar'] → "['foo']['bar']"

src/Runtime.php

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,20 @@ public static function strictLookup(mixed $base, string $key, string $original):
131131
return $base[$key];
132132
}
133133

134+
/**
135+
* assumeObjects / strict-helper-arg key lookup: throw if $base is null (mirroring JS
136+
* TypeError for null/undefined property access); return null silently for a missing key on a
137+
* valid array (mirroring JS returning undefined for a missing object property); return null for
138+
* non-array non-null bases (mirroring JS returning undefined for property access on primitives).
139+
*/
140+
public static function nullCheck(mixed $base, string $key): mixed
141+
{
142+
if ($base === null) {
143+
throw new \ErrorException("Cannot access property \"$key\" on null");
144+
}
145+
return is_array($base) ? ($base[$key] ?? null) : null;
146+
}
147+
134148
/**
135149
* Build a RuntimeContext from raw render options and compile-time partial closures.
136150
*
@@ -176,9 +190,10 @@ public static function dv(mixed $v, mixed ...$args): mixed
176190
}
177191

178192
/**
179-
* Context variable lookup for knownHelpersOnly mode.
193+
* Context variable lookup without helper dispatch.
180194
* Looks up $name in $_this; if the value is a Closure, invokes it with $_this as context.
181-
* Skips helper dispatch (the compiler has already ruled out known helpers).
195+
* Used when helper dispatch is unnecessary: knownHelpersOnly mode (the compiler has already
196+
* ruled out known helpers), and inlined if/unless conditions on single-segment paths.
182197
*
183198
* @param mixed $_this current rendering context
184199
*/

tests/ErrorTest.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,11 @@ public static function renderErrorProvider(): array
6969
'data' => ['foo' => []],
7070
'expected' => '"foo.bar" not defined',
7171
],
72+
[
73+
'template' => '{{#if foo.bar}}bad{{else}}OK{{/if}}',
74+
'options' => new Options(strict: true),
75+
'expected' => 'Cannot access property "bar" on null',
76+
],
7277
[
7378
'template' => '{{foo}}',
7479
'options' => new Options(strict: true),

0 commit comments

Comments
 (0)