merged branch colinfrei/deprecatedErrorHandling (PR #6173)

This PR was squashed before being merged into the master branch (closes #6173). Commits ------- 4878ec0 [HttpKernel] [WebProfilerBundle] Better handling of deprecated methods Discussion ---------- [HttpKernel] [WebProfilerBundle] Better handling of deprecated methods Bug fix: no Feature addition: yes Backwards compatibility break: yes, if you were expecting E_USER_DEPRECATED or E_DEPRECATED to throw an exception Symfony2 tests pass: yes Fixes the following tickets: #6139 partly, I'd go through and add the actual trigger_error() calls in another (or possibly one per component) PR Todo: call trigger_error() License of the code: MIT Documentation PR: - I added the deprecation count with the Exception icon in the Profiler Toolbar, and changed the color of it to be yellow for deprecations and red for exceptions (was yellow for exceptions). --------------------------------------------------------------------------- by fabpot at 2012-12-03T09:43:09Z Adding trigger_error calls should be done in one PR to ease the merging. thanks.
2012-12-06 09:10:08 +01:00 · 2012-12-06 09:10:08 +01:00 · e75c7e20ce
commit e75c7e20ce
parent 869be1df6d 4878ec08e6
7 changed files with 149 additions and 13 deletions
--- a/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug.xml
+++ b/src/Symfony/Bundle/FrameworkBundle/Resources/config/debug.xml
@ -9,6 +9,7 @@
        <parameter key="debug.stopwatch.class">Symfony\Component\Stopwatch\Stopwatch</parameter>
        <parameter key="debug.container.dump">%kernel.cache_dir%/%kernel.container_class%.xml</parameter>
        <parameter key="debug.controller_resolver.class">Symfony\Component\HttpKernel\Controller\TraceableControllerResolver</parameter>
+        <parameter key="debug.deprecation_logger_listener.class">Symfony\Component\HttpKernel\EventListener\DeprecationLoggerListener</parameter>
    </parameters>

    <services>
@ -26,5 +27,11 @@
            <argument type="service" id="controller_resolver" />
            <argument type="service" id="debug.stopwatch" />
        </service>
+
+        <service id="debug.deprecation_logger_listener" class="%debug.deprecation_logger_listener.class%">
+            <tag name="kernel.event_subscriber" />
+            <tag name="monolog.logger" channel="deprecation" />
+            <argument type="service" id="logger" on-invalid="null" />
+        </service>
    </services>
 </container>
--- a/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/logger.html.twig
+++ b/src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/logger.html.twig
@ -1,16 +1,30 @@
 {% extends '@WebProfiler/Profiler/layout.html.twig' %}

 {% block toolbar %}
-    {% if collector.counterrors %}
+    {% if collector.counterrors or collector.countdeprecations %}
        {% set icon %}
            <img width="15" height="28" alt="Logs" src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAcCAYAAABoMT8aAAAA4klEQVQ4y2P4//8/AyWYYXgYwOPp6Xnc3t7+P7EYpB6k7+zZs2ADNEjRjIwDAgKWgAywIUfz8+fPVzg7O/8AGeCATQEQnAfi/SAah/wcV1dXvAYUgORANA75ehcXl+/4DHAABRIe+ZrhbgAhTHsDiEgHBA0glA6GfSDiw5mZma+A+sphBlhVVFQ88vHx+Xfu3Ll7QP5haOjjwtuAuGHv3r3NIMNABqh8+/atsaur666vr+9XUlwSHx//AGQANxCbAnEWyGQicRMQ9wBxIQM0qjiBWAFqkB00/glhayBWHwb1AgB38EJsUtxtWwAAAABJRU5ErkJggg=="/>
-            <span class="sf-toolbar-status sf-toolbar-status-yellow">{{ collector.counterrors }}</span>
+            {% if collector.counterrors %}
+                {% set statusColor = "red" %}
+            {% else %}
+                {% set statusColor = "yellow" %}
+            {% endif %}
+            {% set errorCount = collector.counterrors + collector.countdeprecations %}
+            <span class="sf-toolbar-status sf-toolbar-status-{{ statusColor }}">{{ errorCount }}</span>
        {% endset %}
        {% set text %}
-            <div class="sf-toolbar-info-piece">
-                <b>Exception</b>
-                <span class="sf-toolbar-status sf-toolbar-status-yellow">{{ collector.counterrors }}</span>
-            </div>
+            {% if collector.counterrors %}
+                <div class="sf-toolbar-info-piece">
+                    <b>Exception</b>
+                    <span class="sf-toolbar-status sf-toolbar-status-red">{{ collector.counterrors }}</span>
+                </div>
+            {% endif %}
+            {% if collector.countdeprecations %}
+                <div class="sf-toolbar-info-piece">
+                    <b>Deprecated Calls</b>
+                    <span class="sf-toolbar-status sf-toolbar-status-yellow">{{ collector.countdeprecations }}</span>
+                </div>
+            {% endif %}
        {% endset %}
        {% include '@WebProfiler/Profiler/toolbar_item.html.twig' with { 'link': profiler_url } %}
    {% endif %}
@ -20,9 +34,10 @@
 <span class="label">
    <span class="icon"><img src="data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABoAAAAgCAYAAAAMq2gFAAABjElEQVRIx2MIDw+vd3R0/GFvb/+fGtjFxeVJSUmJ1f///5nv37/PAMMMzs7OVLMEhoODgy/k5+cHJCYmagAtZAJbRG1L0DEwxCYALeOgiUXbt2+/X1NT8xTEdnd3/wi0SI4mFgHBDCBeCLXoF5BtwkCEpvNAvB8JnydCTwgQR0It+g1kWxNjUQEQOyDhAiL0gNUiWWRDjEUOyMkUZsCoRaMWjVpEvEVkFkGjFmEUqgc+fvx4hVYWIReqzi9evKileaoDslnu3LkTNLQtGk3edLPIycnpL9Bge5pb1NXVdQNosDmGRcAm7F+QgKur6783b95cBQoeRGv1kII3QPOdAoZF8+fPP4PUqnx55syZVKCEI1rLh1hsAbWEZ8aMGaUoFoFcMG3atKdIjfSPISEhawICAlaQgwMDA1f6+/sfB5rzE2Sej4/PD3C7DkjoAHHVoUOHLpSVlX3w8vL6Sa34Alr6Z8WKFaCoMARZxAHEoFZ/HBD3A/FyIF4BxMvIxCC964F4G6hZDMTxQCwJAGWE8pur5kFDAAAAAElFTkSuQmCC" alt="Logger" /></span>
    <strong>Logs</strong>
-    {% if collector.counterrors %}
+    {% if collector.counterrors or collector.countdeprecations %}
+        {% set errorCount = collector.counterrors + collector.countdeprecations %}
        <span class="count">
-            <span>{{ collector.counterrors }}</span>
+            <span>{{ errorCount }}</span>
        </span>
    {% endif %}
 </span>
--- a/src/Symfony/Component/HttpKernel/DataCollector/LoggerDataCollector.php
+++ b/src/Symfony/Component/HttpKernel/DataCollector/LoggerDataCollector.php
@ -38,8 +38,9 @@ class LoggerDataCollector extends DataCollector
    {
        if (null !== $this->logger) {
            $this->data = array(
-                'error_count' => $this->logger->countErrors(),
-                'logs'        => $this->sanitizeLogs($this->logger->getLogs()),
+                'error_count'       => $this->logger->countErrors(),
+                'logs'              => $this->sanitizeLogs($this->logger->getLogs()),
+                'deprecation_count' => $this->computeDeprecationCount()
            );
        }
    }
@ -66,6 +67,11 @@ class LoggerDataCollector extends DataCollector
        return isset($this->data['logs']) ? $this->data['logs'] : array();
    }

+    public function countDeprecations()
+    {
+        return isset($this->data['deprecation_count']) ? $this->data['deprecation_count'] : 0;
+    }
+
    /**
     * {@inheritdoc}
     */
@ -103,4 +109,16 @@ class LoggerDataCollector extends DataCollector

        return $context;
    }
+
+    private function computeDeprecationCount()
+    {
+        $count = 0;
+        foreach ($this->logger->getLogs() as $log) {
+            if (isset($log['context']['type']) && 'deprecation' === $log['context']['type']) {
+                $count++;
+            }
+        }
+
+        return $count;
+    }
 }
--- a/src/Symfony/Component/HttpKernel/Debug/ErrorHandler.php
+++ b/src/Symfony/Component/HttpKernel/Debug/ErrorHandler.php
@ -12,6 +12,7 @@
 namespace Symfony\Component\HttpKernel\Debug;

 use Symfony\Component\HttpKernel\Exception\FatalErrorException;
+use Symfony\Component\HttpKernel\Log\LoggerInterface;

 /**
 * ErrorHandler.
@ -40,6 +41,9 @@ class ErrorHandler

    private $reservedMemory;

+    /** @var LoggerInterface */
+    private static $logger;
+
    /**
     * Register the error handler.
     *
@ -64,6 +68,11 @@ class ErrorHandler
        $this->level = null === $level ? error_reporting() : $level;
    }

+    public static function setLogger(LoggerInterface $logger)
+    {
+        self::$logger = $logger;
+    }
+
    /**
     * @throws \ErrorException When error_reporting returns error
     */
@ -73,6 +82,14 @@ class ErrorHandler
            return false;
        }

+        if ($level & E_USER_DEPRECATED || $level & E_DEPRECATED) {
+            if (null !== self::$logger) {
+                self::$logger->warn($message, array('type' => 'deprecation', 'file' => $file, 'line' => $line));
+            }
+
+            return true;
+        }
+
        if (error_reporting() & $level && $this->level & $level) {
            throw new \ErrorException(sprintf('%s: %s in %s line %d', isset($this->levels[$level]) ? $this->levels[$level] : $level, $message, $file, $line), 0, $level, $file, $line);
        }
--- a/src/Symfony/Component/HttpKernel/EventListener/DeprecationLoggerListener.php
+++ b/src/Symfony/Component/HttpKernel/EventListener/DeprecationLoggerListener.php
@ -0,0 +1,44 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Component\HttpKernel\EventListener;
+
+use Symfony\Component\HttpKernel\Log\LoggerInterface;
+use Symfony\Component\HttpKernel\Debug\ErrorHandler;
+use Symfony\Component\EventDispatcher\EventSubscriberInterface;
+use Symfony\Component\HttpKernel\KernelEvents;
+
+/**
+ * Injects the logger into the ErrorHandler, so that it can log deprecation errors.
+ *
+ * @author Colin Frei <colin@colinfrei.com>
+ */
+class DeprecationLoggerListener implements EventSubscriberInterface
+{
+    private $logger;
+
+    public function __construct(LoggerInterface $logger = null)
+    {
+        $this->logger = $logger;
+    }
+
+    public function injectLogger()
+    {
+        if (null !== $this->logger) {
+            ErrorHandler::setLogger($this->logger);
+        }
+    }
+
+    public static function getSubscribedEvents()
+    {
+        return array(KernelEvents::REQUEST => 'injectLogger');
+    }
+}
--- a/src/Symfony/Component/HttpKernel/Tests/DataCollector/LoggerDataCollectorTest.php
+++ b/src/Symfony/Component/HttpKernel/Tests/DataCollector/LoggerDataCollectorTest.php
@ -27,18 +27,19 @@ class LoggerDataCollectorTest extends \PHPUnit_Framework_TestCase
    /**
     * @dataProvider getCollectTestData
     */
-    public function testCollect($nb, $logs, $expected)
+    public function testCollect($nb, $logs, $expectedLogs, $expectedDeprecationCount)
    {
        $logger = $this->getMock('Symfony\Component\HttpKernel\Log\DebugLoggerInterface');
        $logger->expects($this->once())->method('countErrors')->will($this->returnValue($nb));
-        $logger->expects($this->once())->method('getLogs')->will($this->returnValue($logs));
+        $logger->expects($this->exactly(2))->method('getLogs')->will($this->returnValue($logs));

        $c = new LoggerDataCollector($logger);
        $c->collect(new Request(), new Response());

        $this->assertSame('logger', $c->getName());
        $this->assertSame($nb, $c->countErrors());
-        $this->assertSame($expected ? $expected : $logs, $c->getLogs());
+        $this->assertSame($expectedLogs ? $expectedLogs : $logs, $c->getLogs());
+        $this->assertSame($expectedDeprecationCount, $c->countDeprecations());
    }

    public function getCollectTestData()
@ -48,16 +49,28 @@ class LoggerDataCollectorTest extends \PHPUnit_Framework_TestCase
                1,
                array(array('message' => 'foo', 'context' => array())),
                null,
+                0
            ),
            array(
                1,
                array(array('message' => 'foo', 'context' => array('foo' => fopen(__FILE__, 'r')))),
                array(array('message' => 'foo', 'context' => array('foo' => 'Resource(stream)'))),
+                0
            ),
            array(
                1,
                array(array('message' => 'foo', 'context' => array('foo' => new \stdClass()))),
                array(array('message' => 'foo', 'context' => array('foo' => 'Object(stdClass)'))),
+                0
+            ),
+            array(
+                1,
+                array(
+                    array('message' => 'foo', 'context' => array('type' => 'deprecation')),
+                    array('message' => 'foo2', 'context' => array('type' => 'deprecation'))
+                ),
+                null,
+                2
            ),
        );
    }
--- a/src/Symfony/Component/HttpKernel/Tests/Debug/ErrorHandlerTest.php
+++ b/src/Symfony/Component/HttpKernel/Tests/Debug/ErrorHandlerTest.php
@ -56,5 +56,27 @@ class ErrorHandlerTest extends \PHPUnit_Framework_TestCase
        }

        restore_error_handler();
+
+        $handler = ErrorHandler::register(E_USER_DEPRECATED);
+        $this->assertTrue($handler->handle(E_USER_DEPRECATED, 'foo', 'foo.php', 12, 'foo'));
+
+        restore_error_handler();
+
+        $handler = ErrorHandler::register(E_DEPRECATED);
+        $this->assertTrue($handler->handle(E_DEPRECATED, 'foo', 'foo.php', 12, 'foo'));
+
+        restore_error_handler();
+
+        $logger = $this->getMock('Symfony\Component\HttpKernel\Log\LoggerInterface');
+        $logger->expects($this->once())->method('warn')->with(
+            $this->equalTo('foo'),
+            $this->equalTo(array('type' => 'deprecation', 'file' => 'foo.php', 'line' => '12'))
+        );
+
+        $handler = ErrorHandler::register(E_USER_DEPRECATED);
+        $handler->setLogger($logger);
+        $handler->handle(E_USER_DEPRECATED, 'foo', 'foo.php', 12, 'foo');
+
+        restore_error_handler();
    }
 }