Tidy up + document SSViewer classes

src/View/SSViewer.php

@@ -48,42 +48,6 @@ class SSViewer implements Flushable
      */
     const DEFAULT_THEME = '$default';
 
-    /**
-     * @config
-     * @var boolean $source_file_comments
-     */
-    private static $source_file_comments = false;
-
-    /**
-     * @ignore
-     */
-    private static $template_cache_flushed = false;
-
-    /**
-     * @ignore
-     */
-    private static $cacheblock_cache_flushed = false;
-
-    /**
-     * @var array $templates List of templates to select from
-     */
-    protected $templates = null;
-
-    /**
-     * @var string $chosen Absolute path to chosen template file
-     */
-    protected $chosen = null;
-
-    /**
-     * @var array Templates to use when looking up 'Layout' or 'Content'
-     */
-    protected $subTemplates = null;
-
-    /**
-     * @var boolean
-     */
-    protected $rewriteHashlinks = true;
-
     /**
      * @config
      * @var string A list (highest priority first) of themes to use
@@ -109,6 +73,61 @@ class SSViewer implements Flushable
      */
     private static $theme_enabled = true;
 
+    /**
+     * Default prepended cache key for partial caching
+     *
+     * @config
+     * @var string
+     */
+    private static $global_key = '$CurrentReadingMode, $CurrentUser.ID';
+
+    /**
+     * @config
+     * @var boolean $source_file_comments
+     */
+    private static $source_file_comments = false;
+
+    /**
+     * @config
+     * @var boolean
+     */
+    private static $rewrite_hash_links = true;
+
+    /**
+     * @ignore
+     */
+    private static $template_cache_flushed = false;
+
+    /**
+     * @ignore
+     */
+    private static $cacheblock_cache_flushed = false;
+
+    /**
+     * @var array $topLevel List of items being processed
+     */
+    protected static $topLevel = [];
+
+    /**
+     * @var array $templates List of templates to select from
+     */
+    protected $templates = null;
+
+    /**
+     * @var string $chosen Absolute path to chosen template file
+     */
+    protected $chosen = null;
+
+    /**
+     * @var array Templates to use when looking up 'Layout' or 'Content'
+     */
+    protected $subTemplates = null;
+
+    /**
+     * @var boolean
+     */
+    protected $rewriteHashlinks = true;
+
     /**
      * @var boolean
      */
@@ -119,13 +138,42 @@ class SSViewer implements Flushable
      */
     protected $parser;
 
-    /*
+    /**
-	 * Default prepended cache key for partial caching
+     * @var CacheInterface
-	 *
+     */
-	 * @var string
+    protected $partialCacheStore = null;
-	 * @config
+
-	 */
+    /**
-    private static $global_key = '$CurrentReadingMode, $CurrentUser.ID';
+     * @param string|array $templates If passed as a string with .ss extension, used as the "main" template.
+     *  If passed as an array, it can be used for template inheritance (first found template "wins").
+     *  Usually the array values are PHP class names, which directly correlate to template names.
+     *  <code>
+     *  array('MySpecificPage', 'MyPage', 'Page')
+     *  </code>
+     * @param TemplateParser $parser
+     */
+    public function __construct($templates, TemplateParser $parser = null)
+    {
+        if ($parser) {
+            $this->setParser($parser);
+        }
+
+        $this->setTemplate($templates);
+
+        if (!$this->chosen) {
+            $message = 'None of the following templates could be found: ';
+            $message .= print_r($templates, true);
+
+            $themes = self::get_themes();
+            if (!$themes) {
+                $message .= ' (no theme in use)';
+            } else {
+                $message .= ' in themes "' . print_r($themes, true) . '"';
+            }
+
+            user_error($message, E_USER_WARNING);
+        }
+    }
 
     /**
      * Triggered early in the request when someone requests a flush.
@@ -163,6 +211,11 @@ class SSViewer implements Flushable
         SSViewer::config()->set('themes', $themes);
     }
 
+    /**
+     * Add to the list of active themes to apply
+     *
+     * @param array $themes
+     */
     public static function add_themes($themes = [])
     {
         $currentThemes = SSViewer::get_themes();
@@ -171,6 +224,11 @@ class SSViewer implements Flushable
         SSViewer::set_themes(array_values(array_unique($finalThemes)));
     }
 
+    /**
+     * Get the list of active themes
+     *
+     * @return array
+     */
     public static function get_themes()
     {
         $default = [self::DEFAULT_THEME];
@@ -223,7 +281,8 @@ class SSViewer implements Flushable
                 'SSViewer::get_templates_by_class() expects a valid class name as its first parameter.'
             );
         }
-        $templates = array();
+
+        $templates = [];
         $classes = array_reverse(ClassInfo::ancestry($classOrObject));
         foreach ($classes as $class) {
             $template = $class . $suffix;
@@ -239,41 +298,26 @@ class SSViewer implements Flushable
                 break;
             }
         }
+
         return $templates;
     }
 
     /**
-     * @param string|array $templates If passed as a string with .ss extension, used as the "main" template.
+     * Get the current item being processed
-     *  If passed as an array, it can be used for template inheritance (first found template "wins").
+     *
-     *  Usually the array values are PHP class names, which directly correlate to template names.
+     * @return ViewableData
-     *  <code>
-     *  array('MySpecificPage', 'MyPage', 'Page')
-     *  </code>
-     * @param TemplateParser $parser
      */
-    public function __construct($templates, TemplateParser $parser = null)
+    public static function topLevel()
     {
-        if ($parser) {
+        if (SSViewer::$topLevel) {
-            $this->setParser($parser);
+            return SSViewer::$topLevel[sizeof(SSViewer::$topLevel)-1];
-        }
-
-        $this->setTemplate($templates);
-
-        if (!$this->chosen) {
-            $message = 'None of the following templates could be found: ';
-            $message .= print_r($templates, true);
-
-            $themes = self::get_themes();
-            if (!$themes) {
-                $message .= ' (no theme in use)';
-            } else {
-                $message .= ' in themes "' . print_r($themes, true) . '"';
-            }
-
-            user_error($message, E_USER_WARNING);
         }
+        return null;
     }
 
+    /**
+     * @param string|array $templates
+     */
     public function setTemplate($templates)
     {
         $this->templates = $templates;
@@ -327,22 +371,6 @@ class SSViewer implements Flushable
         return (bool)ThemeResourceLoader::inst()->findTemplate($templates, self::get_themes());
     }
 
-    /**
-     * @config
-     * @var boolean
-     */
-    private static $rewrite_hash_links = true;
-
-    protected static $topLevel = array();
-
-    public static function topLevel()
-    {
-        if (SSViewer::$topLevel) {
-            return SSViewer::$topLevel[sizeof(SSViewer::$topLevel)-1];
-        }
-        return null;
-    }
-
     /**
      * Call this to disable rewriting of <a href="#xxx"> links.  This is useful in Ajax applications.
      * It returns the SSViewer objects, so that you can call new SSViewer("X")->dontRewriteHashlinks()->process();
@@ -354,6 +382,9 @@ class SSViewer implements Flushable
         return $this;
     }
 
+    /**
+     * @return string
+     */
     public function exists()
     {
         return $this->chosen;
@@ -362,7 +393,6 @@ class SSViewer implements Flushable
     /**
      * @param string $identifier A template name without '.ss' extension or path
      * @param string $type The template type, either "main", "Includes" or "Layout"
-     *
      * @return string Full system path to a template file
      */
     public static function getTemplateFileByType($identifier, $type = null)
@@ -410,11 +440,6 @@ class SSViewer implements Flushable
         }
     }
 
-    /**
-     * @var CacheInterface
-     */
-    protected $partialCacheStore = null;
-
     /**
      * Set the cache object to use when storing / retrieving partial cache blocks.
      *
@@ -432,7 +457,11 @@ class SSViewer implements Flushable
      */
     public function getPartialCacheStore()
     {
-        return $this->partialCacheStore ? $this->partialCacheStore : Injector::inst()->get(CacheInterface::class . '.cacheblock');
+        if ($this->partialCacheStore) {
+            return $this->partialCacheStore;
+        }
+
+        return Injector::inst()->get(CacheInterface::class . '.cacheblock');
     }
 
     /**
@@ -504,7 +533,7 @@ class SSViewer implements Flushable
         $template = $this->chosen;
 
         $cacheFile = TEMP_FOLDER . "/.cache"
-            . str_replace(array('\\','/',':'), '.', Director::makeRelative(realpath($template)));
+            . str_replace(['\\','/',':'], '.', Director::makeRelative(realpath($template)));
         $lastEdited = filemtime($template);
 
         if (!file_exists($cacheFile) || filemtime($cacheFile) < $lastEdited) {
@@ -516,11 +545,11 @@ class SSViewer implements Flushable
             fclose($fh);
         }
 
-        $underlay = array('I18NNamespace' => basename($template));
+        $underlay = ['I18NNamespace' => basename($template)];
 
         // Makes the rendered sub-templates available on the parent item,
         // through $Content and $Layout placeholders.
-        foreach (array('Content', 'Layout') as $subtemplate) {
+        foreach (['Content', 'Layout'] as $subtemplate) {
             // Detect sub-template to use
             $sub = $this->getSubtemplateFor($subtemplate);
             if (!$sub) {
@@ -557,7 +586,9 @@ class SSViewer implements Flushable
         if ($this->rewriteHashlinks && $rewrite) {
             if (strpos($output, '<base') !== false) {
                 if ($rewrite === 'php') {
-                    $thisURLRelativeToBase = "<?php echo \\SilverStripe\\Core\\Convert::raw2att(preg_replace(\"/^(\\\\/)+/\", \"/\", \$_SERVER['REQUEST_URI'])); ?>";
+                    $thisURLRelativeToBase = <<<PHP
+<?php echo \\SilverStripe\\Core\\Convert::raw2att(preg_replace("/^(\\\\/)+/", "/", \$_SERVER['REQUEST_URI'])); ?>
+PHP;
                 } else {
                     $thisURLRelativeToBase = Convert::raw2att(preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI']));
                 }
@@ -642,6 +673,12 @@ class SSViewer implements Flushable
         return $v->process($data, $arguments);
     }
 
+    /**
+     * Parse given template contents
+     *
+     * @param string $content The template contents
+     * @param string $template The template file name
+     */
     public function parseTemplateContent($content, $template = "")
     {
         return $this->getParser()->compileString(
@@ -654,6 +691,8 @@ class SSViewer implements Flushable
     /**
      * Returns the filenames of the template that will be rendered.  It is a map that may contain
      * 'Content' & 'Layout', and will have to contain 'main'
+     *
+     * @return array
      */
     public function templates()
     {

src/View/SSViewer_BasicIteratorSupport.php

@@ -8,7 +8,6 @@ namespace SilverStripe\View;
  */
 class SSViewer_BasicIteratorSupport implements TemplateIteratorProvider
 {
-
     /**
      * @var int
      */
@@ -19,9 +18,12 @@ class SSViewer_BasicIteratorSupport implements TemplateIteratorProvider
      */
     protected $iteratorTotalItems;
 
+    /**
+     * @return array
+     */
     public static function get_template_iterator_variables()
     {
-        return array(
+        return [
             'First',
             'Last',
             'FirstLast',
@@ -35,7 +37,7 @@ class SSViewer_BasicIteratorSupport implements TemplateIteratorProvider
             'TotalItems',
             'Modulus',
             'MultipleOf',
-        );
+        ];
     }
 
     /**
@@ -183,6 +185,7 @@ class SSViewer_BasicIteratorSupport implements TemplateIteratorProvider
     /**
      * Returns the modulus of the numerical position of the item in the data set.
      * The count starts from $startIndex, which defaults to 1.
+     *
      * @param int $mod The number to perform Mod operation to.
      * @param int $startIndex Number to start count from.
      * @return int
@@ -196,6 +199,7 @@ class SSViewer_BasicIteratorSupport implements TemplateIteratorProvider
      * Returns true or false depending on if the pos of the iterator is a multiple of a specific number.
      * So, <% if MultipleOf(3) %> would return true on indexes: 3,6,9,12,15, etc.
      * The count starts from $offset, which defaults to 1.
+     *
      * @param int $factor The multiple of which to return
      * @param int $offset Number to start count from.
      * @return bool

src/View/SSViewer_DataPresenter.php

@@ -40,95 +40,126 @@ class SSViewer_DataPresenter extends SSViewer_Scope
 
     /**
      * Underlay variables. Concede precedence to overlay variables or anything from the current scope
-     * @var array|null
+     *
+     * @var array
      */
     protected $underlay;
 
-    public function __construct($item, $overlay = null, $underlay = null, $inheritedScope = null)
+    /**
-    {
+     * @var object $item
+     * @var array $overlay
+     * @var array $underlay
+     * @var SSViewer_Scope $inheritedScope
+     */
+    public function __construct(
+        $item,
+        array $overlay = null,
+        array $underlay = null,
+        SSViewer_Scope $inheritedScope = null
+    ) {
         parent::__construct($item, $inheritedScope);
 
-        // Build up global property providers array only once per request
+        $this->overlay = $overlay ?: [];
-        if (self::$globalProperties === null) {
+        $this->underlay = $underlay ?: [];
-            self::$globalProperties = array();
-            // Get all the exposed variables from all classes that implement the TemplateGlobalProvider interface
-            $this->createCallableArray(
-                self::$globalProperties,
-                TemplateGlobalProvider::class,
-                "get_template_global_variables"
-            );
-        }
 
-        // Build up iterator property providers array only once per request
+        $this->cacheGlobalProperties();
-        if (self::$iteratorProperties === null) {
+        $this->cacheIteratorProperties();
-            self::$iteratorProperties = array();
-            // Get all the exposed variables from all classes that implement the TemplateIteratorProvider interface
-            // //call non-statically
-            $this->createCallableArray(
-                self::$iteratorProperties,
-                TemplateIteratorProvider::class,
-                "get_template_iterator_variables",
-                true
-            );
-        }
-
-        $this->overlay = $overlay ? $overlay : array();
-        $this->underlay = $underlay ? $underlay : array();
     }
 
-    protected function createCallableArray(&$extraArray, $interfaceToQuery, $variableMethod, $createObject = false)
+    /**
+     * Build cache of global properties
+     */
+    protected function cacheGlobalProperties()
     {
-        $implementers = ClassInfo::implementorsOf($interfaceToQuery);
+        if (self::$globalProperties !== null) {
-        if ($implementers) {
+            return;
-            foreach ($implementers as $implementer) {
+        }
+
+        self::$globalProperties = $this->getPropertiesFromProvider(
+            TemplateGlobalProvider::class,
+            'get_template_global_variables'
+        );
+    }
+
+    /**
+     * Build cache of global iterator properties
+     */
+    protected function cacheIteratorProperties()
+    {
+        if (self::$iteratorProperties !== null) {
+            return;
+        }
+
+        self::$iteratorProperties = $this->getPropertiesFromProvider(
+            TemplateIteratorProvider::class,
+            'get_template_iterator_variables',
+            true // Call non-statically
+        );
+    }
+
+    /**
+     * @var string $interfaceToQuery
+     * @var string $variableMethod
+     * @var boolean $createObject
+     * @return array
+     */
+    protected function getPropertiesFromProvider($interfaceToQuery, $variableMethod, $createObject = false)
+    {
+        $methods = [];
+
+        $implementors = ClassInfo::implementorsOf($interfaceToQuery);
+        if ($implementors) {
+            foreach ($implementors as $implementor) {
                 // Create a new instance of the object for method calls
                 if ($createObject) {
-                    $implementer = new $implementer();
+                    $implementor = new $implementor();
+                    $exposedVariables = $implementor->$variableMethod();
+                } else {
+                    $exposedVariables = $implementor::$variableMethod();
                 }
 
-                // Get the exposed variables
-                $exposedVariables = call_user_func(array($implementer, $variableMethod));
-
                 foreach ($exposedVariables as $varName => $details) {
                     if (!is_array($details)) {
-                        $details = array(
+                        $details = [
                             'method' => $details,
                             'casting' => ViewableData::config()->uninherited('default_cast')
-                        );
+                        ];
                     }
 
-                    // If just a value (and not a key => value pair), use it for both key and value
+                    // If just a value (and not a key => value pair), use method name for both key and value
                     if (is_numeric($varName)) {
                         $varName = $details['method'];
                     }
 
                     // Add in a reference to the implementing class (might be a string class name or an instance)
-                    $details['implementer'] = $implementer;
+                    $details['implementor'] = $implementor;
 
                     // And a callable array
                     if (isset($details['method'])) {
-                        $details['callable'] = array($implementer, $details['method']);
+                        $details['callable'] = [$implementor, $details['method']];
                     }
 
                     // Save with both uppercase & lowercase first letter, so either works
                     $lcFirst = strtolower($varName[0]) . substr($varName, 1);
-                    $extraArray[$lcFirst] = $details;
+                    $result[$lcFirst] = $details;
-                    $extraArray[ucfirst($varName)] = $details;
+                    $result[ucfirst($varName)] = $details;
                 }
             }
         }
+
+        return $result;
     }
 
     /**
-     * Get the injected value
+     * Look up injected value - it may be part of an "overlay" (arguments passed to <% include %>),
+     * set on the current item, part of an "underlay" ($Layout or $Content), or an iterator/global property
      *
      * @param string $property Name of property
      * @param array $params
      * @param bool $cast If true, an object is always returned even if not an object.
-     * @return array Result array with the keys 'value' for raw value, or 'obj' if contained in an object
+     * @return array|null
-     * @throws InvalidArgumentException
      */
-    public function getInjectedValue($property, $params, $cast = true)
+    public function getInjectedValue($property, array $params, $cast = true)
     {
         // Get source for this value
         $source = $this->getValueSource($property);
@@ -139,7 +170,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope
         // Look up the value - either from a callable, or from a directly provided value
         $res = [];
         if (isset($source['callable'])) {
-            $res['value'] = call_user_func_array($source['callable'], $params);
+            $res['value'] = $source['callable'](...$params);
         } elseif (isset($source['value'])) {
             $res['value'] = $source['value'];
         } else {
@@ -152,6 +183,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope
         if ($cast) {
             $res['obj'] = $this->castValue($res['value'], $source);
         }
+
         return $res;
     }
 
@@ -238,6 +270,9 @@ class SSViewer_DataPresenter extends SSViewer_Scope
         return $this;
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function getObj($name, $arguments = [], $cache = false, $cacheName = null)
     {
         $result = $this->getInjectedValue($name, (array)$arguments);
@@ -247,34 +282,31 @@ class SSViewer_DataPresenter extends SSViewer_Scope
         return parent::getObj($name, $arguments, $cache, $cacheName);
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function __call($name, $arguments)
     {
-        //extract the method name and parameters
+        // Extract the method name and parameters
-        $property = $arguments[0];  //the name of the public function being called
+        $property = $arguments[0];  // The name of the public function being called
 
-        //the public function parameters in an array
+        // The public function parameters in an array
-        if (isset($arguments[1]) && $arguments[1] != null) {
+        $params = (isset($arguments[1])) ? (array)$arguments[1] : [];
-            $params = $arguments[1];
-        } else {
-            $params = array();
-        }
 
         $val = $this->getInjectedValue($property, $params);
         if ($val) {
             $obj = $val['obj'];
             if ($name === 'hasValue') {
-                $res = $obj instanceof ViewableData
+                $result = ($obj instanceof ViewableData) ? $obj->exists() : (bool)$obj;
-                    ? $obj->exists()
-                    : (bool)$obj;
             } else {
-                // XML_val
+                $result = $obj->forTemplate(); // XML_val
-                $res = $obj->forTemplate();
             }
+
             $this->resetLocalScope();
-            return $res;
+            return $result;
-        } else {
-            return parent::__call($name, $arguments);
         }
+
+        return parent::__call($name, $arguments);
     }
 
     /**
@@ -336,18 +368,18 @@ class SSViewer_DataPresenter extends SSViewer_Scope
         // Then for iterator-specific overrides
         if (array_key_exists($property, self::$iteratorProperties)) {
             $source = self::$iteratorProperties[$property];
-            /** @var TemplateIteratorProvider $implementer */
+            /** @var TemplateIteratorProvider $implementor */
-            $implementer = $source['implementer'];
+            $implementor = $source['implementor'];
             if ($this->itemIterator) {
                 // Set the current iterator position and total (the object instance is the first item in
                 // the callable array)
-                $implementer->iteratorProperties(
+                $implementor->iteratorProperties(
                     $this->itemIterator->key(),
                     $this->itemIteratorTotal
                 );
             } else {
                 // If we don't actually have an iterator at the moment, act like a list of length 1
-                $implementer->iteratorProperties(0, 1);
+                $implementor->iteratorProperties(0, 1);
             }
             return $source;
         }

src/View/SSViewer_FromString.php

@@ -9,9 +9,9 @@ use SilverStripe\Core\Config\Config;
  */
 class SSViewer_FromString extends SSViewer
 {
-
     /**
      * The global template caching behaviour if no instance override is specified
+     *
      * @config
      * @var bool
      */
@@ -19,6 +19,7 @@ class SSViewer_FromString extends SSViewer
 
     /**
      * The template to use
+     *
      * @var string
      */
     protected $content;
@@ -30,6 +31,10 @@ class SSViewer_FromString extends SSViewer
      */
     protected $cacheTemplate;
 
+    /**
+     * @param string $content
+     * @param TemplateParser $parser
+     */
     public function __construct($content, TemplateParser $parser = null)
     {
         if ($parser) {
@@ -39,6 +44,9 @@ class SSViewer_FromString extends SSViewer
         $this->content = $content;
     }
 
+    /**
+     * {@inheritdoc}
+     */
     public function process($item, $arguments = null, $scope = null)
     {
         $hash = sha1($this->content);

src/View/SSViewer_Scope.php

@@ -26,7 +26,6 @@ use Iterator;
  */
 class SSViewer_Scope
 {
-
     const ITEM = 0;
     const ITEM_ITERATOR = 1;
     const ITEM_ITERATOR_TOTAL = 2;
@@ -35,12 +34,18 @@ class SSViewer_Scope
     const CURRENT_INDEX = 5;
     const ITEM_OVERLAY = 6;
 
-    // The stack of previous "global" items
+    /**
-    // An indexed array of item, item iterator, item iterator total, pop index, up index, current index & parent overlay
+     * The stack of previous items ("scopes") - an indexed array of: item, item iterator, item iterator total,
-    private $itemStack = array();
+     * pop index, up index, current index & parent overlay
+     *
+     * @var array
+     */
+    private $itemStack = [];
 
     /**
      * The current "global" item (the one any lookup starts from)
+     *
+     * @var object
      */
     protected $item;
 
@@ -51,44 +56,88 @@ class SSViewer_Scope
      */
     protected $itemIterator;
 
-    //Total number of items in the iterator
+    /**
+     * Total number of items in the iterator
+     *
+     * @var int
+     */
     protected $itemIteratorTotal;
 
-    // A pointer into the item stack for which item should be scope on the next pop call
+    /**
+     * A pointer into the item stack for the item that will become the active scope on the next pop call
+     *
+     * @var int
+     */
     private $popIndex;
 
-    // A pointer into the item stack for which item is "up" from this one
+    /**
-    private $upIndex = null;
+     * A pointer into the item stack for which item is "up" from this one
+     *
+     * @var int
+     */
+    private $upIndex;
 
-    // A pointer into the item stack for which item is this one (or null if not in stack yet)
+    /**
-    private $currentIndex = null;
+     * A pointer into the item stack for which the active item (or null if not in stack yet)
+     *
+     * @var int
+     */
+    private $currentIndex;
 
-    private $localIndex;
+    /**
+     * A store of copies of the main item stack, so it's preserved during a lookup from local scope
+     * (which may push/pop items to/from the main item stack)
+     *
+     * @var array
+     */
+    private $localStack = [];
 
-    public function __construct($item, $inheritedScope = null)
+    /**
+     * The index of the current item in the main item stack, so we know where to restore the scope
+     * stored in $localStack.
+     *
+     * @var int
+     */
+    private $localIndex = 0;
+
+    /**
+     * @var object $item
+     * @var SSViewer_Scope $inheritedScope
+     */
+    public function __construct($item, SSViewer_Scope $inheritedScope = null)
     {
         $this->item = $item;
-        $this->localIndex = 0;
+
-        $this->localStack = array();
+        $this->itemIterator = ($inheritedScope) ? $inheritedScope->itemIterator : null;
-        if ($inheritedScope instanceof SSViewer_Scope) {
+        $this->itemIteratorTotal = ($inheritedScope) ? $inheritedScope->itemIteratorTotal : 0;
-            $this->itemIterator = $inheritedScope->itemIterator;
+        $this->itemStack[] = [$this->item, $this->itemIterator, $this->itemIteratorTotal, null, null, 0];
-            $this->itemIteratorTotal = $inheritedScope->itemIteratorTotal;
-            $this->itemStack[] = array($this->item, $this->itemIterator, $this->itemIteratorTotal, null, null, 0);
-        } else {
-            $this->itemStack[] = array($this->item, null, 0, null, null, 0);
-        }
     }
 
+    /**
+     * Returns the current "active" item
+     *
+     * @return object
+     */
     public function getItem()
     {
         return $this->itemIterator ? $this->itemIterator->current() : $this->item;
     }
 
-    /** Called at the start of every lookup chain by SSTemplateParser to indicate a new lookup from local scope */
+    /**
+     * Called at the start of every lookup chain by SSTemplateParser to indicate a new lookup from local scope
+     *
+     * @return self
+     */
     public function locally()
     {
-        list($this->item, $this->itemIterator, $this->itemIteratorTotal, $this->popIndex, $this->upIndex,
+        list(
-            $this->currentIndex) = $this->itemStack[$this->localIndex];
+            $this->item,
+            $this->itemIterator,
+            $this->itemIteratorTotal,
+            $this->popIndex,
+            $this->upIndex,
+            $this->currentIndex
+        ) = $this->itemStack[$this->localIndex];
 
         // Remember any  un-completed (resetLocalScope hasn't been called) lookup chain. Even if there isn't an
         // un-completed chain we need to store an empty item, as resetLocalScope doesn't know the difference later
@@ -97,16 +146,33 @@ class SSViewer_Scope
         return $this;
     }
 
+    /**
+     * Reset the local scope - restores saved state to the "global" item stack. Typically called after
+     * a lookup chain has been completed
+     */
     public function resetLocalScope()
     {
+        // Restore previous un-completed lookup chain if set
         $previousLocalState = $this->localStack ? array_pop($this->localStack) : null;
-
         array_splice($this->itemStack, $this->localIndex + 1, count($this->itemStack), $previousLocalState);
 
-        list($this->item, $this->itemIterator, $this->itemIteratorTotal, $this->popIndex, $this->upIndex,
+        list(
-            $this->currentIndex) = end($this->itemStack);
+            $this->item,
+            $this->itemIterator,
+            $this->itemIteratorTotal,
+            $this->popIndex,
+            $this->upIndex,
+            $this->currentIndex
+        ) = end($this->itemStack);
     }
 
+    /**
+     * @param string $name
+     * @param array $arguments
+     * @param bool $cache
+     * @param string $cacheName
+     * @return mixed
+     */
     public function getObj($name, $arguments = [], $cache = false, $cacheName = null)
     {
         $on = $this->itemIterator ? $this->itemIterator->current() : $this->item;
@@ -128,15 +194,25 @@ class SSViewer_Scope
                     user_error('Up called when we\'re already at the top of the scope', E_USER_ERROR);
                 }
 
-                list($this->item, $this->itemIterator, $this->itemIteratorTotal, $unused2, $this->upIndex,
+                list(
-                    $this->currentIndex) = $this->itemStack[$this->upIndex];
+                    $this->item,
+                    $this->itemIterator,
+                    $this->itemIteratorTotal,
+                    /* dud */,
+                    $this->upIndex,
+                    $this->currentIndex
+                ) = $this->itemStack[$this->upIndex];
                 break;
-
             case 'Top':
-                list($this->item, $this->itemIterator, $this->itemIteratorTotal, $unused2, $this->upIndex,
+                list(
-                    $this->currentIndex) = $this->itemStack[0];
+                    $this->item,
+                    $this->itemIterator,
+                    $this->itemIteratorTotal,
+                    /* dud */,
+                    $this->upIndex,
+                    $this->currentIndex
+                ) = $this->itemStack[0];
                 break;
-
             default:
                 $this->item = $this->getObj($name, $arguments, $cache, $cacheName);
                 $this->itemIterator = null;
@@ -145,14 +221,14 @@ class SSViewer_Scope
                 break;
         }
 
-        $this->itemStack[] = array(
+        $this->itemStack[] = [
             $this->item,
             $this->itemIterator,
             $this->itemIteratorTotal,
             null,
             $this->upIndex,
             $this->currentIndex
-        );
+        ];
         return $this;
     }
 
@@ -169,6 +245,11 @@ class SSViewer_Scope
         return $result;
     }
 
+    /**
+     * Jump to the last item in the stack, called when a new item is added before a loop/with
+     *
+     * @return self
+     */
     public function pushScope()
     {
         $newLocalIndex = count($this->itemStack) - 1;
@@ -183,6 +264,11 @@ class SSViewer_Scope
         return $this;
     }
 
+    /**
+     * Jump back to "previous" item in the stack, called after a loop/with block
+     *
+     * @return self
+     */
     public function popScope()
     {
         $this->localIndex = $this->popIndex;
@@ -191,6 +277,11 @@ class SSViewer_Scope
         return $this;
     }
 
+    /**
+     * Fast-forwards the current iterator to the next item
+     *
+     * @return mixed
+     */
     public function next()
     {
         if (!$this->item) {
@@ -205,7 +296,7 @@ class SSViewer_Scope
             }
 
             $this->itemStack[$this->localIndex][SSViewer_Scope::ITEM_ITERATOR] = $this->itemIterator;
-            $this->itemIteratorTotal = iterator_count($this->itemIterator); //count the total number of items
+            $this->itemIteratorTotal = iterator_count($this->itemIterator); // Count the total number of items
             $this->itemStack[$this->localIndex][SSViewer_Scope::ITEM_ITERATOR_TOTAL] = $this->itemIteratorTotal;
             $this->itemIterator->rewind();
         } else {
@@ -217,13 +308,19 @@ class SSViewer_Scope
         if (!$this->itemIterator->valid()) {
             return false;
         }
+
         return $this->itemIterator->key();
     }
 
+    /**
+     * @param string $name
+     * @param array $arguments
+     * @return mixed
+     */
     public function __call($name, $arguments)
     {
         $on = $this->itemIterator ? $this->itemIterator->current() : $this->item;
-        $retval = $on ? call_user_func_array(array($on, $name), $arguments) : null;
+        $retval = $on ? $on->$name(...$arguments) : null;
 
         $this->resetLocalScope();
         return $retval;