Skip to content

Commit 3860588

Browse files
committed
Fix ambiguous helper calls in strict mode
1 parent fc143fc commit 3860588

3 files changed

Lines changed: 67 additions & 45 deletions

File tree

src/Compiler.php

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ private function classifySexpr(?string $simpleName, array $params, ?Hash $hash):
227227
return SexprType::Helper;
228228
}
229229
if ($simpleName !== null) {
230-
return SexprType::Ambiguous;
230+
return $this->context->options->knownHelpersOnly ? SexprType::Simple : SexprType::Ambiguous;
231231
}
232232
return SexprType::Simple;
233233
}
@@ -485,16 +485,16 @@ private function PartialBlockStatement(PartialBlockStatement $statement): string
485485

486486
if ($partialName !== null && !$found) {
487487
// Register the block body as a fallback partial only if no runtime partial with this name exists yet.
488-
$parts[] = "(isset(\$cx->inlinePartials[$p]) || isset(\$cx->partials[$p]) ? '' : " . self::getRuntimeFunc('in', "\$cx, $p, $bodyClosure") . ')';
488+
$parts[] = "(isset(\$cx->inlinePartials[$p]) || isset(\$cx->partials[$p]) ? '' : "
489+
. self::getRuntimeFunc('in', "\$cx, $p, $bodyClosure") . ')';
489490
}
490491
$parts[] = self::getRuntimeFunc('p', "\$cx, $p, $vars, '', $bodyClosure");
491492
return implode('.', $parts);
492493
}
493494

494495
private function MustacheStatement(MustacheStatement $mustache): string
495496
{
496-
$raw = !$mustache->escaped || $this->context->options->noEscape;
497-
$fn = $raw ? 'raw' : 'encq';
497+
$fn = (!$mustache->escaped || $this->context->options->noEscape) ? 'raw' : 'encq';
498498
$path = $mustache->path;
499499

500500
// SubExpression path: {{(path args)}} — always a direct helper call result
@@ -512,27 +512,38 @@ private function MustacheStatement(MustacheStatement $mustache): string
512512
return self::getRuntimeFunc($fn, $call);
513513
}
514514

515-
if ($helperName !== null && $type === SexprType::Ambiguous && !$this->context->options->strict) {
515+
if ($type === SexprType::Ambiguous) {
516+
assert($helperName !== null);
516517
$escapedKey = self::quote($helperName);
517518
$isData = $path instanceof PathExpression && $path->data;
518-
if ($this->context->options->knownHelpersOnly) {
519-
$lookup = $isData ? "\$cx->data[$escapedKey] ?? null" : self::getRuntimeFunc('cv', "\$in, $escapedKey");
520-
return self::getRuntimeFunc($fn, $lookup);
521-
}
522519
$scope = $isData ? '$cx->data' : '$in';
523-
$hvArgs = "\$cx, $escapedKey, $scope" . ($this->context->options->assumeObjects ? ', true' : '');
520+
$hvArgs = "\$cx, $escapedKey, $scope";
521+
if ($this->context->options->assumeObjects) {
522+
$hvArgs .= ', true';
523+
} elseif ($this->context->options->strict) {
524+
$hvArgs .= ', false, true';
525+
}
524526
return self::getRuntimeFunc($fn, self::getRuntimeFunc('hv', $hvArgs));
525527
}
526528

527-
// Simple: direct path lookup
529+
// Simple: direct path lookup with lambda resolution.
530+
// For knownHelpersOnly bare identifiers (single-segment, non-data): use cv() to pass the
531+
// current context to any Closure, mirroring JS fn.call(context) where context is `this`.
532+
// For all other simple paths (multi-segment, scoped, depth, data): use dv() with zero args,
533+
// matching HBS.js container.lambda which also passes no positional arguments.
528534
if ($path instanceof PathExpression) {
529-
return self::getRuntimeFunc($fn, self::getRuntimeFunc('dv', $this->PathExpression($path)));
535+
if ($helperName !== null && !$path->data && $this->context->options->knownHelpersOnly) {
536+
$cvArgs = '$in, ' . self::quote($helperName) . ($this->context->options->strict ? ', true' : '');
537+
return self::getRuntimeFunc($fn, self::getRuntimeFunc('cv', $cvArgs));
538+
}
539+
$expression = $this->PathExpression($path);
540+
} else {
541+
// Literal in simple position: same lambda resolution as PathExpression above.
542+
$literalKey = $this->getLiteralKeyName($path);
543+
$expression = $this->compileModeAwareLookup('$in', [$literalKey], $literalKey);
530544
}
531545

532-
// Literal in simple/fallthrough position (strict, assumeObjects, or knownHelpersOnly):
533-
// compile as a direct context lookup using the normalized key name.
534-
$literalKey = $this->getLiteralKeyName($path);
535-
return self::getRuntimeFunc($fn, $this->compileModeAwareLookup('$in', [$literalKey], $literalKey));
546+
return self::getRuntimeFunc($fn, self::getRuntimeFunc('dv', $expression));
536547
}
537548

538549
// ── Expressions ─────────────────────────────────────────────────
@@ -547,26 +558,24 @@ private function SubExpression(SubExpression $expression): string
547558
return $this->compileHelperCall($helperName, $path, $expression->params, $expression->hash);
548559
}
549560

550-
private function PathExpression(PathExpression $expression): string
561+
private function PathExpression(PathExpression $path): string
551562
{
552-
$data = $expression->data;
553-
$depth = $expression->depth;
554-
$parts = $expression->parts;
563+
$data = $path->data;
564+
$depth = $path->depth;
555565

556566
// When the path head is a SubExpression (e.g. (helper).foo.bar), compile the
557567
// sub-expression as the base and use the string tail as the remaining key accesses.
558-
$hasSubExprHead = $expression->head instanceof SubExpression;
568+
$hasSubExprHead = $path->head instanceof SubExpression;
559569
if ($hasSubExprHead) {
560-
$base = '(' . $this->SubExpression($expression->head) . ')';
561-
$stringParts = $expression->tail;
570+
$base = '(' . $this->SubExpression($path->head) . ')';
571+
$stringParts = $path->tail;
562572
} else {
563573
$base = $this->buildBasePath($data, $depth);
564-
/** @var string[] $parts */
565-
$stringParts = $parts;
574+
/** @var string[] $stringParts */
575+
$stringParts = $path->parts;
566576
}
567577

568-
// `this` with no parts or empty parts
569-
if (($expression->this_ && !$parts) || !$stringParts) {
578+
if (!$stringParts) {
570579
return $base;
571580
}
572581

@@ -578,8 +587,8 @@ private function PathExpression(PathExpression $expression): string
578587
$isLength = end($stringParts) === 'length';
579588

580589
// Check block params (depth-0, non-data, non-scoped paths only, not SubExpression-headed)
581-
if (!$hasSubExprHead && !$data && $depth === 0 && !self::scopedId($expression)) {
582-
$bp = $this->lookupBlockParam($expression->head);
590+
if (!$hasSubExprHead && !$data && $depth === 0 && !self::scopedId($path)) {
591+
$bp = $this->lookupBlockParam($path->head);
583592
if ($bp !== null) {
584593
[$bpDepth, $bpIndex] = $bp;
585594
$bpBase = "\$blockParams[$bpDepth][$bpIndex]";
@@ -589,8 +598,8 @@ private function PathExpression(PathExpression $expression): string
589598
}
590599

591600
// Skip the block param name since it has been resolved to a $blockParams index.
592-
$keys = $isLength ? array_slice($expression->tail, 0, -1) : $expression->tail;
593-
$lookup = $this->compileModeAwareLookup($bpBase, $keys, $expression->original);
601+
$keys = $isLength ? array_slice($path->tail, 0, -1) : $path->tail;
602+
$lookup = $this->compileModeAwareLookup($bpBase, $keys, $path->original);
594603
return $isLength ? $this->buildLookupLength($lookup) : $lookup;
595604
}
596605
}
@@ -601,11 +610,11 @@ private function PathExpression(PathExpression $expression): string
601610
if ($isLength) {
602611
$partsExceptLength = array_slice($stringParts, 0, -1);
603612
return $this->buildLookupLength(
604-
$this->compileModeAwareLookup($base, $partsExceptLength, $expression->original),
613+
$this->compileModeAwareLookup($base, $partsExceptLength, $path->original),
605614
);
606615
}
607616

608-
return $this->compileModeAwareLookup($base, $stringParts, $expression->original);
617+
return $this->compileModeAwareLookup($base, $stringParts, $path->original);
609618
}
610619

611620
/**

src/Runtime.php

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ public static function createContext(mixed $context, array $options, array $comp
202202
}
203203

204204
/**
205-
* Invoke $v if it is a Closure; otherwise return $v as-is.
205+
* Invoke $v without arguments if it is a Closure; otherwise return $v as-is.
206206
*/
207207
public static function dv(mixed $v): mixed
208208
{
@@ -211,36 +211,42 @@ public static function dv(mixed $v): mixed
211211

212212
/**
213213
* Context variable lookup without helper dispatch.
214-
* Looks up $name in $_this; if the value is a Closure, invokes it with $_this as context.
215-
* Used when helper dispatch is unnecessary: knownHelpersOnly mode (the compiler has already
216-
* ruled out known helpers), and inlined if/unless conditions on single-segment paths.
214+
* Looks up $name in $_this; if the value is a Closure, invokes it with $_this as a positional arg
215+
* (PHP equivalent of JS fn.call(context), where context binds as `this` with no positional args).
216+
* When $strict is true, throws for missing keys.
217217
*
218218
* @param mixed $_this current rendering context
219219
*/
220-
public static function cv(mixed &$_this, string $name): mixed
220+
public static function cv(mixed &$_this, string $name, bool $strict = false): mixed
221221
{
222-
$v = $_this[$name] ?? null;
222+
$v = $strict ? static::strictLookup($_this, $name, $name) : ($_this[$name] ?? null);
223223
return $v instanceof Closure ? $v($_this) : $v;
224224
}
225225

226226
/**
227227
* Helper-or-variable lookup for bare {{identifier}} expressions.
228-
* Checks runtime helpers first, then context value, then helperMissing fallback.
228+
* Checks runtime helpers first, then context value.
229+
* In non-strict mode: falls back to helperMissing when the context value is null.
229230
* When $assumeObjects is true, uses nullCheck for context lookup (throws on null context).
231+
* When $strict is true, uses strictLookup after the helper check (throws for missing keys; no helperMissing fallback).
230232
*
231233
* @param mixed $_this current rendering context
232234
*/
233-
public static function hv(RuntimeContext $cx, string $name, mixed &$_this, bool $assumeObjects = false): mixed
235+
public static function hv(RuntimeContext $cx, string $name, mixed &$_this, bool $assumeObjects = false, bool $strict = false): mixed
234236
{
235237
$helper = $cx->helpers[$name] ?? null;
236238
if ($helper !== null) {
237239
return static::hbch($cx, $helper, $name, [], [], $_this);
238240
}
239-
$value = $assumeObjects ? static::nullCheck($_this, $name) : ($_this[$name] ?? null);
240-
if ($value === null || $value instanceof Closure) {
241-
return static::hbch($cx, $value ?? $cx->helpers['helperMissing'], $name, [], [], $_this);
241+
if ($strict) {
242+
$value = static::strictLookup($_this, $name, $name);
243+
} else {
244+
$value = $assumeObjects ? static::nullCheck($_this, $name) : ($_this[$name] ?? null);
245+
if ($value === null) {
246+
return static::hbch($cx, $cx->helpers['helperMissing'], $name, [], [], $_this);
247+
}
242248
}
243-
return $value;
249+
return $value instanceof Closure ? static::hbch($cx, $value, $name, [], [], $_this) : $value;
244250
}
245251

246252
/**

tests/RegressionTest.php

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2233,6 +2233,13 @@ public static function missingDataProvider(): array
22332233
'data' => ['foo' => null],
22342234
'expected' => '',
22352235
],
2236+
2237+
'strict mode should invoke ambiguous helper not present in context data' => [
2238+
'template' => '{{greeting}}',
2239+
'options' => new Options(strict: true),
2240+
'helpers' => ['greeting' => fn() => 'Hello'],
2241+
'expected' => 'Hello',
2242+
],
22362243
];
22372244
}
22382245

0 commit comments

Comments
 (0)