FIX Passing 0 as first argument breaks template

Replace !empty with explicit string test Added test for falseish first parameter arguments
silverstripe · Jun 9, 2023 · face943 · face943
1 parent fd57f06
commit face943
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 2 deletions.
diff --git a/src/View/SSTemplateParser.peg b/src/View/SSTemplateParser.peg
@@ -242,7 +242,7 @@ class SSTemplateParser extends Parser implements TemplateParser
      */
     function CallArguments_Argument(&$res, $sub)
     {
-        if (!empty($res['php'])) {
+        if ($res['php'] !== '') {
             $res['php'] .= ', ';
         }
 

diff --git a/src/View/SSTemplateParser.php b/src/View/SSTemplateParser.php
@@ -567,7 +567,7 @@ function match_CallArguments ($stack = array()) {
      */
     function CallArguments_Argument(&$res, $sub)
     {
-        if (!empty($res['php'])) {
+        if ($res['php'] !== '') {
             $res['php'] .= ', ';
         }
 

diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php
@@ -552,6 +552,40 @@ public function testCurrentScopeLoopWith()
         $this->assertEquals("SubKid1SubKid2Number6", $result, "Loop in current scope works");
     }
 
+    public function provideArgumentTypes()
+    {
+        return [
+            [
+                'arg1:0,arg2:"string",arg3:true',
+                '$methodWithTypedArguments(0, "string", true).RAW',
+            ],
+            [
+                'arg1:false,arg2:"string",arg3:true',
+                '$methodWithTypedArguments(false, "string", true).RAW',
+            ],
+            [
+                'arg1:null,arg2:"string",arg3:true',
+                '$methodWithTypedArguments(null, "string", true).RAW',
+            ],
+            [
+                'arg1:"",arg2:"string",arg3:true',
+                '$methodWithTypedArguments("", "string", true).RAW',
+            ],
+            [
+                'arg1:0,arg2:1,arg3:2',
+                '$methodWithTypedArguments(0, 1, 2).RAW',
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider provideArgumentTypes
+     */
+    public function testArgumentTypes(string $expected, string $template)
+    {
+        $this->assertEquals($expected, $this->render($template, new TestViewableData()));
+    }
+
     public function testObjectDotArguments()
     {
         $this->assertEquals(

diff --git a/tests/php/View/SSViewerTest/TestViewableData.php b/tests/php/View/SSViewerTest/TestViewableData.php
@@ -29,6 +29,11 @@ public function methodWithTwoArguments($arg1, $arg2)
         return "arg1:{$arg1},arg2:{$arg2}";
     }
 
+    public function methodWithTypedArguments($arg1, $arg2, $arg3)
+    {
+        return 'arg1:' . json_encode($arg1) . ',arg2:' . json_encode($arg2) . ',arg3:' . json_encode($arg3);
+    }
+
     public function Type($arg)
     {
         return gettype($arg) . ':' . $arg;