diff --git a/src/ORM/ArrayList.php b/src/ORM/ArrayList.php index f953ffb6176..2d98d26f984 100644 --- a/src/ORM/ArrayList.php +++ b/src/ORM/ArrayList.php @@ -127,7 +127,7 @@ public function exists() public function getIterator(): Traversable { foreach ($this->items as $i => $item) { - if (is_array($item)) { + if (is_array($item) && !array_is_list($item)) { yield new ArrayData($item); } else { yield $item; diff --git a/src/View/SSTemplateParser.peg b/src/View/SSTemplateParser.peg index 2ae672b39be..2061c1879a5 100644 --- a/src/View/SSTemplateParser.peg +++ b/src/View/SSTemplateParser.peg @@ -287,8 +287,6 @@ class SSTemplateParser extends Parser implements TemplateParser if (isset($sub['Call']['CallArguments']) && isset($sub['Call']['CallArguments']['php'])) { $arguments = $sub['Call']['CallArguments']['php']; $res['php'] .= "->$method('$property', [$arguments], true)"; - } elseif ($property === 'Count') { - $res['php'] .= "->$property()"; } else { $res['php'] .= "->$method('$property', null, true)"; } diff --git a/src/View/SSTemplateParser.php b/src/View/SSTemplateParser.php index ed1b8b54c88..db9f0a6f342 100644 --- a/src/View/SSTemplateParser.php +++ b/src/View/SSTemplateParser.php @@ -778,8 +778,6 @@ function Lookup_AddLookupStep(&$res, $sub, $method) if (isset($sub['Call']['CallArguments']) && isset($sub['Call']['CallArguments']['php'])) { $arguments = $sub['Call']['CallArguments']['php']; $res['php'] .= "->$method('$property', [$arguments], true)"; - } elseif ($property === 'Count') { - $res['php'] .= "->$property()"; } else { $res['php'] .= "->$method('$property', null, true)"; } @@ -1888,8 +1886,6 @@ function PresenceCheck_Argument(&$res, $sub) $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 ?? ''); } } @@ -5294,8 +5290,6 @@ function Text__finalise(&$res) $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'] ) ) @@ -5334,8 +5328,7 @@ public function compileString($string, $templateName = "", $includeDebuggingComm $this->includeDebuggingComments = $includeDebuggingComments; - // Ignore UTF8 BOM at beginning of string. TODO: Confirm this is needed, make sure SSViewer handles UTF - // (and other encodings) properly + // Ignore UTF8 BOM at beginning of string. if (substr($string ?? '', 0, 3) == pack("CCC", 0xef, 0xbb, 0xbf)) { $this->pos = 3; } diff --git a/src/View/SSViewer_DataPresenter.php b/src/View/SSViewer_DataPresenter.php index 3c836937d07..729930174a2 100644 --- a/src/View/SSViewer_DataPresenter.php +++ b/src/View/SSViewer_DataPresenter.php @@ -308,29 +308,6 @@ public function getObj($name, $arguments = [], $cache = false, $cacheName = null */ public function __call($name, $arguments) { - // $Count should be handled specially, so we can count raw arrays and iterables - // which aren't ViewableData. - if (empty($arguments) && ($name === 'Count' || $name === 'count')) { - $item = $this->getItem(); - $result = null; - if ($item instanceof ViewableData) { - // Respect ViewableData casting - $result = $item->XML_val($name, [], true); - } elseif (is_object($item)) { - // Get the method or property from objects, if there is one - if (ClassInfo::hasMethod($item, $name)) { - $result = $item->$name(); - } elseif (isset($item->$name)) { - $result = $item->$name; - } - } elseif (is_countable($item)) { - // Count countables - $result = count($item); - } - $this->resetLocalScope(); - return $result; - } - // Extract the method name and parameters $property = $arguments[0]; // The name of the public function being called @@ -341,14 +318,7 @@ public function __call($name, $arguments) if ($val) { $obj = $val['obj']; if ($name === 'hasValue') { - // Check if a value exists, e.g. <% if $obj %> - if ($obj instanceof ViewableData) { - $result = $obj->exists(); - } elseif (is_countable($obj)) { - $result = count($obj) > 0; - } else { - $result = (bool) $obj; - } + $result = ($obj instanceof ViewableData) ? $obj->exists() : (bool)$obj; } elseif (is_null($obj) || (is_scalar($obj) && !is_string($obj))) { $result = $obj; // Nulls and non-string scalars don't need casting } else { diff --git a/src/View/SSViewer_Scope.php b/src/View/SSViewer_Scope.php index 6f4af08c308..9805b22ec95 100644 --- a/src/View/SSViewer_Scope.php +++ b/src/View/SSViewer_Scope.php @@ -5,6 +5,7 @@ use ArrayIterator; use Countable; use Iterator; +use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\FieldType\DBBoolean; use SilverStripe\ORM\FieldType\DBText; use SilverStripe\ORM\FieldType\DBFloat; @@ -130,16 +131,17 @@ public function getItem() if (is_scalar($item)) { $item = $this->convertScalarToDBField($item); } - // Wrap arrays + + // Wrap arrays in ViewableData so templates can handle them if (is_array($item)) { if (array_is_list($item)) { - // Wrap in ArrayIterator to respect method signature - $item = new ArrayIterator($item); + $item = ArrayList::create($item); } else { - // Wrap in ArrayData so values can be accessed by key in templates + // Allow values to be accessed by key in templates $item = ArrayData::create($item); } } + return $item; } diff --git a/src/View/ViewableData.php b/src/View/ViewableData.php index e694bd8e5fb..8fb05b48284 100644 --- a/src/View/ViewableData.php +++ b/src/View/ViewableData.php @@ -20,6 +20,7 @@ use SilverStripe\Dev\Debug; use SilverStripe\Dev\Deprecation; use SilverStripe\ORM\ArrayLib; +use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\ORM\FieldType\DBHTMLText; use SilverStripe\View\SSViewer; @@ -558,13 +559,12 @@ public function obj($fieldName, $arguments = [], $cache = false, $cacheName = nu $value = $this->$fieldName; } - // Wrap arrays + // Wrap arrays in ViewableData so templates can handle them if (is_array($value)) { if (array_is_list($value)) { - // Wrap in ArrayIterator to respect method signature - $value = new ArrayIterator($value); + $value = ArrayList::create($value); } else { - // Wrap in ArrayData so values can be accessed by key in templates + // Allow values to be accessed by key in templates $value = ArrayData::create($value); } } @@ -615,9 +615,6 @@ public function hasValue($field, $arguments = [], $cache = true) if ($result instanceof ViewableData) { return $result->exists(); } - if (is_countable($result)) { - return count($result) > 0; - } return (bool) $result; } diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index ac42398fdce..06a3b2ba534 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -1429,10 +1429,6 @@ public function provideCountIterable(): array 'iterable' => [1, 2, 3], 'inScope' => false, ], - 'iterator' => [ - 'iterable' => new ArrayIterator([1, 2, 3]), - 'inScope' => false, - ], 'ArrayList' => [ 'iterable' => new ArrayList([['Val' => 1], ['Val' => 2], ['Val' => 3]]), 'inScope' => false, diff --git a/tests/php/View/ViewableDataTest.php b/tests/php/View/ViewableDataTest.php index 3403fb2b9e2..b25d62464e1 100644 --- a/tests/php/View/ViewableDataTest.php +++ b/tests/php/View/ViewableDataTest.php @@ -2,10 +2,10 @@ namespace SilverStripe\View\Tests; -use ArrayIterator; use ReflectionMethod; use SilverStripe\ORM\FieldType\DBField; use SilverStripe\Dev\SapphireTest; +use SilverStripe\ORM\ArrayList; use SilverStripe\View\ArrayData; use SilverStripe\View\SSViewer; use SilverStripe\View\Tests\ViewableDataTest\ViewableDataTestExtension; @@ -285,14 +285,14 @@ public function provideWrapArrayInObj(): array return [ 'empty array' => [ 'arr' => [], - 'expectedClass' => ArrayIterator::class, + 'expectedClass' => ArrayList::class, ], 'fully indexed array' => [ 'arr' => [ 'value1', 'value2', ], - 'expectedClass' => ArrayIterator::class, + 'expectedClass' => ArrayList::class, ], 'fully associative array' => [ 'arr' => [