Merge remote-tracking branch 'origin/ignore-non-functions'

* origin/ignore-non-functions:
  Refactor php error collection to it own class
  Fix method names and constants
  Fix data provider array
  Trigger a E_USER_NOTICE for invalid arguments passed to then()
  Ignore non-function callback arguments

Author: igorw

phpunit.xml.dist

@@ -4,7 +4,7 @@
          backupStaticAttributes="false"
          colors="true"
          convertErrorsToExceptions="true"
-         convertNoticesToExceptions="true"
+         convertNoticesToExceptions="false"
          convertWarningsToExceptions="true"
          processIsolation="false"
          stopOnFailure="false"

src/React/Promise/Deferred.php

@@ -18,7 +18,7 @@ public function then($fulfilledHandler = null, $errorHandler = null, $progressHa
 
         $deferred = new static();
 
-        if ($progressHandler) {
+        if (is_callable($progressHandler)) {
             $progHandler = function ($update) use ($deferred, $progressHandler) {
                 try {
                     $deferred->progress(call_user_func($progressHandler, $update));
@@ -27,6 +27,10 @@ public function then($fulfilledHandler = null, $errorHandler = null, $progressHa
                 }
             };
         } else {
+            if (null !== $progressHandler) {
+                trigger_error('Invalid $progressHandler argument passed to then(), must be null or callable.', E_USER_NOTICE);
+            }
+
             $progHandler = array($deferred, 'progress');
         }
 

src/React/Promise/FulfilledPromise.php

@@ -15,8 +15,11 @@ public function then($fulfilledHandler = null, $errorHandler = null, $progressHa
     {
         try {
             $result = $this->result;
-            if ($fulfilledHandler) {
+
+            if (is_callable($fulfilledHandler)) {
                 $result = call_user_func($fulfilledHandler, $result);
+            } elseif (null !== $fulfilledHandler) {
+                trigger_error('Invalid $fulfilledHandler argument passed to then(), must be null or callable.', E_USER_NOTICE);
             }
 
             return Util::promiseFor($result);

src/React/Promise/RejectedPromise.php

@@ -14,7 +14,11 @@ public function __construct($reason = null)
     public function then($fulfilledHandler = null, $errorHandler = null, $progressHandler = null)
     {
         try {
-            if (!$errorHandler) {
+            if (!is_callable($errorHandler)) {
+                if (null !== $errorHandler) {
+                    trigger_error('Invalid $errorHandler argument passed to then(), must be null or callable.', E_USER_NOTICE);
+                }
+
                 return new RejectedPromise($this->reason);
             }
 

tests/React/Promise/DeferredProgressTest.php

@@ -312,4 +312,38 @@ public function shouldAllowRejectAfterProgress()
             ->resolver()
             ->reject(2);
     }
+
+    /**
+     * @test
+     * @dataProvider invalidCallbackDataProvider
+     **/
+    public function shouldIgnoreNonFunctionsAndTriggerPhpNotice($var)
+    {
+        $errorCollector = new ErrorCollector();
+        $errorCollector->register();
+
+        $mock = $this->createCallableMock();
+        $mock
+            ->expects($this->once())
+            ->method('__invoke')
+            ->with($this->identicalTo(1));
+
+        $d = new Deferred();
+        $d
+            ->then(
+                null,
+                null,
+                $var
+            )
+            ->then(
+                $this->expectCallableNever(),
+                $this->expectCallableNever(),
+                $mock
+            );
+
+        $d->progress(1);
+
+        $errorCollector->assertCollectedError('Invalid $progressHandler argument passed to then(), must be null or callable.', E_USER_NOTICE);
+        $errorCollector->unregister();
+    }
 }

tests/React/Promise/DeferredRejectTest.php

@@ -85,4 +85,36 @@ public function shouldForwardReasonWhenCallbackIsNull()
 
         $d->reject(1);
     }
+
+    /**
+     * @test
+     * @dataProvider invalidCallbackDataProvider
+     **/
+    public function shouldIgnoreNonFunctionsAndTriggerPhpNotice($var)
+    {
+        $errorCollector = new ErrorCollector();
+        $errorCollector->register();
+
+        $mock = $this->createCallableMock();
+        $mock
+            ->expects($this->once())
+            ->method('__invoke')
+            ->with($this->identicalTo(1));
+
+        $d = new Deferred();
+        $d
+            ->then(
+                null,
+                $var
+            )
+            ->then(
+                $this->expectCallableNever(),
+                $mock
+            );
+
+        $d->reject(1);
+
+        $errorCollector->assertCollectedError('Invalid $errorHandler argument passed to then(), must be null or callable.', E_USER_NOTICE);
+        $errorCollector->unregister();
+    }
 }

tests/React/Promise/DeferredResolveTest.php

@@ -162,4 +162,35 @@ public function shouldForwardValueWhenCallbackIsNull()
 
         $d->resolve(1);
     }
+
+    /**
+     * @test
+     * @dataProvider invalidCallbackDataProvider
+     **/
+    public function shouldIgnoreNonFunctionsAndTriggerPhpNotice($var)
+    {
+        $errorCollector = new ErrorCollector();
+        $errorCollector->register();
+
+        $mock = $this->createCallableMock();
+        $mock
+            ->expects($this->once())
+            ->method('__invoke')
+            ->with($this->identicalTo(1));
+
+        $d = new Deferred();
+        $d
+            ->then(
+                $var
+            )
+            ->then(
+                $mock,
+                $this->expectCallableNever()
+            );
+
+        $d->resolve(1);
+
+        $errorCollector->assertCollectedError('Invalid $fulfilledHandler argument passed to then(), must be null or callable.', E_USER_NOTICE);
+        $errorCollector->unregister();
+    }
 }

tests/React/Promise/ErrorCollector.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace React\Promise;
+
+class ErrorCollector
+{
+    private $errors = array();
+
+    public function register()
+    {
+        $errors = array();
+
+        set_error_handler(function ($errno, $errstr, $errfile, $errline, $errcontext) use (&$errors) {
+            $errors[] = compact('errno', 'errstr', 'errfile', 'errline', 'errcontext');
+        });
+
+        $this->errors = &$errors;
+    }
+
+    public function unregister()
+    {
+        $this->errors = array();
+        restore_error_handler();
+    }
+
+    public function assertCollectedError($errstr, $errno)
+    {
+        foreach ($this->errors as $error) {
+            if ($error['errstr'] === $errstr && $error['errno'] === $errno) {
+                return;
+            }
+        }
+
+        $message = 'Error with level ' . $errno . ' and message "' . $errstr . '" not found in ' . var_export($this->errors, true);
+
+        throw new \PHPUnit_Framework_AssertionFailedError($message);
+    }
+}

tests/React/Promise/TestCase.php

@@ -38,4 +38,16 @@ public function createCallableMock()
     {
         return $this->getMock('React\\Promise\Stub\CallableStub');
     }
+
+    public function invalidCallbackDataProvider()
+    {
+        return array(
+            'empty string' => array(''),
+            'true'         => array(true),
+            'false'        => array(false),
+            'object'       => array(new \stdClass),
+            'truthy'       => array(1),
+            'falsey'       => array(0)
+        );
+    }
 }