From ae10081123bba9942fd82a89a1af53caf81aca70 Mon Sep 17 00:00:00 2001 From: Jasper Smet Date: Wed, 11 Mar 2026 08:20:27 +0100 Subject: [PATCH 1/2] Fix: Allow PHP alternative syntax control structures in debug mode --- src/Core/Compiler/PhpSyntaxValidator.php | 67 ++++++++ .../PhpNormalizationIntegrationTest.php | 66 ++++++++ .../Core/Compiler/PhpSyntaxValidatorTest.php | 153 ++++++++++++++++++ 3 files changed, 286 insertions(+) diff --git a/src/Core/Compiler/PhpSyntaxValidator.php b/src/Core/Compiler/PhpSyntaxValidator.php index a2aed2b..6adadc3 100644 --- a/src/Core/Compiler/PhpSyntaxValidator.php +++ b/src/Core/Compiler/PhpSyntaxValidator.php @@ -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; @@ -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); @@ -301,6 +308,66 @@ 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 counts alternative + * syntax openers (control structures followed by `:`) against closers (`T_ENDIF`, + * `T_ENDFOREACH`, `T_ENDFOR`, `T_ENDWHILE`, `T_ENDSWITCH`). A non-zero balance + * means the snippet is part of a multi-block construct. + * + * @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('id, $endTokens, true)) { + $depth--; + $lastWasControlWithParen = false; + $lastWasElse = false; + } elseif (in_array($token->id, $controlTokens, true)) { + $lastWasControlWithParen = true; + $lastWasElse = false; + $parenDepth = 0; + } elseif ($token->id === T_ELSE) { + $lastWasElse = true; + $lastWasControlWithParen = false; + $parenDepth = 0; + } elseif ($token->text === '(') { + $parenDepth++; + } elseif ($token->text === ')') { + $parenDepth--; + } elseif ($token->text === ':') { + if ($lastWasControlWithParen && $parenDepth === 0) { + $depth++; + $lastWasControlWithParen = false; + } elseif ($lastWasElse) { + $depth++; + $lastWasElse = false; + } + } elseif ($token->text === '{') { + $lastWasControlWithParen = false; + $lastWasElse = false; + } + } + + return $depth !== 0; + } + /** * Strip optional PHP open/close tags from raw snippet content. */ diff --git a/tests/Integration/PhpNormalizationIntegrationTest.php b/tests/Integration/PhpNormalizationIntegrationTest.php index 701547e..ce49f47 100644 --- a/tests/Integration/PhpNormalizationIntegrationTest.php +++ b/tests/Integration/PhpNormalizationIntegrationTest.php @@ -320,4 +320,70 @@ public function testHeredocBodyWithCloseTagCompilesCorrectly(): void $this->assertStringContainsString('some ?> content', $output); } + + public function testAlternativeSyntaxIfBlockCompilesAndRendersInDebugMode(): void + { + $this->setUpCompilerWithStringLoader( + templates: [], + config: new SugarConfig(), + ); + + $template = <<<'SUGAR' + +
hello
+ +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('
hello
', $output); + } + + public function testAlternativeSyntaxForeachBlockCompilesAndRendersInDebugMode(): void + { + $this->setUpCompilerWithStringLoader( + templates: [], + config: new SugarConfig(), + ); + + $template = <<<'SUGAR' + +SUGAR; + + $compiled = $this->compiler->compile($template, 'alt-foreach.sugar.php', debug: true); + $output = $this->executeTemplate($compiled, ['items' => ['Apple', 'Banana']]); + + $this->assertStringContainsString('
  • Apple
    • ', $output); + $this->assertStringContainsString('
  • Banana
    • ', $output); + } + + public function testAlternativeSyntaxElseBlockCompilesAndRendersInDebugMode(): void + { + $this->setUpCompilerWithStringLoader( + templates: [], + config: new SugarConfig(), + ); + + $template = <<<'SUGAR' + + visible + + hidden + +SUGAR; + + $compiled = $this->compiler->compile($template, 'alt-else.sugar.php', debug: true); + + $outputVisible = $this->executeTemplate($compiled, ['show' => true]); + $this->assertStringContainsString('visible', $outputVisible); + + $outputHidden = $this->executeTemplate($compiled, ['show' => false]); + $this->assertStringContainsString('hidden', $outputHidden); + } } diff --git a/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php b/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php index cf6e770..6398c18 100644 --- a/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php +++ b/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php @@ -561,6 +561,159 @@ public function testTemplateSegmentsValidateMalformedPhpImportNode(): void } } + public function testTemplateSegmentsAllowAlternativeSyntaxIfOpener(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // Opener block: `if(true):` – has no matching endif in this snippet + $document = new DocumentNode([ + new RawPhpNode('if(true):', 1, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsAllowAlternativeSyntaxIfCloser(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // Closer block: `endif;` – has no matching opener in this snippet + $document = new DocumentNode([ + new RawPhpNode('endif;', 5, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsAllowAlternativeSyntaxForeachBlocks(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + $document = new DocumentNode([ + new RawPhpNode('foreach ($items as $item):', 1, 1), + new RawPhpNode('endforeach;', 5, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsAllowAlternativeSyntaxWithBodyInOpener(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // Opener block that also contains body code before the close tag + $document = new DocumentNode([ + new RawPhpNode("if (\$show):\n \$label = 'hello';", 1, 1), + new RawPhpNode('endif;', 5, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsAllowAlternativeSyntaxWhileAndElse(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + $document = new DocumentNode([ + new RawPhpNode('while ($i < 10):', 1, 1), + new RawPhpNode('endwhile;', 3, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsStillValidateRegularPhpBlockErrors(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // Regular (brace-based) but syntactically broken code must still be caught + $document = new DocumentNode([ + new RawPhpNode('$x = ;', 1, 1), + ]); + + $validator = new PhpSyntaxValidator(); + + $this->expectException(SyntaxException::class); + $this->expectExceptionMessage('Invalid PHP block'); + + $validator->templateSegments($document, $context); + } + /** * @param \PhpParser\Parser|null $parser Parser instance to inject */ From 8e9620725eea10593cc626a41bcbc97c2f949936 Mon Sep 17 00:00:00 2001 From: Jasper Smet Date: Wed, 11 Mar 2026 08:34:45 +0100 Subject: [PATCH 2/2] address copilot reviews --- src/Core/Compiler/PhpSyntaxValidator.php | 68 +++++++----- tests/Helper/Trait/CompilerTestTrait.php | 2 + .../PhpNormalizationIntegrationTest.php | 3 + .../Core/Compiler/PhpSyntaxValidatorTest.php | 103 +++++++++++++++++- 4 files changed, 150 insertions(+), 26 deletions(-) diff --git a/src/Core/Compiler/PhpSyntaxValidator.php b/src/Core/Compiler/PhpSyntaxValidator.php index 6adadc3..39e0b1d 100644 --- a/src/Core/Compiler/PhpSyntaxValidator.php +++ b/src/Core/Compiler/PhpSyntaxValidator.php @@ -315,10 +315,17 @@ private function prepareRawPhpValidationCode(RawPhpNode $node): array * 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 counts alternative - * syntax openers (control structures followed by `:`) against closers (`T_ENDIF`, - * `T_ENDFOREACH`, `T_ENDFOR`, `T_ENDWHILE`, `T_ENDSWITCH`). A non-zero balance - * means the snippet is part of a multi-block construct. + * 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 @@ -327,41 +334,52 @@ private function hasOpenAlternativeSyntax(string $code): bool { $tokens = PhpToken::tokenize('isIgnorable()) { + continue; + } + if (in_array($token->id, $endTokens, true)) { $depth--; - $lastWasControlWithParen = false; - $lastWasElse = false; - } elseif (in_array($token->id, $controlTokens, true)) { - $lastWasControlWithParen = true; - $lastWasElse = false; + $expectColon = false; + $orphanColon = false; + } elseif (in_array($token->id, $openerTokens, true)) { + $expectColon = true; + $orphanColon = false; $parenDepth = 0; - } elseif ($token->id === T_ELSE) { - $lastWasElse = true; - $lastWasControlWithParen = false; + } 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 === ':') { - if ($lastWasControlWithParen && $parenDepth === 0) { - $depth++; - $lastWasControlWithParen = false; - } elseif ($lastWasElse) { - $depth++; - $lastWasElse = false; + } elseif ($token->text === ':' && $parenDepth === 0 && $expectColon) { + if ($orphanColon) { + return true; } + + $depth++; + $expectColon = false; + $orphanColon = false; } elseif ($token->text === '{') { - $lastWasControlWithParen = false; - $lastWasElse = false; + $expectColon = false; + $orphanColon = false; } } diff --git a/tests/Helper/Trait/CompilerTestTrait.php b/tests/Helper/Trait/CompilerTestTrait.php index f46a07b..4f32098 100644 --- a/tests/Helper/Trait/CompilerTestTrait.php +++ b/tests/Helper/Trait/CompilerTestTrait.php @@ -107,6 +107,7 @@ protected function setUpCompilerWithStringLoader( bool $withDefaultDirectives = true, bool $absolutePathsOnly = false, array $customPasses = [], + bool $phpSyntaxValidationEnabled = false, ): void { $loaderConfig = $config ?? new SugarConfig(); @@ -141,6 +142,7 @@ protected function setUpCompilerWithStringLoader( templateLoader: $this->templateLoader, config: $config, customPasses: $customPasses, + phpSyntaxValidationEnabled: $phpSyntaxValidationEnabled, ); // Registry property is initialized before pass wiring. diff --git a/tests/Integration/PhpNormalizationIntegrationTest.php b/tests/Integration/PhpNormalizationIntegrationTest.php index ce49f47..68a6758 100644 --- a/tests/Integration/PhpNormalizationIntegrationTest.php +++ b/tests/Integration/PhpNormalizationIntegrationTest.php @@ -326,6 +326,7 @@ public function testAlternativeSyntaxIfBlockCompilesAndRendersInDebugMode(): voi $this->setUpCompilerWithStringLoader( templates: [], config: new SugarConfig(), + phpSyntaxValidationEnabled: true, ); $template = <<<'SUGAR' @@ -346,6 +347,7 @@ public function testAlternativeSyntaxForeachBlockCompilesAndRendersInDebugMode() $this->setUpCompilerWithStringLoader( templates: [], config: new SugarConfig(), + phpSyntaxValidationEnabled: true, ); $template = <<<'SUGAR' @@ -368,6 +370,7 @@ public function testAlternativeSyntaxElseBlockCompilesAndRendersInDebugMode(): v $this->setUpCompilerWithStringLoader( templates: [], config: new SugarConfig(), + phpSyntaxValidationEnabled: true, ); $template = <<<'SUGAR' diff --git a/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php b/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php index 6398c18..79ea149 100644 --- a/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php +++ b/tests/Unit/Core/Compiler/PhpSyntaxValidatorTest.php @@ -662,7 +662,7 @@ public function testTemplateSegmentsAllowAlternativeSyntaxWithBodyInOpener(): vo $this->addToAssertionCount(1); } - public function testTemplateSegmentsAllowAlternativeSyntaxWhileAndElse(): void + public function testTemplateSegmentsAllowAlternativeSyntaxWhileBlock(): void { if (!$this->hasPhpParserSupport()) { $this->expectNotToPerformAssertions(); @@ -714,6 +714,107 @@ public function testTemplateSegmentsStillValidateRegularPhpBlockErrors(): void $validator->templateSegments($document, $context); } + public function testTemplateSegmentsValidateBalancedIfElseEndifInSingleBlock(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // A complete if/else/endif in one block is balanced — validation must run, not be skipped. + // This verifies that else: inside an already-open block does not inflate the depth. + $document = new DocumentNode([ + new RawPhpNode("if (\$x):\n \$a = 1;\nelse:\n \$a = 2;\nendif;", 1, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsAllowOrphanedElseColon(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // Standalone else: is an orphaned continuation block in a multi-block template + $document = new DocumentNode([ + new RawPhpNode('else:', 3, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsAllowOrphanedCaseBlock(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // Standalone case label is an orphaned inner block in a multi-block switch + $document = new DocumentNode([ + new RawPhpNode('case 1:', 2, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + + public function testTemplateSegmentsValidateBalancedSwitchInSingleBlock(): void + { + if (!$this->hasPhpParserSupport()) { + $this->expectNotToPerformAssertions(); + + return; + } + + $context = new CompilationContext( + templatePath: '@app/pages/home.sugar.php', + source: '', + debug: true, + ); + + // A complete switch/endswitch with case labels in one block is balanced + $document = new DocumentNode([ + new RawPhpNode("switch (\$x):\ncase 1:\n \$r = 'a';\n break;\ndefault:\n \$r = 'b';\nendswitch;", 1, 1), + ]); + + $validator = new PhpSyntaxValidator(); + $validator->templateSegments($document, $context); + + $this->addToAssertionCount(1); + } + /** * @param \PhpParser\Parser|null $parser Parser instance to inject */