From f742efd602f310c37fdbe40c5ffcf4d61f5a4f52 Mon Sep 17 00:00:00 2001 From: nikic <+@ni-po.com> Date: Tue, 24 May 2011 12:48:47 +0200 Subject: [PATCH] Refactor GetAttr defined implementation to consider null defined --- lib/Twig/Node/Expression/Test.php | 2 - lib/Twig/Template.php | 47 +++++-- test/Twig/Tests/Fixtures/tests/defined.test | 15 ++- test/Twig/Tests/TemplateTest.php | 208 ++++++++++++++++----------- 4 files changed, 169 insertions(+), 103 deletions(-) diff --git a/lib/Twig/Node/Expression/Test.php b/lib/Twig/Node/Expression/Test.php index 3025209..204da25 100644 --- a/lib/Twig/Node/Expression/Test.php +++ b/lib/Twig/Node/Expression/Test.php @@ -33,9 +33,7 @@ class Twig_Node_Expression_Test extends Twig_Node_Expression } elseif ($this->getNode('node') instanceof Twig_Node_Expression_GetAttr) { $this->getNode('node')->setAttribute('is_defined_test', true); $compiler - ->raw('(null !== ') ->subcompile($this->getNode('node')) - ->raw(')') ; } else { throw new Twig_Error_Syntax('The "defined" test only works with simple variables', $this->getLine()); diff --git a/lib/Twig/Template.php b/lib/Twig/Template.php index 42387eb..aaef78a 100644 --- a/lib/Twig/Template.php +++ b/lib/Twig/Template.php @@ -224,8 +224,6 @@ abstract class Twig_Template implements Twig_TemplateInterface * @param array $context The context * @param string $item The variable to return from the context * - * @param mixed The variable value in the context - * * @throws Twig_Error_Runtime if the variable does not exist */ protected function getContext($context, $item) @@ -243,19 +241,29 @@ abstract class Twig_Template implements Twig_TemplateInterface * @param mixed $object The object or array from where to get the item * @param mixed $item The item to get from the array or object * @param array $arguments An array of arguments to pass if the item is an object method - * @param integer $type The type of attribute (@see Twig_TemplateInterface) - * @param Boolean $noStrictCheck Whether to throw an exception if the item does not exist ot not + * @param string $type The type of attribute (@see Twig_TemplateInterface) + * @param Boolean $isDefinedTest Whether this is only a defined check */ - protected function getAttribute($object, $item, array $arguments = array(), $type = Twig_TemplateInterface::ANY_CALL, $noStrictCheck = false) + protected function getAttribute($object, $item, array $arguments = array(), $type = Twig_TemplateInterface::ANY_CALL, $isDefinedTest = false) { // array if (Twig_TemplateInterface::METHOD_CALL !== $type) { - if ((is_array($object) || is_object($object) && $object instanceof ArrayAccess) && isset($object[$item])) { + if ((is_array($object) && array_key_exists($item, $object)) + || ($object instanceof ArrayAccess && isset($object[$item])) + ) { + if ($isDefinedTest) { + return true; + } + return $object[$item]; } if (Twig_TemplateInterface::ARRAY_CALL === $type) { - if (!$this->env->isStrictVariables() || $noStrictCheck) { + if ($isDefinedTest) { + return false; + } + + if (!$this->env->isStrictVariables()) { return null; } @@ -269,9 +277,14 @@ abstract class Twig_Template implements Twig_TemplateInterface } if (!is_object($object)) { - if (!$this->env->isStrictVariables() || $noStrictCheck) { + if ($isDefinedTest) { + return false; + } + + if (!$this->env->isStrictVariables()) { return null; } + throw new Twig_Error_Runtime(sprintf('Item "%s" for "%s" does not exist', $item, $object)); } @@ -291,7 +304,13 @@ abstract class Twig_Template implements Twig_TemplateInterface // object property if (Twig_TemplateInterface::METHOD_CALL !== $type) { - if (isset(self::$cache[$class]['properties'][$item]) || isset($object->$item)) { + if (isset(self::$cache[$class]['properties'][$item]) + || isset($object->$item) || array_key_exists($item, $object) + ) { + if ($isDefinedTest) { + return true; + } + if ($this->env->hasExtension('sandbox')) { $this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item); } @@ -311,13 +330,21 @@ abstract class Twig_Template implements Twig_TemplateInterface } elseif (isset(self::$cache[$class]['methods']['__call'])) { $method = $item; } else { - if (!$this->env->isStrictVariables() || $noStrictCheck) { + if ($isDefinedTest) { + return false; + } + + if (!$this->env->isStrictVariables()) { return null; } throw new Twig_Error_Runtime(sprintf('Method "%s" for object "%s" does not exist', $item, get_class($object))); } + if ($isDefinedTest) { + return true; + } + if ($this->env->hasExtension('sandbox')) { $this->env->getExtension('sandbox')->checkMethodAllowed($object, $method); } diff --git a/test/Twig/Tests/Fixtures/tests/defined.test b/test/Twig/Tests/Fixtures/tests/defined.test index 7d94b21..8f2b63a 100644 --- a/test/Twig/Tests/Fixtures/tests/defined.test +++ b/test/Twig/Tests/Fixtures/tests/defined.test @@ -5,21 +5,23 @@ {{ definedVar is not defined ? 'ko' : 'ok' }} {{ undefinedVar is defined ? 'ko' : 'ok' }} {{ undefinedVar is not defined ? 'ok' : 'ko' }} -{{ nullVar is defined ? 'ok' : 'ko' }} {{ zeroVar is defined ? 'ok' : 'ko' }} +{{ nullVar is defined ? 'ok' : 'ko' }} {{ nested.definedVar is defined ? 'ok' : 'ko' }} {{ nested.definedVar is not defined ? 'ko' : 'ok' }} {{ nested.undefinedVar is defined ? 'ko' : 'ok' }} {{ nested.undefinedVar is not defined ? 'ok' : 'ko' }} {{ nested.zeroVar is defined ? 'ok' : 'ko' }} +{{ nested.nullVar is defined ? 'ok' : 'ko' }} --DATA-- return array( - 'definedVar' => 'bar', - 'nullVar' => null, + 'definedVar' => 'defined', 'zeroVar' => 0, - 'nested' => array( - 'definedVar' => 'foo', - 'zeroVar' => 0 + 'nullVar' => null, + 'nested' => array( + 'definedVar' => 'defined', + 'zeroVar' => 0, + 'nullVar' => null, ) ); --EXPECT-- @@ -34,3 +36,4 @@ ok ok ok ok +ok diff --git a/test/Twig/Tests/TemplateTest.php b/test/Twig/Tests/TemplateTest.php index 06dd356..0089f9b 100644 --- a/test/Twig/Tests/TemplateTest.php +++ b/test/Twig/Tests/TemplateTest.php @@ -10,78 +10,116 @@ */ class Twig_Tests_TemplateTest extends PHPUnit_Framework_TestCase { - public function getUnknownPropertyOnArrayTests() + /** + * @dataProvider getGetAttributeTests + */ + public function testGetAttribute($defined, $value, $object, $item, $arguments, $type) { - $tests = array( - array(array('foo' => 'foo', 'bar' => 'value')), - array(new Twig_TemplateObjectArrayAccess()), - ); + $template = new Twig_TemplateTest(new Twig_Environment()); - return $tests; + $this->assertEquals($value, $template->getAttribute($object, $item, $arguments, $type)); } /** - * @dataProvider getUnknownPropertyOnArrayTests - * @expectedException Twig_Error_Runtime + * @dataProvider getGetAttributeTests */ - public function testUnknownPropertyOnArray($array) + public function testGetAttributeStrict($defined, $value, $object, $item, $arguments, $type) { - $env = new Twig_Environment(null, array('strict_variables' => true)); - $template = new Twig_TemplateTest($env); + $template = new Twig_TemplateTest( + new Twig_Environment(null, array('strict_variables' => true)) + ); + + if ($defined) { + $this->assertEquals($value, $template->getAttribute($object, $item, $arguments, $type)); + } else { + try { + $this->assertEquals($value, $template->getAttribute($object, $item, $arguments, $type)); - $template->getAttribute($array, 'unknown', array(), Twig_TemplateInterface::ARRAY_CALL); + throw new Exception('Expected Twig_Error_Runtime exception.'); + } catch (Twig_Error_Runtime $e) { } + } } /** * @dataProvider getGetAttributeTests */ - public function testGetAttribute($expected, $object, $item, $arguments, $type) + public function testGetAttributeDefined($defined, $value, $object, $item, $arguments, $type) { $template = new Twig_TemplateTest(new Twig_Environment()); - $this->assertEquals($expected, $template->getAttribute($object, $item, $arguments, $type)); + $this->assertEquals($defined, $template->getAttribute($object, $item, $arguments, $type, true)); + } + + /** + * @dataProvider getGetAttributeTests + */ + public function testGetAttributeDefinedStrict($defined, $value, $object, $item, $arguments, $type) + { + $template = new Twig_TemplateTest( + new Twig_Environment(null, array('strict_variables' => true)) + ); + + $this->assertEquals($defined, $template->getAttribute($object, $item, $arguments, $type, true)); } public function getGetAttributeTests() { - $array = array('foo' => 'foo'); - $object = new Twig_TemplateObject(); - $objectArray = new Twig_TemplateObjectArrayAccess(); - $objectMagic = new Twig_TemplateObjectMagic(); + $array = array( + 'defined' => 'defined', + 'zero' => 0, + 'null' => null, + ); + + $objectArray = new Twig_TemplateArrayAccessObject; + $stdObject = (object) $array; + $magicPropertyObject = new Twig_TemplateMagicPropertyObject; + $methodObject = new Twig_TemplateMethodObject; + $magicMethodObject = new Twig_TemplateMagicMethodObject; - $anyType = Twig_TemplateInterface::ANY_CALL; + $anyType = Twig_TemplateInterface::ANY_CALL; $methodType = Twig_TemplateInterface::METHOD_CALL; - $arrayType = Twig_TemplateInterface::ARRAY_CALL; - - $tests = array( - // ARRAY - array('foo', $array, 'foo', array(), $arrayType), - array(null, $array, 'foobar', array(), $arrayType), - array('foo', $objectArray, 'foo', array(), $arrayType), - array(null, $objectArray, 'foobar', array(), $arrayType), - - // METHOD - array('bar', $object, 'bar', array(), $methodType), - array('bar', $object, 'getBar', array(), $methodType), - array('bar', $object, 'getbar', array(), $methodType), - array('foobar', $object, 'foobar', array(), $methodType), - array('babar', $object, 'babar', array(), $methodType), - array('babarStatic', $object, 'babarStatic', array(), $methodType), - array('__call_baz', $objectMagic, 'baz', array(), $methodType), - array('__call_Baz', $objectMagic, 'Baz', array(), $methodType), - - // ANY - array('foo', $object, 'foo', array(), $anyType), - array('foo', $objectMagic, 'foo', array(), $anyType), - array('Foo', $objectMagic, 'Foo', array(), $anyType), - array('babar', $object, 'babar', array(), $anyType), - array(null, $object, 'null', array(), $anyType), + $arrayType = Twig_TemplateInterface::ARRAY_CALL; + + $basicTests = array( + // array(defined, value, property to fetch) + array(true, 'defined', 'defined'), + array(false, null, 'undefined'), + array(true, 0, 'zero'), + array(true, null, 'null'), ); + $testObjects = array( + // array(object, type of fetch) + array($array, $arrayType), + array($objectArray, $arrayType), + array($stdObject, $anyType), + array($magicPropertyObject, $anyType), + array($methodObject, $methodType), + ); + + $tests = array(); + foreach ($testObjects as $testObject) { + foreach ($basicTests as $test) { + $tests[] = array($test[0], $test[1], $testObject[0], $test[2], array(), $testObject[1]); + } + } + + // additional method tests + $tests = array_merge($tests, array( + array(true, 'defined', $methodObject, 'defined', array(), $methodType), + array(true, 'defined', $methodObject, 'DEFINED', array(), $methodType), + array(true, 'defined', $methodObject, 'getDefined', array(), $methodType), + array(true, 'defined', $methodObject, 'GETDEFINED', array(), $methodType), + array(true, 'static', $methodObject, 'static', array(), $methodType), + array(true, 'static', $methodObject, 'getStatic', array(), $methodType), + + array(true, '__call_undefined', $magicMethodObject, 'undefined', array(), $methodType), + array(true, '__call_UNDEFINED', $magicMethodObject, 'UNDEFINED', array(), $methodType), + )); // add the same tests for the any type foreach ($tests as $test) { - if ($anyType !== $test[4]) { - $test[4] = $anyType; + if ($anyType !== $test[5]) { + $test[5] = $anyType; $tests[] = $test; } } @@ -96,84 +134,84 @@ class Twig_TemplateTest extends Twig_Template { } - public function getAttribute($object, $item, array $arguments = array(), $type = Twig_TemplateInterface::ANY_CALL, $noStrictCheck = false, $lineno = -1) + public function getAttribute($object, $item, array $arguments = array(), $type = Twig_TemplateInterface::ANY_CALL, $isDefinedTest = false) { - return parent::getAttribute($object, $item, $arguments, $type); + return parent::getAttribute($object, $item, $arguments, $type, $isDefinedTest); } } -class Twig_TemplateObject +class Twig_TemplateArrayAccessObject implements ArrayAccess { - public $foo = 'foo'; - public $null = null; - protected $babar = 'babar...'; - static protected $babarStatic = 'babarStatic...'; + public $attributes = array( + 'defined' => 'defined', + 'zero' => 0, + 'null' => null, + ); - static public function getBabarStatic() - { - return 'babarStatic'; - } - - public function getBabar() + public function offsetExists($name) { - return 'babar'; + return array_key_exists($name, $this->attributes); } - public function getNull() + public function offsetGet($name) { - return 'null...'; + return array_key_exists($name, $this->attributes) ? $this->attributes[$name] : null; } - public function getBar() + public function offsetSet($name, $value) { - return 'bar'; } - public function fooBar() + public function offsetUnset($name) { - return 'foobar'; } } -class Twig_TemplateObjectArrayAccess implements ArrayAccess +class Twig_TemplateMagicPropertyObject { - public $attributes = array('foo' => 'foo'); + public $attributes = array( + 'defined' => 'defined', + 'zero' => 0, + 'null' => null, + ); - public function offsetExists($name) + public function __isset($name) { - return isset($this->attributes[$name]); + return array_key_exists($name, $this->attributes); } - public function offsetGet($name) + public function __get($name) { - return isset($this->attributes[$name]) ? $this->attributes[$name] : null; + return array_key_exists($name, $this->attributes) ? $this->attributes[$name] : null; } +} - public function offsetSet($name, $value) +class Twig_TemplateMethodObject +{ + public function getDefined() { + return 'defined'; } - public function offsetUnset($name) + public function getZero() { + return 0; } -} - -class Twig_TemplateObjectMagic -{ - public $attributes = array('foo' => 'foo', 'Foo' => 'Foo'); - public function __isset($name) + public function getNull() { - return isset($this->attributes[$name]); + return null; } - public function __get($name) + static public function getStatic() { - return isset($this->attributes[$name]) ? $this->attributes[$name] : null; + return 'static'; } +} - public function __call($method, $arguments) - { +class Twig_TemplateMagicMethodObject +{ + public function __call($method, $arguments) { return '__call_'.$method; } } -- 1.7.2.5