Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 85 additions & 0 deletions src/Core/Compiler/PhpSyntaxValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use PhpParser\Error;
use PhpParser\Parser as PhpAstParser;
use PhpParser\ParserFactory;
use PhpToken;
use Sugar\Core\Ast\ComponentNode;
use Sugar\Core\Ast\DirectiveNode;
use Sugar\Core\Ast\DocumentNode;
Expand Down Expand Up @@ -278,6 +279,12 @@ private function prepareRawPhpValidationCode(RawPhpNode $node): array
return ['', 0];
}

// Alternative syntax control structures (e.g. `if():` ... `endif;`) span multiple
// PHP blocks and cannot be validated as an isolated snippet.
if ($this->hasOpenAlternativeSyntax($normalizedCode)) {
return ['', 0];
}

$lineOffset = 0;
if (str_contains($normalizedCode, 'use')) {
$normalizedNode = new RawPhpNode($normalizedCode, $node->line, $node->column);
Expand All @@ -301,6 +308,84 @@ private function prepareRawPhpValidationCode(RawPhpNode $node): array
return [$normalizedCode, $lineOffset];
}

/**
* Detect whether a raw PHP snippet contains unmatched alternative control structure syntax.
*
* PHP alternative syntax (e.g. `if (): ... endif;`) can span multiple separate PHP
* blocks inside a template. Validating such a fragment in isolation would always fail
* because the opener block lacks its matching `endXxx` counterpart (or vice-versa).
*
* The method tokenises the snippet with `PhpToken::tokenize()` and tracks two categories:
* - Openers (`T_IF`, `T_FOR`, `T_FOREACH`, `T_WHILE`, `T_SWITCH`, `T_DECLARE`) followed
* by `:` always increment the nesting depth.
* - Continuations (`T_ELSE`, `T_ELSEIF`) and inner tokens (`T_CASE`, `T_DEFAULT`)
* followed by `:` only increment depth when currently at depth 0 (orphaned snippets);
* when depth > 0 they are continuations of a block already opened in this snippet and
* do not affect the balance.
* - Closers (`T_ENDIF`, `T_ENDFOR`, `T_ENDFOREACH`, `T_ENDWHILE`, `T_ENDSWITCH`,
* `T_ENDDECLARE`) decrement the depth.
* A non-zero balance means the snippet is part of a multi-block construct and should not
* be validated in isolation.
*
* @param string $code Stripped PHP code (without open/close tags)
* @return bool True when the snippet has unbalanced alternative syntax
*/
private function hasOpenAlternativeSyntax(string $code): bool
{
$tokens = PhpToken::tokenize('<?php ' . $code);
$depth = 0;
$expectColon = false;
$orphanColon = false;
$parenDepth = 0;

$openerTokens = [T_IF, T_FOR, T_FOREACH, T_WHILE, T_SWITCH, T_DECLARE];
$continuationTokens = [T_ELSE, T_ELSEIF, T_CASE, T_DEFAULT];
$endTokens = [T_ENDIF, T_ENDFOR, T_ENDFOREACH, T_ENDWHILE, T_ENDSWITCH, T_ENDDECLARE];

foreach ($tokens as $token) {
if ($token->isIgnorable()) {
continue;
}

if (in_array($token->id, $endTokens, true)) {
$depth--;
$expectColon = false;
$orphanColon = false;
} elseif (in_array($token->id, $openerTokens, true)) {
$expectColon = true;
$orphanColon = false;
$parenDepth = 0;
} elseif (in_array($token->id, $continuationTokens, true)) {
// When depth > 0 this is a continuation inside a block opened in this snippet;
// do not change depth so that the matching closer brings it back to zero.
// When depth === 0 it is an orphaned clause in a multi-block template.
if ($depth === 0) {
$expectColon = true;
$orphanColon = true;
}

$parenDepth = 0;
} elseif ($token->text === '(') {
$parenDepth++;
} elseif ($token->text === ')') {
$parenDepth--;
} elseif ($token->text === ':' && $parenDepth === 0 && $expectColon) {
if ($orphanColon) {
return true;
}

$depth++;
$expectColon = false;
$orphanColon = false;
} elseif ($token->text === '{') {
$expectColon = false;
$orphanColon = false;
}
}

return $depth !== 0;
}

/**
* Strip optional PHP open/close tags from raw snippet content.
*/
Expand Down
2 changes: 2 additions & 0 deletions tests/Helper/Trait/CompilerTestTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ protected function setUpCompilerWithStringLoader(
bool $withDefaultDirectives = true,
bool $absolutePathsOnly = false,
array $customPasses = [],
bool $phpSyntaxValidationEnabled = false,
): void {
$loaderConfig = $config ?? new SugarConfig();

Expand Down Expand Up @@ -141,6 +142,7 @@ protected function setUpCompilerWithStringLoader(
templateLoader: $this->templateLoader,
config: $config,
customPasses: $customPasses,
phpSyntaxValidationEnabled: $phpSyntaxValidationEnabled,
);

// Registry property is initialized before pass wiring.
Expand Down
69 changes: 69 additions & 0 deletions tests/Integration/PhpNormalizationIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,73 @@ public function testHeredocBodyWithCloseTagCompilesCorrectly(): void

$this->assertStringContainsString('some ?&gt; content', $output);
}

public function testAlternativeSyntaxIfBlockCompilesAndRendersInDebugMode(): void
{
$this->setUpCompilerWithStringLoader(
templates: [],
config: new SugarConfig(),
phpSyntaxValidationEnabled: true,
);

$template = <<<'SUGAR'
<?php if(true): ?>
<div>hello</div>
<?php endif; ?>
SUGAR;

// debug: true was previously triggering a false "Invalid PHP block" error
$compiled = $this->compiler->compile($template, 'alt-syntax.sugar.php', debug: true);
$output = $this->executeTemplate($compiled);

$this->assertStringContainsString('<div>hello</div>', $output);
}

public function testAlternativeSyntaxForeachBlockCompilesAndRendersInDebugMode(): void
{
$this->setUpCompilerWithStringLoader(
templates: [],
config: new SugarConfig(),
phpSyntaxValidationEnabled: true,
);

$template = <<<'SUGAR'
<ul>
<?php foreach ($items as $item): ?>
<li><?= $item ?></li>
<?php endforeach; ?>
</ul>
SUGAR;

$compiled = $this->compiler->compile($template, 'alt-foreach.sugar.php', debug: true);
$output = $this->executeTemplate($compiled, ['items' => ['Apple', 'Banana']]);

$this->assertStringContainsString('<li>Apple</li>', $output);
$this->assertStringContainsString('<li>Banana</li>', $output);
}

public function testAlternativeSyntaxElseBlockCompilesAndRendersInDebugMode(): void
{
$this->setUpCompilerWithStringLoader(
templates: [],
config: new SugarConfig(),
phpSyntaxValidationEnabled: true,
);

$template = <<<'SUGAR'
<?php if($show): ?>
<span>visible</span>
<?php else: ?>
<span>hidden</span>
<?php endif; ?>
SUGAR;

$compiled = $this->compiler->compile($template, 'alt-else.sugar.php', debug: true);

$outputVisible = $this->executeTemplate($compiled, ['show' => true]);
$this->assertStringContainsString('<span>visible</span>', $outputVisible);

$outputHidden = $this->executeTemplate($compiled, ['show' => false]);
$this->assertStringContainsString('<span>hidden</span>', $outputHidden);
}
}
Loading