Strip now removes whitespaces after comment (#582)

* Mark text sections as to be stripped, but do not strip them right away. Combine equivalent sections in chunks at the template level and strip combined chunks where possible. - simplified Smarty_Internal_TemplateCompilerBase::processText along the way Fixes #447
smarty-php · Apr 13, 2020 · 19ef834 · 19ef834
1 parent 4f89f6d
commit 19ef834
Show file tree

Hide file tree

Showing 7 changed files with 164 additions and 118 deletions.
diff --git a/TODO.md b/TODO.md
@@ -1,18 +1,5 @@
 # Todo
 
-## Add unit test for strip issue in correct branch
-tests/UnitTests/TemplateSource/TagTests/Strip/CompileStripTest.php 
-```
-@@ -76,6 +76,7 @@ class CompileStripTest extends PHPUnit_Smarty
-                      array("{'Var'}\n <b></b> <c></c>", 'Var<b></b> <c></c>', '', $i ++),
-                      array("\n<b></b>  <c></c>", '<b></b> <c></c>', '', $i ++),
-                      array("\n<b></b>\n  <c></c>", '<b></b><c></c>', '', $i ++),
-+                     array("\n<b>\n  {* a comment *}\n   <c>", '<b><c>', '', $i ++),
-
-         );
-     }
-```
-
 ## Add unit test for isset issue in correct branch
 tests/UnitTests/TemplateSource/ValueTests/PHPfunctions/PhpFunctionTest.php
 ```php

diff --git a/change_log.txt b/change_log.txt
@@ -1,3 +1,4 @@
+ - remove whitespaces after comments https://github.com/smarty-php/smarty/issues/447
  - fix foreachelse on arrayiterators https://github.com/smarty-php/smarty/issues/506
  - throw SmartyException when setting caching attributes for cacheable plugin https://github.com/smarty-php/smarty/issues/457
  - fix errors that occured where isset was replaced with null check such as https://github.com/smarty-php/smarty/issues/453

diff --git a/libs/sysplugins/smarty_internal_parsetree_template.php b/libs/sysplugins/smarty_internal_parsetree_template.php
@@ -85,45 +85,85 @@ public function prepend_array(Smarty_Internal_Templateparser $parser, $array = a
     public function to_smarty_php(Smarty_Internal_Templateparser $parser)
     {
         $code = '';
-        for ($key = 0, $cnt = count($this->subtrees); $key < $cnt; $key++) {
-            if ($this->subtrees[ $key ] instanceof Smarty_Internal_ParseTree_Text) {
-                $subtree = $this->subtrees[ $key ]->to_smarty_php($parser);
-                while ($key + 1 < $cnt && ($this->subtrees[ $key + 1 ] instanceof Smarty_Internal_ParseTree_Text ||
-                                           $this->subtrees[ $key + 1 ]->data === '')) {
-                    $key++;
-                    if ($this->subtrees[ $key ]->data === '') {
-                        continue;
-                    }
-                    $subtree .= $this->subtrees[ $key ]->to_smarty_php($parser);
-                }
-                if ($subtree === '') {
-                    continue;
-                }
-                $code .= preg_replace(
-                    '/((<%)|(%>)|(<\?php)|(<\?)|(\?>)|(<\/?script))/',
-                    "<?php echo '\$1'; ?>\n",
-                    $subtree
-                );
-                continue;
-            }
-            if ($this->subtrees[ $key ] instanceof Smarty_Internal_ParseTree_Tag) {
-                $subtree = $this->subtrees[ $key ]->to_smarty_php($parser);
-                while ($key + 1 < $cnt && ($this->subtrees[ $key + 1 ] instanceof Smarty_Internal_ParseTree_Tag ||
-                                           $this->subtrees[ $key + 1 ]->data === '')) {
-                    $key++;
-                    if ($this->subtrees[ $key ]->data === '') {
-                        continue;
-                    }
-                    $subtree = $parser->compiler->appendCode($subtree, $this->subtrees[ $key ]->to_smarty_php($parser));
-                }
-                if ($subtree === '') {
-                    continue;
-                }
-                $code .= $subtree;
-                continue;
-            }
-            $code .= $this->subtrees[ $key ]->to_smarty_php($parser);
+
+        foreach ($this->getChunkedSubtrees() as $chunk) {
+	        $text = '';
+	        switch ($chunk['mode']) {
+		        case 'textstripped':
+			        foreach ($chunk['subtrees'] as $subtree) {
+				        $text .= $subtree->to_smarty_php($parser);
+			        }
+			        $code .= preg_replace(
+				        '/((<%)|(%>)|(<\?php)|(<\?)|(\?>)|(<\/?script))/',
+				        "<?php echo '\$1'; ?>\n",
+				        $parser->compiler->processText($text)
+			        );
+			        break;
+		        case 'text':
+			        foreach ($chunk['subtrees'] as $subtree) {
+				        $text .= $subtree->to_smarty_php($parser);
+			        }
+			        $code .= preg_replace(
+				        '/((<%)|(%>)|(<\?php)|(<\?)|(\?>)|(<\/?script))/',
+				        "<?php echo '\$1'; ?>\n",
+				        $text
+			        );
+			        break;
+		        case 'tag':
+			        foreach ($chunk['subtrees'] as $subtree) {
+				        $text = $parser->compiler->appendCode($text, $subtree->to_smarty_php($parser));
+			        }
+			        $code .= $text;
+			        break;
+		        default:
+			        foreach ($chunk['subtrees'] as $subtree) {
+				        $text = $subtree->to_smarty_php($parser);
+			        }
+			        $code .= $text;
+
+	        }
         }
         return $code;
     }
+
+    private function getChunkedSubtrees() {
+    	$chunks = [];
+    	$currentMode = null;
+    	$currentChunk = [];
+	    for ($key = 0, $cnt = count($this->subtrees); $key < $cnt; $key++) {
+
+	    	if ($this->subtrees[ $key ]->data === '' && in_array($currentMode, ['textstripped', 'text', 'tag'])) {
+	    		continue;
+	    	}
+
+		    if ($this->subtrees[ $key ] instanceof Smarty_Internal_ParseTree_Text
+			    && $this->subtrees[ $key ]->isToBeStripped()) {
+		    	$newMode = 'textstripped';
+		    } elseif ($this->subtrees[ $key ] instanceof Smarty_Internal_ParseTree_Text) {
+			    $newMode = 'text';
+		    } elseif ($this->subtrees[ $key ] instanceof Smarty_Internal_ParseTree_Tag) {
+			    $newMode = 'tag';
+		    } else {
+			    $newMode = 'other';
+		    }
+
+		    if ($newMode == $currentMode) {
+			    $currentChunk[] = $this->subtrees[ $key ];
+		    } else {
+		    	$chunks[] = [
+		    		'mode' => $currentMode,
+				    'subtrees' => $currentChunk
+			    ];
+		    	$currentMode = $newMode;
+			    $currentChunk = [$this->subtrees[ $key ]];
+		    }
+	    }
+	    if ($currentMode && $currentChunk) {
+		    $chunks[] = [
+			    'mode' => $currentMode,
+			    'subtrees' => $currentChunk
+		    ];
+	    }
+		return $chunks;
+    }
 }
diff --git a/libs/sysplugins/smarty_internal_parsetree_text.php b/libs/sysplugins/smarty_internal_parsetree_text.php
@@ -16,14 +16,31 @@
  */
 class Smarty_Internal_ParseTree_Text extends Smarty_Internal_ParseTree
 {
-    /**
-     * Create template text buffer
-     *
-     * @param string $data text
-     */
-    public function __construct($data)
+
+	/**
+	 * Wether this section should be stripped on output to smarty php
+	 * @var bool
+	 */
+	private $toBeStripped = false;
+
+	/**
+	 * Create template text buffer
+	 *
+	 * @param string $data text
+	 * @param bool $toBeStripped wether this section should be stripped on output to smarty php
+	 */
+    public function __construct($data, $toBeStripped = false)
     {
         $this->data = $data;
+        $this->toBeStripped = $toBeStripped;
+    }
+
+	/**
+	 * Wether this section should be stripped on output to smarty php
+	 * @return bool
+	 */
+	public function isToBeStripped() {
+    	return $this->toBeStripped;
     }
 
     /**

diff --git a/libs/sysplugins/smarty_internal_templatecompilerbase.php b/libs/sysplugins/smarty_internal_templatecompilerbase.php
@@ -680,73 +680,69 @@ private function syntaxMatchesVariable($string) {
     }
 
     /**
-     * This method is called from parser to process a text content section
+     * This method is called from parser to process a text content section if strip is enabled
      * - remove text from inheritance child templates as they may generate output
-     * - strip text if strip is enabled
      *
      * @param string $text
      *
-     * @return null|\Smarty_Internal_ParseTree_Text
+     * @return string
      */
     public function processText($text)
     {
-        if ((string)$text != '') {
-            $store = array();
-            $_store = 0;
-            if ($this->parser->strip) {
-                if (strpos($text, '<') !== false) {
-                    // capture html elements not to be messed with
-                    $_offset = 0;
-                    if (preg_match_all(
-                        '#(<script[^>]*>.*?</script[^>]*>)|(<textarea[^>]*>.*?</textarea[^>]*>)|(<pre[^>]*>.*?</pre[^>]*>)#is',
-                        $text,
-                        $matches,
-                        PREG_OFFSET_CAPTURE | PREG_SET_ORDER
-                    )
-                    ) {
-                        foreach ($matches as $match) {
-                            $store[] = $match[ 0 ][ 0 ];
-                            $_length = strlen($match[ 0 ][ 0 ]);
-                            $replace = '@!@SMARTY:' . $_store . ':SMARTY@!@';
-                            $text = substr_replace($text, $replace, $match[ 0 ][ 1 ] - $_offset, $_length);
-                            $_offset += $_length - strlen($replace);
-                            $_store++;
-                        }
-                    }
-                    $expressions = array(// replace multiple spaces between tags by a single space
-                                         '#(:SMARTY@!@|>)[\040\011]+(?=@!@SMARTY:|<)#s'                            => '\1 \2',
-                                         // remove newline between tags
-                                         '#(:SMARTY@!@|>)[\040\011]*[\n]\s*(?=@!@SMARTY:|<)#s'                     => '\1\2',
-                                         // remove multiple spaces between attributes (but not in attribute values!)
-                                         '#(([a-z0-9]\s*=\s*("[^"]*?")|(\'[^\']*?\'))|<[a-z0-9_]+)\s+([a-z/>])#is' => '\1 \5',
-                                         '#>[\040\011]+$#Ss'                                                       => '> ',
-                                         '#>[\040\011]*[\n]\s*$#Ss'                                                => '>',
-                                         $this->stripRegEx                                                         => '',
-                    );
-                    $text = preg_replace(array_keys($expressions), array_values($expressions), $text);
-                    $_offset = 0;
-                    if (preg_match_all(
-                        '#@!@SMARTY:([0-9]+):SMARTY@!@#is',
-                        $text,
-                        $matches,
-                        PREG_OFFSET_CAPTURE | PREG_SET_ORDER
-                    )
-                    ) {
-                        foreach ($matches as $match) {
-                            $_length = strlen($match[ 0 ][ 0 ]);
-                            $replace = $store[ $match[ 1 ][ 0 ] ];
-                            $text = substr_replace($text, $replace, $match[ 0 ][ 1 ] + $_offset, $_length);
-                            $_offset += strlen($replace) - $_length;
-                            $_store++;
-                        }
-                    }
-                } else {
-                    $text = preg_replace($this->stripRegEx, '', $text);
-                }
+
+        if (strpos($text, '<') === false) {
+        	return preg_replace($this->stripRegEx, '', $text);
+        }
+
+	    $store = array();
+	    $_store = 0;
+
+        // capture html elements not to be messed with
+        $_offset = 0;
+        if (preg_match_all(
+            '#(<script[^>]*>.*?</script[^>]*>)|(<textarea[^>]*>.*?</textarea[^>]*>)|(<pre[^>]*>.*?</pre[^>]*>)#is',
+            $text,
+            $matches,
+            PREG_OFFSET_CAPTURE | PREG_SET_ORDER
+        )
+        ) {
+            foreach ($matches as $match) {
+                $store[] = $match[ 0 ][ 0 ];
+                $_length = strlen($match[ 0 ][ 0 ]);
+                $replace = '@!@SMARTY:' . $_store . ':SMARTY@!@';
+                $text = substr_replace($text, $replace, $match[ 0 ][ 1 ] - $_offset, $_length);
+                $_offset += $_length - strlen($replace);
+                $_store++;
+            }
+        }
+        $expressions = array(// replace multiple spaces between tags by a single space
+                             '#(:SMARTY@!@|>)[\040\011]+(?=@!@SMARTY:|<)#s'                            => '\1 \2',
+                             // remove newline between tags
+                             '#(:SMARTY@!@|>)[\040\011]*[\n]\s*(?=@!@SMARTY:|<)#s'                     => '\1\2',
+                             // remove multiple spaces between attributes (but not in attribute values!)
+                             '#(([a-z0-9]\s*=\s*("[^"]*?")|(\'[^\']*?\'))|<[a-z0-9_]+)\s+([a-z/>])#is' => '\1 \5',
+                             '#>[\040\011]+$#Ss'                                                       => '> ',
+                             '#>[\040\011]*[\n]\s*$#Ss'                                                => '>',
+                             $this->stripRegEx                                                         => '',
+        );
+        $text = preg_replace(array_keys($expressions), array_values($expressions), $text);
+        $_offset = 0;
+        if (preg_match_all(
+            '#@!@SMARTY:([0-9]+):SMARTY@!@#is',
+            $text,
+            $matches,
+            PREG_OFFSET_CAPTURE | PREG_SET_ORDER
+        )
+        ) {
+            foreach ($matches as $match) {
+                $_length = strlen($match[ 0 ][ 0 ]);
+                $replace = $store[ $match[ 1 ][ 0 ] ];
+                $text = substr_replace($text, $replace, $match[ 0 ][ 1 ] + $_offset, $_length);
+                $_offset += strlen($replace) - $_length;
+                $_store++;
             }
-            return new Smarty_Internal_ParseTree_Text($text);
         }
-        return null;
+        return $text;
     }
 
     /**

diff --git a/libs/sysplugins/smarty_internal_templateparser.php b/libs/sysplugins/smarty_internal_templateparser.php
@@ -2169,8 +2169,13 @@ public function yy_r1()
     // line 255 "../smarty/lexer/smarty_internal_templateparser.y"
     public function yy_r2()
     {
-        $this->current_buffer->append_subtree($this,
-            $this->compiler->processText($this->yystack[ $this->yyidx + 0 ]->minor));
+    	$text = $this->yystack[ $this->yyidx + 0 ]->minor;
+
+	    if ((string)$text == '') {
+	    	$this->current_buffer->append_subtree($this, null);
+	    }
+
+	    $this->current_buffer->append_subtree($this, new Smarty_Internal_ParseTree_Text($text, $this->strip));
     }
 
     // line 259 "../smarty/lexer/smarty_internal_templateparser.y"

diff --git a/tests/UnitTests/TemplateSource/TagTests/Strip/CompileStripTest.php b/tests/UnitTests/TemplateSource/TagTests/Strip/CompileStripTest.php
@@ -76,7 +76,7 @@ public function dataTestStrip()
                      array("{'Var'}\n <b></b> <c></c>", 'Var<b></b> <c></c>', '', $i ++),
                      array("\n<b></b>  <c></c>", '<b></b> <c></c>', '', $i ++),
                      array("\n<b></b>\n  <c></c>", '<b></b><c></c>', '', $i ++),
-
+                     array("\n<b>\n  {* a comment *}\n   <c>", '<b><c>', '', $i ++),
         );
     }