From d6e822935205c44534054a2a96a31df2fbe40a09 Mon Sep 17 00:00:00 2001 From: Loz Calver Date: Fri, 5 Oct 2018 12:06:58 +0100 Subject: [PATCH] FIX: Fix type preservation in <% include %> arguments --- src/View/SSViewer_DataPresenter.php | 56 ++++---- tests/php/View/SSViewerTest.php | 121 +++++++++++------- .../Includes/SSViewerTestTypePreservation.ss | 1 + 3 files changed, 106 insertions(+), 72 deletions(-) create mode 100644 tests/php/View/SSViewerTest/templates/Includes/SSViewerTestTypePreservation.ss diff --git a/src/View/SSViewer_DataPresenter.php b/src/View/SSViewer_DataPresenter.php index 26cdf70c3..c5cd32db4 100644 --- a/src/View/SSViewer_DataPresenter.php +++ b/src/View/SSViewer_DataPresenter.php @@ -162,16 +162,17 @@ class SSViewer_DataPresenter extends SSViewer_Scope public function getInjectedValue($property, array $params, $cast = true) { // Get source for this value - $source = $this->getValueSource($property); - if (!$source) { + $result = $this->getValueSource($property); + if (!array_key_exists('source', $result)) { return null; } // Look up the value - either from a callable, or from a directly provided value + $source = $result['source']; $res = []; if (isset($source['callable'])) { $res['value'] = $source['callable'](...$params); - } elseif (isset($source['value'])) { + } elseif (array_key_exists('value', $source)) { $res['value'] = $source['value']; } else { throw new InvalidArgumentException( @@ -298,6 +299,8 @@ class SSViewer_DataPresenter extends SSViewer_Scope $obj = $val['obj']; if ($name === 'hasValue') { $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 { $result = $obj->forTemplate(); // XML_val } @@ -310,16 +313,18 @@ class SSViewer_DataPresenter extends SSViewer_Scope } /** - * Evaluate a template override + * Evaluate a template override. Returns an array where the presence of + * a 'value' key indiciates whether an override was successfully found, + * as null is a valid override value * * @param string $property Name of override requested * @param array $overrides List of overrides available - * @return null|array Null if not provided, or array with 'value' or 'callable' key + * @return array An array with a 'value' key if a value has been found, or empty if not */ protected function processTemplateOverride($property, $overrides) { - if (!isset($overrides[$property])) { - return null; + if (!array_key_exists($property, $overrides)) { + return []; } // Detect override type @@ -331,38 +336,40 @@ class SSViewer_DataPresenter extends SSViewer_Scope // Late override may yet return null if (!isset($override)) { - return null; + return []; } } - return [ 'value' => $override ]; + return ['value' => $override]; } /** - * Determine source to use for getInjectedValue + * Determine source to use for getInjectedValue. Returns an array where the presence of + * a 'source' key indiciates whether a value source was successfully found, as a source + * may be a null value returned from an override * * @param string $property - * @return array|null + * @return array An array with a 'source' key if a value source has been found, or empty if not */ protected function getValueSource($property) { // Check for a presenter-specific override - $overlay = $this->processTemplateOverride($property, $this->overlay); - if (isset($overlay)) { - return $overlay; + $result = $this->processTemplateOverride($property, $this->overlay); + if (array_key_exists('value', $result)) { + return ['source' => $result]; } // Check if the method to-be-called exists on the target object - if so, don't check any further // injection locations $on = $this->itemIterator ? $this->itemIterator->current() : $this->item; if (isset($on->$property) || method_exists($on, $property ?? '')) { - return null; + return []; } // Check for a presenter-specific override - $underlay = $this->processTemplateOverride($property, $this->underlay); - if (isset($underlay)) { - return $underlay; + $result = $this->processTemplateOverride($property, $this->underlay); + if (array_key_exists('value', $result)) { + return ['source' => $result]; } // Then for iterator-specific overrides @@ -381,16 +388,19 @@ class SSViewer_DataPresenter extends SSViewer_Scope // If we don't actually have an iterator at the moment, act like a list of length 1 $implementor->iteratorProperties(0, 1); } - return $source; + + return ($source) ? ['source' => $source] : []; } // And finally for global overrides if (array_key_exists($property, self::$globalProperties)) { - return self::$globalProperties[$property]; //get the method call + return [ + 'source' => self::$globalProperties[$property] // get the method call + ]; } // No value - return null; + return []; } /** @@ -402,8 +412,8 @@ class SSViewer_DataPresenter extends SSViewer_Scope */ protected function castValue($value, $source) { - // Already cast - if (is_object($value)) { + // If the value has already been cast, is null, or is a non-string scalar + if (is_object($value) || is_null($value) || (is_scalar($value) && !is_string($value))) { return $value; } diff --git a/tests/php/View/SSViewerTest.php b/tests/php/View/SSViewerTest.php index f044eab7d..766661860 100644 --- a/tests/php/View/SSViewerTest.php +++ b/tests/php/View/SSViewerTest.php @@ -713,63 +713,86 @@ after' ); } - public function testTypesArePreserved() + public function typePreservationDataProvider() + { + return [ + // Null + ['NULL:', 'null'], + ['NULL:', 'NULL'], + // Booleans + ['boolean:1', 'true'], + ['boolean:1', 'TRUE'], + ['boolean:', 'false'], + ['boolean:', 'FALSE'], + // Strings which loosely look like booleans + ['string:truthy', 'truthy'], + ['string:falsy', 'falsy'], + // Integers + ['integer:0', '0'], + ['integer:1', '1'], + ['integer:15', '15'], + ['integer:-15', '-15'], + // Octal integers + ['integer:83', '0123'], + ['integer:-83', '-0123'], + // Hexadecimal integers + ['integer:26', '0x1A'], + ['integer:-26', '-0x1A'], + // Binary integers + ['integer:255', '0b11111111'], + ['integer:-255', '-0b11111111'], + // Floats (aka doubles) + ['double:0', '0.0'], + ['double:1', '1.0'], + ['double:15.25', '15.25'], + ['double:-15.25', '-15.25'], + ['double:1200', '1.2e3'], + ['double:-1200', '-1.2e3'], + ['double:0.07', '7E-2'], + ['double:-0.07', '-7E-2'], + // Explicitly quoted strings + ['string:0', '"0"'], + ['string:1', '\'1\''], + ['string:foobar', '"foobar"'], + ['string:foo bar baz', '"foo bar baz"'], + // Implicit strings + ['string:foobar', 'foobar'], + ['string:foo bar baz', 'foo bar baz'] + ]; + } + + /** + * @dataProvider typePreservationDataProvider + */ + public function testTypesArePreserved($expected, $templateArg) { $data = new ArrayData([ 'Test' => new TestViewableData() ]); - // Null - $this->assertEquals('NULL:', $this->render('$Test.Type(null)', $data)); - $this->assertEquals('NULL:', $this->render('$Test.Type(NULL)', $data)); + $this->assertEquals($expected, $this->render("\$Test.Type({$templateArg})", $data)); + } - // Booleans - $this->assertEquals('boolean:1', $this->render('$Test.Type(TRUE)', $data)); - $this->assertEquals('boolean:1', $this->render('$Test.Type(true)', $data)); - $this->assertEquals('boolean:', $this->render('$Test.Type(FALSE)', $data)); - $this->assertEquals('boolean:', $this->render('$Test.Type(false)', $data)); + /** + * @dataProvider typePreservationDataProvider + */ + public function testTypesArePreservedAsIncludeArguments($expected, $templateArg) + { + $data = new ArrayData([ + 'Test' => new TestViewableData() + ]); - // Strings which loosely look like booleans - $this->assertEquals('string:truthy', $this->render('$Test.Type(truthy)', $data)); - $this->assertEquals('string:falsy', $this->render('$Test.Type(falsy)', $data)); + $this->assertEquals( + $expected, + $this->render("<% include SSViewerTestTypePreservation Argument={$templateArg} %>", $data) + ); + } - // Integers - $this->assertEquals('integer:0', $this->render('$Test.Type(0)', $data)); - $this->assertEquals('integer:1', $this->render('$Test.Type(1)', $data)); - $this->assertEquals('integer:15', $this->render('$Test.Type(15)', $data)); - $this->assertEquals('integer:-15', $this->render('$Test.Type(-15)', $data)); - - # Octal integers - $this->assertEquals('integer:83', $this->render('$Test.Type(0123)', $data)); - $this->assertEquals('integer:-83', $this->render('$Test.Type(-0123)', $data)); - - # Hexadecimal integers - $this->assertEquals('integer:26', $this->render('$Test.Type(0x1A)', $data)); - $this->assertEquals('integer:-26', $this->render('$Test.Type(-0x1A)', $data)); - - # Binary integers - $this->assertEquals('integer:255', $this->render('$Test.Type(0b11111111)', $data)); - $this->assertEquals('integer:-255', $this->render('$Test.Type(-0b11111111)', $data)); - - // Floats (aka doubles) - $this->assertEquals('double:0', $this->render('$Test.Type(0.0)', $data)); - $this->assertEquals('double:1', $this->render('$Test.Type(1.0)', $data)); - $this->assertEquals('double:15.25', $this->render('$Test.Type(15.25)', $data)); - $this->assertEquals('double:-15.25', $this->render('$Test.Type(-15.25)', $data)); - $this->assertEquals('double:1200', $this->render('$Test.Type(1.2e3)', $data)); - $this->assertEquals('double:-1200', $this->render('$Test.Type(-1.2e3)', $data)); - $this->assertEquals('double:0.07', $this->render('$Test.Type(7E-2)', $data)); - $this->assertEquals('double:-0.07', $this->render('$Test.Type(-7E-2)', $data)); - - // Explicitly quoted strings - $this->assertEquals('string:0', $this->render('$Test.Type("0")', $data)); - $this->assertEquals('string:1', $this->render('$Test.Type(\'1\')', $data)); - $this->assertEquals('string:foobar', $this->render('$Test.Type("foobar")', $data)); - $this->assertEquals('string:foo bar baz', $this->render('$Test.Type("foo bar baz")', $data)); - - // Implicit strings - $this->assertEquals('string:foobar', $this->render('$Test.Type(foobar)', $data)); - $this->assertEquals('string:foo bar baz', $this->render('$Test.Type(foo bar baz)', $data)); + public function testTypePreservationInConditionals() + { + $data = new ArrayData([ + 'Test' => new TestViewableData() + ]); // Types in conditionals $this->assertEquals('pass', $this->render('<% if true %>pass<% else %>fail<% end_if %>', $data)); diff --git a/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestTypePreservation.ss b/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestTypePreservation.ss new file mode 100644 index 000000000..0f9e2f8bf --- /dev/null +++ b/tests/php/View/SSViewerTest/templates/Includes/SSViewerTestTypePreservation.ss @@ -0,0 +1 @@ +$Test.Type($Argument)