Skip to content

Commit

Permalink
Fix foreachelse with foreach on objects
Browse files Browse the repository at this point in the history
Try to address issue #506. After this patch the foreach logic always
count the total number of elements when the underlying structure can
easily be queried. E.g.: arrays and Countables can be queried with count()
for the total number of elements. For all iterators we do not
know the total element count in advance and calculating it is a wasteful
operation. And it can be dangerous, too: forward-iterators cannot be
rewinded and the foreach-loop will break. So with this patch the @ToTal
property will be simply set to -1 for iterators.
This has implications to other properties as well, when foreach is
used with iterarors: The @show property is
always true (even when there are no elements), and @last is always false.

This patch changes a runtime function call. So all templates that uses
foreach must be recompiled.
  • Loading branch information
volkarts-dev committed Feb 8, 2019
1 parent 70fc2fd commit cda5bb9
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 31 deletions.
13 changes: 3 additions & 10 deletions libs/sysplugins/smarty_internal_compile_foreach.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,6 @@ public function compile($args, Smarty_Internal_TemplateCompilerBase $compiler)
if ($this->isNamed) {
$foreachVar = "\$_smarty_tpl->tpl_vars['__smarty_foreach_{$attributes['name']}']";
}
$needTotal = isset($itemAttr[ 'total' ]);
// Register tag
$this->openTag(
$compiler,
Expand All @@ -200,9 +199,6 @@ public function compile($args, Smarty_Internal_TemplateCompilerBase $compiler)
$output = "<?php\n";
$output .= "\$_from = \$_smarty_tpl->smarty->ext->_foreach->init(\$_smarty_tpl, $from, " .
var_export($item, true);
if ($name || $needTotal || $key) {
$output .= ', ' . var_export($needTotal, true);
}
if ($name || $key) {
$output .= ', ' . var_export($key, true);
}
Expand All @@ -211,15 +207,14 @@ public function compile($args, Smarty_Internal_TemplateCompilerBase $compiler)
}
$output .= ");\n";
if (isset($itemAttr[ 'show' ])) {
$output .= "{$itemVar}->show = ({$itemVar}->total > 0);\n";
$output .= "{$itemVar}->show = ({$itemVar}->total == -1) || ({$itemVar}->total > 0);\n";
}
if (isset($itemAttr[ 'iteration' ])) {
$output .= "{$itemVar}->iteration = 0;\n";
}
if (isset($itemAttr[ 'index' ])) {
$output .= "{$itemVar}->index = -1;\n";
}
$output .= "if (\$_from !== null) {\n";
$output .= "foreach (\$_from as {$keyTerm}{$itemVar}->value) {\n";
if (isset($attributes[ 'key' ]) && isset($itemAttr[ 'key' ])) {
$output .= "\$_smarty_tpl->tpl_vars['{$key}']->value = {$itemVar}->key;\n";
Expand Down Expand Up @@ -296,7 +291,8 @@ public function compile($args, Smarty_Internal_TemplateCompilerBase $compiler)
if ($restore === 2) {
$output .= "{$itemVar} = {$local}saved;\n";
}
$output .= "}\n} else {\n?>";
$output .= "}\n";
$output .= "if ({$itemVar}->index == -1) {\n?>";
return $output;
}
}
Expand Down Expand Up @@ -332,9 +328,6 @@ public function compile($args, Smarty_Internal_TemplateCompilerBase $compiler)
if ($restore === 2) {
$output .= "{$itemVar} = {$local}saved;\n";
}
if ($restore > 0) {
$output .= "}\n";
}
$output .= "}\n";
/* @var Smarty_Internal_Compile_Foreach $foreachCompiler */
$foreachCompiler = $compiler->getTagCompiler('foreach');
Expand Down
32 changes: 11 additions & 21 deletions libs/sysplugins/smarty_internal_runtime_foreach.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,21 @@ public function init(
Smarty_Internal_Template $tpl,
$from,
$item,
$needTotal = false,
$key = null,
$name = null,
$properties = array()
) {
$needTotal = $needTotal || isset($properties[ 'total' ]);
$saveVars = array();
$total = null;
if (!is_array($from)) {
if (is_object($from)) {
if ($needTotal) {
$total = $this->count($from);
}
$total = $this->count($from);
} else {
settype($from, 'array');
}
}
if (!isset($total)) {
$total = empty($from) ? 0 : ($needTotal ? count($from) : 1);
$total = empty($from) ? 0 : count($from);
}
if (isset($tpl->tpl_vars[ $item ])) {
$saveVars[ 'item' ] = array(
Expand All @@ -63,9 +59,7 @@ public function init(
);
}
$tpl->tpl_vars[ $item ] = new Smarty_Variable(null, $tpl->isRenderingCache);
if ($total === 0) {
$from = null;
} else {
if ($total !== 0) {
if ($key) {
if (isset($tpl->tpl_vars[ $key ])) {
$saveVars[ 'key' ] = array(
Expand All @@ -76,9 +70,7 @@ public function init(
$tpl->tpl_vars[ $key ] = new Smarty_Variable(null, $tpl->isRenderingCache);
}
}
if ($needTotal) {
$tpl->tpl_vars[ $item ]->total = $total;
}
$tpl->tpl_vars[ $item ]->total = $total;
if ($name) {
$namedVar = "__smarty_foreach_{$name}";
if (isset($tpl->tpl_vars[ $namedVar ])) {
Expand All @@ -98,7 +90,7 @@ public function init(
$namedProp[ 'index' ] = -1;
}
if (isset($properties[ 'show' ])) {
$namedProp[ 'show' ] = ($total > 0);
$namedProp[ 'show' ] = ($total = -1) || ($total > 0);
}
$tpl->tpl_vars[ $namedVar ] = new Smarty_Variable($namedProp);
}
Expand All @@ -116,18 +108,16 @@ public function init(
*/
public function count($value)
{
if ($value instanceof IteratorAggregate) {
// Note: getIterator() returns a Traversable, not an Iterator
// thus rewind() and valid() methods may not be present
return iterator_count($value->getIterator());
} elseif ($value instanceof Iterator) {
return $value instanceof Generator ? 1 : iterator_count($value);
} elseif ($value instanceof Countable) {
if ($value instanceof Countable) {
return count($value);
} elseif ($value instanceof PDOStatement) {
return $value->rowCount();
} elseif ($value instanceof Iterator) {
return $value instanceof Generator ? 1 : -1;
} elseif ($value instanceof Traversable) {
return iterator_count($value);
return -1;
} elseif ($value instanceof IteratorAggregate) {
return -1;
}
return count((array)$value);
}
Expand Down

0 comments on commit cda5bb9

Please sign in to comment.