Merge pull request #59 from sanmai/fix-include-obj-rebase

Context: defer object-to-string conversion

Author: sanmai

CHANGELOG

@@ -1,6 +1,11 @@
 * master
 
+* 1.4.0 (2017-09-25)
+
  * Dropped support for EOL'ed versions of PHP (< 5.6)
+ * Arrays won't be silently cast to string as 'Array' anymore
+ * Complex objects could now be passed between templates and to filters
+ * Additional test coverage
 
 * 1.3.1 (2017-09-23)
 

src/Liquid/AbstractBlock.php

@@ -114,7 +114,17 @@ protected function renderAll(array $list, Context $context) {
 		$result = '';
 
 		foreach ($list as $token) {
-			$result .= (is_object($token) && method_exists($token, 'render')) ? $token->render($context) : $token;
+			if (method_exists($token, 'render')) {
+				$value = $token->render($context);
+			} else {
+				$value = $token;
+			}
+
+			if (is_array($value)) {
+				throw new LiquidException("Implicit rendering of arrays not supported. Use index operator.");
+			}
+
+			$result .= $value;
 
 			if (isset($context->registers['break'])) {
 				break;

src/Liquid/AbstractTag.php

@@ -66,9 +66,7 @@ public function parse(array &$tokens) {
 	 *
 	 * @return string
 	 */
-	public function render(Context $context) {
-		return '';
-	}
+	abstract public function render(Context $context);
 
 	/**
 	 * Extracts tag attributes from a markup string.

src/Liquid/Context.php

@@ -234,6 +234,9 @@ private function fetch($key) {
 	 *
 	 * @param string $key
 	 *
+	 * @see Decision::stringValue
+	 * @see AbstractBlock::renderAll
+	 *
 	 * @throws LiquidException
 	 * @return mixed
 	 */
@@ -333,23 +336,22 @@ private function variable($key) {
 			// we'll try casting this object in the next iteration
 		}
 
-		// Traversable objects are taken care of inside filters
-		if ($object instanceof \Traversable) {
-			return $object;
-		}
-
-		// finally, resolve an object to a string or a plain value
-		if (method_exists($object, '__toString')) {
-			$object = (string) $object;
-		} elseif (method_exists($object, 'toLiquid')) {
+		// lastly, try to get an embedded value of an object
+		// value could be of any type, not just string, so we have to do this
+		// conversion here, not laster in AbstractBlock::renderAll
+		if (method_exists($object, 'toLiquid')) {
 			$object = $object->toLiquid();
 		}
 
-		// if everything else fails, throw up
-		if (is_object($object)) {
-			$class = get_class($object);
-			throw new LiquidException("Value of type $class has no `toLiquid` nor `__toString` methods");
-		}
+		/*
+		 * Before here were checks for object types and object to string conversion.
+		 *
+		 * Now we just return what we have:
+		 * - Traversable objects are taken care of inside filters
+		 * - Object-to-string conversion is handled at the last moment in Decision::stringValue, and in AbstractBlock::renderAll
+		 *
+		 * This way complex objects could be passed between templates and to filters
+		 */
 
 		return $object;
 	}

src/Liquid/Decision.php

@@ -44,7 +44,9 @@ private function stringValue($value) {
 			if (method_exists($value, '__toString')) {
 				$value = (string) $value;
 			} else {
-				throw new LiquidException("Cannot convert $value to string"); // harry
+				// toLiquid is handled in Context::variable
+				$class = get_class($value);
+				throw new LiquidException("Value of type $class has no `toLiquid` nor `__toString` methods");
 			}
 		}
 

tests/Liquid/ContextTest.php

@@ -166,12 +166,9 @@ public function testGetSetObject() {
 		$this->assertNull($this->context->get('object.invalid'));
 	}
 
-	/**
-	 * @expectedException \Liquid\LiquidException
-	 */
-	public function testFinalVariableIsObject() {
+	public function testFinalVariableCanBeObject() {
 		$this->context->set('test', (object) array('value' => (object) array()));
-		$this->context->get('test.value');
+		$this->assertInstanceOf(\stdClass::class, $this->context->get('test.value'));
 	}
 
 	public function testVariables() {

tests/Liquid/Tag/TagCaseTest.php

@@ -20,6 +20,13 @@ public function __toString() {
 	}
 }
 
+class HasToLiquid
+{
+	public function toLiquid() {
+		return "100";
+	}
+}
+
 class TagCaseTest extends TestCase
 {
 	public function testCase() {
@@ -78,4 +85,8 @@ public function testObject() {
 	public function testStringable() {
 		$this->assertTemplateResult('hit', '{% case variable %}{% when 100 %}hit{% endcase %}', array('variable' => new Stringable()));
 	}
+
+	public function testToLiquid() {
+		$this->assertTemplateResult('hit', '{% case variable %}{% when 100 %}hit{% endcase %}', array('variable' => new HasToLiquid()));
+	}
 }

tests/Liquid/Tag/TagIncludeTest.php

@@ -16,21 +16,17 @@
 use Liquid\Liquid;
 use Liquid\Cache\Local;
 use Liquid\FileSystem\Virtual;
+use Liquid\TestFileSystem;
 
 class TagIncludeTest extends TestCase
 {
 	private $fs;
 
 	protected function setUp() {
-		$this->fs = new Virtual(function ($templatePath) {
-			if ($templatePath == 'inner') {
-				return "Inner: {{ inner }}{{ other }}";
-			}
-
-			if ($templatePath == 'example') {
-				return "Example: {% include 'inner' %}";
-			}
-		});
+		$this->fs = TestFileSystem::fromArray(array(
+			'inner' => "Inner: {{ inner }}{{ other }}",
+			'example' => "Example: {% include 'inner' %}",
+		));
 	}
 
 	protected function tearDown() {
@@ -115,4 +111,60 @@ public function testIncludeTemplateFile() {
 		// template include inserts a new line
 		$this->assertEquals("test content\n", $template->render());
 	}
+
+	public function testIncludePassPlainValue() {
+		$template = new Template();
+		$template->setFileSystem(TestFileSystem::fromArray(array(
+			'inner' => "[{{ other }}]",
+			'example' => "({% include 'inner' other:var %})",
+		)));
+
+		$template->parse("{% include 'example' %}");
+
+		$output = $template->render(array("var" => "test"));
+		$this->assertEquals("([test])", $output);
+	}
+
+	/**
+	 * @expectedException \Liquid\LiquidException
+	 * @expectedExceptionMessage Use index operator
+	 */
+	public function testIncludePassArrayWithoutIndex() {
+
+		$template = new Template();
+		$template->setFileSystem(TestFileSystem::fromArray(array(
+			'inner' => "[{{ other }}]",
+			'example' => "({% include 'inner' other:var %})",
+		)));
+
+		$template->parse("{% include 'example' %}");
+		$template->render(array("var" => array("a", "b", "c")));
+	}
+
+	public function testIncludePassArrayWithIndex() {
+
+		$template = new Template();
+		$template->setFileSystem(TestFileSystem::fromArray(array(
+			'inner' => "[{{ other[0] }}]",
+			'example' => "({% include 'inner' other:var %})",
+		)));
+
+		$template->parse("{% include 'example' %}");
+
+		$output = $template->render(array("var" => array("a", "b", "c")));
+		$this->assertEquals("([a])", $output);
+	}
+
+	public function testIncludePassObjectValue() {
+		$template = new Template();
+		$template->setFileSystem(TestFileSystem::fromArray(array(
+			'inner' => "[{{ other.a }}]",
+			'example' => "({% include 'inner' other:var %})",
+		)));
+
+		$template->parse("{% include 'example' %}");
+
+		$output = $template->render(array("var" => (object) array('a' => 'b')));
+		$this->assertEquals("([b])", $output);
+	}
 }

tests/Liquid/TestFileSystem.php

@@ -0,0 +1,29 @@
+<?php
+
+/**
+ * This file is part of the Liquid package.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ *
+ * @package Liquid
+ */
+
+namespace Liquid;
+
+use Liquid\FileSystem\Virtual;
+
+class TestFileSystem extends Virtual
+{
+	/** @return TestFileSystem */
+	public static function fromArray($array)
+	{
+		return new static(function ($path) use ($array) {
+			if (isset($array[$path])) {
+				return $array[$path];
+			}
+
+			return '';
+		});
+	}
+}