Refactor GetAttr defined implementation to consider null defined

diff --git a/lib/Twig/Node/Expression/Test.php b/lib/Twig/Node/Expression/Test.php
index 3025209..204da25 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;
     }
 }