Fix modifier default implementation and improve unit tests

Fixes #336

.gitignore

@@ -11,3 +11,4 @@ utilies/*.php
 phpunit*
 vendor/*
 composer.lock
+/composer.phar

CHANGELOG.md

@@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased]
 
 ### Changed
+- default modifier uses empty instead of @ error suppression modifier when testing a variable https://github.com/smarty-php/smarty/issues/336
 - modifier escape now triggers a E_USER_NOTICE when an unsupported escape type is used https://github.com/smarty-php/smarty/pull/649
 
 ## [3.1.39] - 2021-02-17

libs/plugins/modifiercompiler.default.php

@@ -18,7 +18,7 @@
  *
  * @return string with compiled code
  */
-function smarty_modifiercompiler_default($params)
+function smarty_modifiercompiler_default($params, Smarty_Internal_TemplateCompilerBase $compiler)
 {
     $output = $params[ 0 ];
     if (!isset($params[ 1 ])) {
@@ -26,7 +26,13 @@ function smarty_modifiercompiler_default($params)
     }
     array_shift($params);
     foreach ($params as $param) {
-        $output = '(($tmp = @' . $output . ')===null||$tmp===\'\' ? ' . $param . ' : $tmp)';
+
+        if ($compiler->syntaxMatchesVariable($output)) {
+            $output = '(!isset(' . $output . ') || ' . $output . ' === \'\' ? ' . $param . ' : ' . $output . ')';
+        } else {
+            $output = '(($tmp = ' . $output . ')===null||$tmp===\'\' ? ' . $param . ' : $tmp)';
+        }
+
     }
     return $output;
 }

libs/sysplugins/smarty_internal_templatecompilerbase.php

@@ -674,7 +674,7 @@ abstract class Smarty_Internal_TemplateCompilerBase
 	 * @param $string
 	 * @return bool
 	 */
-	private function syntaxMatchesVariable($string) {
+	public function syntaxMatchesVariable($string) {
     	static $regex_pattern = '/^\$[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*((->)[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*|\[.*]*\])*$/';
     	return 1 === preg_match($regex_pattern, trim($string));
     }

tests/PHPUnit_Smarty.php

@@ -177,7 +177,6 @@ class PHPUnit_Smarty extends PHPUnit_Framework_TestCase
                 $this->smartyBC->setCacheDir(dirname(__FILE__) . '/cache');
             }
         }
-        $smarty = $this->getSmartyObj();
     }
 
     /**

tests/UnitTests/A_Core/MuteExpectedErrors/MuteExpectedErrorsTest.php

@@ -64,10 +64,10 @@ class MuteExpectedErrorsTest extends PHPUnit_Smarty
         $this->smarty->clearCompiledTemplate('default.tpl');
         $this->smarty->fetch('default.tpl');
 
-        $this->assertEquals(Smarty::$_IS_WINDOWS ? 2 : 2, count($this->_errors));
+        $this->assertEquals(0, count($this->_errors));
 
         @filemtime('ckxladanwijicajscaslyxck');
-        $this->assertEquals(Smarty::$_IS_WINDOWS ? 3 : 3, count($this->_errors));
+        $this->assertEquals(1, count($this->_errors));
 
         restore_error_handler();
     }
@@ -112,11 +112,11 @@ class MuteExpectedErrorsTest extends PHPUnit_Smarty
         $this->smarty->clearCompiledTemplate('default.tpl');
         $this->smarty->fetch('default.tpl');
 
-        $this->assertEquals(Smarty::$_IS_WINDOWS ? 2 : 2, count($this->_errors));
+        $this->assertEquals(0, count($this->_errors));
 
         @filemtime('ckxladanwijicajscaslyxck');
         $error = array(__FILE__ . ' line ' . (__LINE__ - 1));
-        $this->assertEquals(Smarty::$_IS_WINDOWS ? 3 : 3, count($this->_errors));
+        $this->assertEquals(1, count($this->_errors));
 
         restore_error_handler();
     }

tests/UnitTests/ResourceTests/File/FileResourceTest.php

@@ -110,6 +110,9 @@ class FileResourceTest extends PHPUnit_Smarty
         );
     }
 
+    /**
+     * @doesNotPerformAssertions
+     */
     public function testGetCompiledTimestampPrepare()
     {
         $tpl = $this->smarty->createTemplate('helloworld.tpl');
@@ -142,6 +145,9 @@ class FileResourceTest extends PHPUnit_Smarty
         $this->assertTrue($tpl->mustCompile());
     }
 
+    /**
+     * @doesNotPerformAssertions
+     */
     public function testMustCompileTouchedSourcePrepare()
     {
         // touch to prepare next test

tests/UnitTests/ResourceTests/Php/PhpResourceTest.php

@@ -161,7 +161,7 @@ class PhpResourceTest extends PHPUnit_Smarty
      *
      * @runInSeparateProcess
      * @preserveGlobalState disabled
-     *
+     * @doesNotPerformAssertions
      */
     public function testIsCachedTouchedSourcePrepare()
     {
@@ -212,6 +212,7 @@ class PhpResourceTest extends PHPUnit_Smarty
 
     /**
      * test $smarty->is_cached
+     * @doesNotPerformAssertions
      */
     public function testSmartyIsCachedPrepare()
     {

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T1.tpl

@@ -0,0 +1 @@
+{* this is a comment *}

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T10.tpl

@@ -0,0 +1,7 @@
+=
+{* comment *}
+{* comment *}
+    b
+{* comment *}
+{* comment *}
+=

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T11.tpl

@@ -0,0 +1,7 @@
+=
+a
+{* comment 1 *}
+{* comment 2 *}
+{* comment 3 *}
+b
+=

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T12.tpl

@@ -0,0 +1,7 @@
+=
+a
+{* comment 1 *}
+ {* comment 2 *}
+{* comment 3 *}
+b
+=

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T13.tpl

@@ -0,0 +1,7 @@
+=
+a
+{* comment 1 *}
+{* comment 2 *} 
+{* comment 3 *}
+b
+=

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T14.tpl

@@ -0,0 +1,7 @@
+=
+a
+{* comment 1 *}
+ {* comment 2 *} 
+{* comment 3 *}
+b
+=

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T2.tpl

@@ -0,0 +1 @@
+{* another $foo comment *}

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T3.tpl

@@ -0,0 +1 @@
+{* another  comment *}some in between{* another  comment *}

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T4.tpl

@@ -0,0 +1,2 @@
+{* multi line 
+ comment *}

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T5.tpl

@@ -0,0 +1 @@
+{* /* foo * / *}

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T6.tpl

@@ -0,0 +1,2 @@
+A{* comment *}B
+C

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T7.tpl

@@ -0,0 +1,3 @@
+D{* comment *}
+{* comment *}E
+F

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T8.tpl

@@ -0,0 +1,2 @@
+G{* multi 
+line *}H

tests/UnitTests/TemplateSource/Comments/templates_tmp/testComments_T9.tpl

@@ -0,0 +1,3 @@
+I{* multi 
+line *}
+J

tests/UnitTests/TemplateSource/TagTests/PluginFunction/PluginFunctionHtmlImageTest.php

@@ -20,6 +20,9 @@ class PluginFunctionHtmlImageTest extends PHPUnit_Smarty
         $this->setUpSmarty(dirname(__FILE__));
     }
 
+    /**
+     * @doesNotPerformAssertions
+     */
     public function testFoo()
     {
         // TODO: UnitTests for {html_image}

tests/UnitTests/TemplateSource/TagTests/PluginModifier/PluginModifierDefaultTest.php

@@ -0,0 +1,151 @@
+<?php
+/**
+ * Smarty PHPunit tests of modifier
+ *
+ * @package PHPunit
+ */
+
+/**
+ * class for modifier tests
+ */
+class PluginModifierDefaultTest extends PHPUnit_Smarty
+{
+	public function setUp()
+	{
+		$this->setUpSmarty(dirname(__FILE__));
+		$this->smarty->setErrorReporting(E_ALL ^ E_NOTICE);
+	}
+
+	/**
+	 * @dataProvider        dataTestSimpleVars
+	 */
+	public function testSimpleVars($template, $expected)
+	{
+		$tpl = $this->smarty->createTemplate($template);
+		$this->smarty->assign('s', 'v0');
+		$this->assertEquals($expected, $this->smarty->fetch($tpl));
+	}
+
+	/**
+	 * Generates test data for ::testSimpleVars
+	 */
+	public function dataTestSimpleVars() {
+		return array(
+			array('string:{$s|default:"B"}', 'v0'),       // simple set variable
+			array('string:{$u|default:"B"}', 'B'),       // simple unset variable
+		);
+	}
+
+	/**
+	 * @dataProvider        dataTestSimpleVarsNoDefaultValue
+	 */
+	public function testSimpleVarsNoDefaultValue($template, $expected)
+	{
+		$tpl = $this->smarty->createTemplate($template);
+		$this->smarty->assign('s', 'v0');
+		$this->assertEquals($expected, $this->smarty->fetch($tpl));
+	}
+
+	/**
+	 * Generates test data for ::testSimpleVarsNoDefaultValue
+	 */
+	public function dataTestSimpleVarsNoDefaultValue() {
+		return array(
+			array('string:{$s|default}', 'v0'),       // simple set variable
+			array('string:{$u|default}', ''),       // simple unset variable
+		);
+	}
+
+	public function testEmptyString()
+	{
+		$tpl = $this->smarty->createTemplate('string:{$s|default:"B"}');
+		$this->smarty->assign('s', '');
+		$this->assertEquals('B', $this->smarty->fetch($tpl));
+	}
+
+	/**
+	 * @dataProvider dataFalsyValues
+	 */
+	public function testFalsyValues($falsyvalue, $expected)
+	{
+		$tpl = $this->smarty->createTemplate('string:{$s|default:"B"}');
+		$this->smarty->assign('s', $falsyvalue);
+		$this->assertEquals($expected, $this->smarty->fetch($tpl));
+	}
+
+	/**
+	 * Data for ::testFalsyValues
+	 */
+	public function dataFalsyValues() {
+		return array(
+			array(false, false),
+			array(0, 0),
+			array('0', '0'),
+			array(array(), 'Array'),
+			array(null, 'B'),
+			array('', 'B'),
+		);
+	}
+
+
+	public function testFunctionCall()
+	{
+		$tpl = $this->smarty->createTemplate('string:{strlen("a")|default:"B"}');
+		$this->assertEquals('1', $this->smarty->fetch($tpl));
+	}
+
+	public function testFunctionCallEmptyString()
+	{
+		$tpl = $this->smarty->createTemplate('string:{trim("")|default:"B"}');
+		$this->assertEquals('B', $this->smarty->fetch($tpl));
+	}
+
+	/**
+	 * @dataProvider        dataTestArrayObjectCombinations
+	 */
+	public function testArrayObjectCombinations($template, $expected)
+	{
+		$tpl = $this->smarty->createTemplate($template);
+		$this->smarty->assign('a', array(
+			'k' => (object) array ('v' => 'v3'),
+			'v' => 'v1'
+		));
+		$this->smarty->assign('o', (object) array (
+			'p' => array('v' => 'v4'),
+			'v' => 'v2'
+		));
+		$this->smarty->assign('k', 'k');
+		$this->smarty->assign('nk', 'nk');
+		$this->assertEquals($expected, $this->smarty->fetch($tpl));
+	}
+
+	/**
+	 * Generates test data for ::testArrayObjectCombinations
+	 */
+	public function dataTestArrayObjectCombinations() {
+		return array(
+			array('string:{$a.v|default:"B"}', 'v1'),     // array key set
+			array('string:{$o->v|default:"B"}', 'v2'),    // object property set
+			array('string:{$a.k->v|default:"B"}', 'v3'),  // complex combi of array and key access
+			array('string:{$a["k"]->v|default:"B"}', 'v3'),  // complex combi of array and key access
+			array('string:{$a[$k]->v|default:"B"}', 'v3'),  // complex combi of array and key access
+			array('string:{$a[$nk]->v|default:"B"}', 'B'),  // complex combi of array and key access
+			array('string:{$a[$u]->v|default:"B"}', 'B'),  // complex combi of array and key access
+			array('string:{$o->p.v|default:"B"}', 'v4'),  // complex combi of array and key access
+
+			array('string:{$a.c|default:"B"}', 'B'),     // array key not set
+			array('string:{$u.c|default:"B"}', 'B'),     // array not set, referencing key
+			array('string:{$o->u|default:"B"}', 'B'),    // object property not set
+			array('string:{$u->u|default:"B"}', 'B'),    // object not set
+			array('string:{$a.k->u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+			array('string:{$a["k"]->u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+			array('string:{$a[$k]->u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+			array('string:{$a[$nk]->u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+			array('string:{$a[$u]->u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+			array('string:{$a.u->u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+			array('string:{$o->p.u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+			array('string:{$o->u.u|default:"B"}', 'B'),  // complex combi of array and key access with something unset
+
+		);
+	}
+}

tests/UnitTests/TemplateSource/X_Scopes/ScopeTest.php

@@ -291,6 +291,9 @@ class ScopeTest extends PHPUnit_Smarty
                            'no  smarty', $i ++,),);
     }
 
+    /**
+     * @doesNotPerformAssertions
+     */
     public function testFunctionScope()
     {
         $this->smarty->assign('scope', 'none');