Skip to content

Commit 8a88577

Browse files
author
Enno Woortmann
committed
Patch foreach for mullable values guarded by ifs
1 parent 9d45ebf commit 8a88577

2 files changed

Lines changed: 91 additions & 5 deletions

File tree

src/Render.php

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,18 @@ protected function resolveLoops(string $template, array $variables): string
131131
'/\{%\s*foreach(?<index>-[\d]+-[\d]+-)\s+' . self::REGEX_VARIABLE . '\s+as\s+((?<key>\w+)\s*,\s*)?(?<value>\w+)\s*%\}' .
132132
'(?<body>.+)' .
133133
'\{%\s*endforeach\k<index>\s*%\}/si',
134-
function (array $matches) use ($variables): string {
134+
function (array $matches) use ($variables, $template): string {
135+
// If this foreach is preceded by an unclosed {% if %} open tag in the template, it is nested inside a
136+
// conditional that hasn't been evaluated yet. Return the original match unchanged so that
137+
// resolveConditionals can strip the enclosing if-branch first, then call resolveLoops on the result.
138+
$foreachPos = strpos($template, $matches[0]);
139+
$prefix = substr($template, 0, $foreachPos);
140+
$openIfCount = preg_match_all('/\{%\s*if-[\d]+-[\d]+-/i', $prefix);
141+
$closeIfCount = preg_match_all('/\{%\s*endif-[\d]+-[\d]+-/i', $prefix);
142+
if ($openIfCount > $closeIfCount) {
143+
return $matches[0];
144+
}
145+
135146
$output = '';
136147

137148
foreach ($this->getValue($matches, $variables) as $key => $value) {
@@ -196,11 +207,9 @@ function (array $matches) use ($variables): string {
196207
$orBranches[] = $andBranchTrue;
197208
}
198209

199-
if (in_array(true, $orBranches)) {
200-
return $conditionalBody[0];
201-
}
210+
$branch = in_array(true, $orBranches) ? $conditionalBody[0] : ($conditionalBody[1] ?? '');
202211

203-
return $conditionalBody[1] ?? '';
212+
return $this->resolveLoops($branch, $variables);
204213
},
205214
$template,
206215
-1,

tests/RenderTest.php

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,83 @@ public function testNotCallableMethod(): void
417417
$this->render->renderTemplateString("{{ unknownMethod('abc') }}");
418418
}
419419

420+
/**
421+
* A {% foreach %} whose iterable is a method call on a nullable variable must not be evaluated when
422+
* it is wrapped in a falsy {% if %} guard — previously the foreach iterable was resolved before the
423+
* enclosing if, causing a fatal "Trying to call ... on non-object" error.
424+
*/
425+
public function testForeachInsideFalsyIfIsNotEvaluated(): void
426+
{
427+
$template = '{% if items %}{% foreach items.getCategories() as cat %}{{ cat }}{% endforeach %}{% endif %}';
428+
429+
$this->assertSame(
430+
'',
431+
$this->render->renderTemplateString($template, ['items' => null]),
432+
);
433+
}
434+
435+
/**
436+
* When the {% if %} guard is truthy the {% foreach %} inside must still execute normally.
437+
*/
438+
public function testForeachInsideTruthyIfIsEvaluated(): void
439+
{
440+
$template = '{% if items %}{% foreach items.getCategories() as cat %}{{ cat }},{% endforeach %}{% endif %}';
441+
442+
$this->assertSame(
443+
'Oak,Birch,',
444+
$this->render->renderTemplateString(
445+
$template,
446+
['items' => new Product('Wood', true, ['Oak', 'Birch'])],
447+
),
448+
);
449+
}
450+
451+
/**
452+
* A {% if %} inside a {% foreach %} that uses the loop variable in its condition must still work
453+
* correctly — the loop variable must be in scope when the conditional is evaluated.
454+
*/
455+
public function testIfInsideForeachCanUseLoopVariable(): void
456+
{
457+
$template = '{% foreach items as item %}{% if item.isVisible() %}{{ item.getTitle() }}{% endif %}{% endforeach %}';
458+
459+
$products = [
460+
new Product('Hammer', true),
461+
new Product('Nails', false),
462+
new Product('Wood', true),
463+
];
464+
465+
$this->assertSame(
466+
'HammerWood',
467+
$this->render->renderTemplateString($template, ['items' => $products]),
468+
);
469+
}
470+
471+
/**
472+
* A {% foreach %} inside the true-branch of a top-level {% if %}, and a sibling {% foreach %} outside
473+
* any conditional, must both resolve correctly in a single template.
474+
*/
475+
public function testForeachInsideIfAlongsideSiblingForeach(): void
476+
{
477+
$template =
478+
'{% if extra %}{% foreach extra.getCategories() as cat %}[{{ cat }}]{% endforeach %}{% endif %}' .
479+
'{% foreach items as item %}{{ item.getTitle() }}{% endforeach %}';
480+
481+
$products = [new Product('Hammer', true), new Product('Wood', true)];
482+
483+
$this->assertSame(
484+
'HammerWood',
485+
$this->render->renderTemplateString($template, ['items' => $products, 'extra' => null]),
486+
);
487+
488+
$this->assertSame(
489+
'[Oak][Birch]HammerWood',
490+
$this->render->renderTemplateString(
491+
$template,
492+
['items' => $products, 'extra' => new Product('Extra', true, ['Oak', 'Birch'])],
493+
),
494+
);
495+
}
496+
420497
public function propertyDataProvider(): array
421498
{
422499
return [

0 commit comments

Comments
 (0)