From 699a8bf2daf1b7f28c034c68d0b69d499ee53a0e Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <guy.sartorelli@silverstripe.com>
Date: Fri, 17 May 2024 15:17:02 +1200
Subject: [PATCH] FIX Loop over current scope when no argument passed to loop
 block

---
 src/View/SSTemplateParser.peg   |  6 +++---
 src/View/SSTemplateParser.php   | 13 +++++++++----
 tests/php/View/SSViewerTest.php |  9 +++++++++
 3 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/View/SSTemplateParser.peg b/src/View/SSTemplateParser.peg
index 2061c1879..0f15460f9 100644
--- a/src/View/SSTemplateParser.peg
+++ b/src/View/SSTemplateParser.peg
@@ -1031,13 +1031,13 @@ class SSTemplateParser extends Parser implements TemplateParser
     function ClosedBlock_Handle_Loop(&$res)
     {
         if ($res['ArgumentCount'] > 1) {
-            throw new SSTemplateParseException('Either no or too many arguments in control block. Must be one ' .
-                'argument only.', $this);
+            throw new SSTemplateParseException('Too many arguments in control block. Must be one or no' .
+                'arguments only.', $this);
         }
 
         //loop without arguments loops on the current scope
         if ($res['ArgumentCount'] == 0) {
-            $on = '$scope->obj(\'Up\', null)->obj(\'Foo\', null)';
+            $on = '$scope->locally()->obj(\'Me\', null, true)';
         } else {    //loop in the normal way
             $arg = $res['Arguments'][0];
             if ($arg['ArgumentMode'] == 'string') {
diff --git a/src/View/SSTemplateParser.php b/src/View/SSTemplateParser.php
index db9f0a6f3..9ca62eaa5 100644
--- a/src/View/SSTemplateParser.php
+++ b/src/View/SSTemplateParser.php
@@ -1886,6 +1886,8 @@ class SSTemplateParser extends Parser implements TemplateParser
             $res['php'] .= '((bool)'.$sub['php'].')';
         } else {
             $php = ($sub['ArgumentMode'] == 'default' ? $sub['lookup_php'] : $sub['php']);
+            // TODO: kinda hacky - maybe we need a way to pass state down the parse chain so
+            // Lookup_LastLookupStep and Argument_BareWord can produce hasValue instead of XML_val
             $res['php'] .= str_replace('$$FINAL', 'hasValue', $php ?? '');
         }
     }
@@ -4257,13 +4259,13 @@ class SSTemplateParser extends Parser implements TemplateParser
     function ClosedBlock_Handle_Loop(&$res)
     {
         if ($res['ArgumentCount'] > 1) {
-            throw new SSTemplateParseException('Either no or too many arguments in control block. Must be one ' .
-                'argument only.', $this);
+            throw new SSTemplateParseException('Too many arguments in control block. Must be one or no' .
+                'arguments only.', $this);
         }
 
         //loop without arguments loops on the current scope
         if ($res['ArgumentCount'] == 0) {
-            $on = '$scope->obj(\'Up\', null)->obj(\'Foo\', null)';
+            $on = '$scope->locally()->obj(\'Me\', null, true)';
         } else {    //loop in the normal way
             $arg = $res['Arguments'][0];
             if ($arg['ArgumentMode'] == 'string') {
@@ -5290,6 +5292,8 @@ class SSTemplateParser extends Parser implements TemplateParser
         $text = stripslashes($text ?? '');
         $text = addcslashes($text ?? '', '\'\\');
 
+        // TODO: This is pretty ugly & gets applied on all files not just html. I wonder if we can make this
+        // non-dynamically calculated
         $code = <<<'EOC'
 (\SilverStripe\View\SSViewer::getRewriteHashLinksDefault()
     ? \SilverStripe\Core\Convert::raw2att( preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI'] ) )
@@ -5328,7 +5332,8 @@ EOC;
 
             $this->includeDebuggingComments = $includeDebuggingComments;
 
-            // Ignore UTF8 BOM at beginning of string.
+            // Ignore UTF8 BOM at beginning of string. TODO: Confirm this is needed, make sure SSViewer handles UTF
+            // (and other encodings) properly
             if (substr($string ?? '', 0, 3) == pack("CCC", 0xef, 0xbb, 0xbf)) {
                 $this->pos = 3;
             }
diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php
index e5307a7c1..b80b1c809 100644
--- a/tests/php/View/SSViewerTest.php
+++ b/tests/php/View/SSViewerTest.php
@@ -500,6 +500,15 @@ SS;
         );
     }
 
+    public function testCurrentScopeLoop(): void
+    {
+        $data = new ArrayList([['Val' => 'one'], ['Val' => 'two'], ['Val' => 'three']]);
+        $this->assertEqualIgnoringWhitespace(
+            'one two three',
+            $this->render('<% loop %>$Val<% end_loop %>', $data)
+        );
+    }
+
     public function testCurrentScopeLoopWith()
     {
         // Data to run the loop tests on - one sequence of three items, each with a subitem