feature #23816 [Debug] Detect internal and deprecated methods (GuilhemN)

This PR was squashed before being merged into the 3.4 branch (closes #23816).

Discussion
----------

[Debug] Detect internal and deprecated methods

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes <!-- don't forget updating src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | no <!-- don't forget updating UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

I just realized we do not detect `@deprecated` methods like we do for `@final` methods, so here's a fix.

I reorganized the file to avoid code duplication... so the diff is kind of impressive but the behavior is the same.

Commits
-------

16eeabf [Debug] Detect internal and deprecated methods

src/Symfony/Component/Debug/DebugClassLoader.php

@@ -27,10 +27,12 @@ class DebugClassLoader
     private $classLoader;
     private $isFinder;
     private static $caseCheck;
-    private static $internal = array();
     private static $final = array();
     private static $finalMethods = array();
     private static $deprecated = array();
+    private static $deprecatedMethods = array();
+    private static $internal = array();
+    private static $internalMethods = array();
     private static $php7Reserved = array('int', 'float', 'bool', 'string', 'true', 'false', 'null');
     private static $darwinCache = array('/' => array('/', array()));
 
@@ -166,53 +168,6 @@ public function loadClass($class)
                 throw new \RuntimeException(sprintf('Case mismatch between loaded and declared class names: "%s" vs "%s".', $class, $name));
             }
 
-            $parent = get_parent_class($class);
-            $doc = $refl->getDocComment();
-            if (preg_match('#\n \* @internal(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) {
-                self::$internal[$name] = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
-            }
-
-            // Not an interface nor a trait
-            if (class_exists($name, false)) {
-                if (preg_match('#\n \* @final(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) {
-                    self::$final[$name] = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
-                }
-
-                if ($parent && isset(self::$final[$parent])) {
-                    @trigger_error(sprintf('The "%s" class is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $parent, self::$final[$parent], $name), E_USER_DEPRECATED);
-                }
-
-                // Inherit @final annotations
-                self::$finalMethods[$name] = $parent && isset(self::$finalMethods[$parent]) ? self::$finalMethods[$parent] : array();
-
-                foreach ($refl->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $method) {
-                    if ($method->class !== $name) {
-                        continue;
-                    }
-
-                    if ($parent && isset(self::$finalMethods[$parent][$method->name])) {
-                        @trigger_error(sprintf('%s It may change without further notice as of its next major version. You should not extend it from "%s".', self::$finalMethods[$parent][$method->name], $name), E_USER_DEPRECATED);
-                    }
-
-                    $doc = $method->getDocComment();
-                    if (false === $doc || false === strpos($doc, '@final')) {
-                        continue;
-                    }
-
-                    if (preg_match('#\n\s+\* @final(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
-                        $message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
-                        self::$finalMethods[$name][$method->name] = sprintf('The "%s::%s()" method is considered final%s.', $name, $method->name, $message);
-                    }
-                }
-            }
-
-            if (in_array(strtolower($refl->getShortName()), self::$php7Reserved)) {
-                @trigger_error(sprintf('The "%s" class uses the reserved name "%s", it will break on PHP 7 and higher', $name, $refl->getShortName()), E_USER_DEPRECATED);
-            }
-            if (preg_match('#\n \* @deprecated (.*?)\r?\n \*(?: @|/$)#s', $refl->getDocComment(), $notice)) {
-                self::$deprecated[$name] = preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]);
-            }
-
             // Don't trigger deprecations for classes in the same vendor
             if (2 > $len = 1 + (strpos($name, '\\', 1 + strpos($name, '\\')) ?: strpos($name, '_'))) {
                 $len = 0;
@@ -228,40 +183,87 @@ public function loadClass($class)
                 }
             }
 
-            foreach (array_merge(array($parent), class_implements($name, false), class_uses($name, false)) as $use) {
+            // Detect annotations on the class
+            if (false !== $doc = $refl->getDocComment()) {
+                foreach (array('final', 'deprecated', 'internal') as $annotation) {
+                    if (false !== strpos($doc, '@'.$annotation) && preg_match('#\n \* @'.$annotation.'(?:( .+?)\.?)?\r?\n \*(?: @|/$)#s', $doc, $notice)) {
+                        self::${$annotation}[$name] = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
+                    }
+                }
+            }
+
+            $parentAndTraits = class_uses($name, false);
+            if ($parent = get_parent_class($class)) {
+                $parentAndTraits[] = $parent;
+
+                if (isset(self::$final[$parent])) {
+                    @trigger_error(sprintf('The "%s" class is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $parent, self::$final[$parent], $name), E_USER_DEPRECATED);
+                }
+            }
+
+            // Detect if the parent is annotated
+            foreach ($parentAndTraits + $this->getOwnInterfaces($name, $parent) as $use) {
+                if (isset(self::$deprecated[$use]) && strncmp($ns, $use, $len)) {
+                    $type = class_exists($name, false) ? 'class' : (interface_exists($name, false) ? 'interface' : 'trait');
+                    $verb = class_exists($use, false) || interface_exists($name, false) ? 'extends' : (interface_exists($use, false) ? 'implements' : 'uses');
+
+                    @trigger_error(sprintf('The "%s" %s %s "%s" that is deprecated%s.', $name, $type, $verb, $use, self::$deprecated[$use]), E_USER_DEPRECATED);
+                }
                 if (isset(self::$internal[$use]) && strncmp($ns, $use, $len)) {
                     @trigger_error(sprintf('The "%s" %s is considered internal%s. It may change without further notice. You should not use it from "%s".', $use, class_exists($use, false) ? 'class' : (interface_exists($use, false) ? 'interface' : 'trait'), self::$internal[$use], $name), E_USER_DEPRECATED);
                 }
             }
 
-            if (!$parent || strncmp($ns, $parent, $len)) {
-                if ($parent && isset(self::$deprecated[$parent]) && strncmp($ns, $parent, $len)) {
-                    @trigger_error(sprintf('The "%s" class extends "%s" that is deprecated %s', $name, $parent, self::$deprecated[$parent]), E_USER_DEPRECATED);
+            // Inherit @final and @deprecated annotations for methods
+            self::$finalMethods[$name] = array();
+            self::$deprecatedMethods[$name] = array();
+            self::$internalMethods[$name] = array();
+            foreach ($parentAndTraits as $use) {
+                foreach (array('finalMethods', 'deprecatedMethods', 'internalMethods') as $property) {
+                    if (isset(self::${$property}[$use])) {
+                        self::${$property}[$name] = array_merge(self::${$property}[$name], self::${$property}[$use]);
+                    }
                 }
+            }
 
-                $parentInterfaces = array();
-                $deprecatedInterfaces = array();
-                if ($parent) {
-                    foreach (class_implements($parent) as $interface) {
-                        $parentInterfaces[$interface] = 1;
-                    }
+            $isClass = class_exists($name, false);
+            foreach ($refl->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $method) {
+                if ($method->class !== $name) {
+                    continue;
                 }
 
-                foreach ($refl->getInterfaceNames() as $interface) {
-                    if (isset(self::$deprecated[$interface]) && strncmp($ns, $interface, $len)) {
-                        $deprecatedInterfaces[] = $interface;
+                if ($isClass && $parent && isset(self::$finalMethods[$parent][$method->name])) {
+                    list($methodShortName, $message) = self::$finalMethods[$parent][$method->name];
+                    @trigger_error(sprintf('The "%s" method is considered final%s. It may change without further notice as of its next major version. You should not extend it from "%s".', $methodShortName, $message, $name), E_USER_DEPRECATED);
+                }
+
+                foreach ($parentAndTraits as $use) {
+                    if (isset(self::$deprecatedMethods[$use][$method->name]) && strncmp($ns, $use, $len)) {
+                        list($methodShortName, $message) = self::$deprecatedMethods[$use][$method->name];
+                        @trigger_error(sprintf('The "%s" method is deprecated%s. You should not extend it from "%s".', $methodShortName, $message, $name), E_USER_DEPRECATED);
                     }
-                    foreach (class_implements($interface) as $interface) {
-                        $parentInterfaces[$interface] = 1;
+                    if (isset(self::$internalMethods[$use][$method->name]) && strncmp($ns, $use, $len)) {
+                        list($methodShortName, $message) = self::$internalMethods[$use][$method->name];
+                        @trigger_error(sprintf('The "%s" method is considered internal%s. It may change without further notice. You should not use it from "%s".', $methodShortName, $message, $name), E_USER_DEPRECATED);
                     }
                 }
 
-                foreach ($deprecatedInterfaces as $interface) {
-                    if (!isset($parentInterfaces[$interface])) {
-                        @trigger_error(sprintf('The "%s" %s "%s" that is deprecated %s', $name, $refl->isInterface() ? 'interface extends' : 'class implements', $interface, self::$deprecated[$interface]), E_USER_DEPRECATED);
+                // Detect method annotations
+                if (false === $doc = $method->getDocComment()) {
+                    continue;
+                }
+
+                foreach (array('final', 'deprecated', 'internal') as $annotation) {
+                    if (false !== strpos($doc, '@'.$annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
+                        $message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
+                        self::${$annotation.'Methods'}[$name][$method->name] = array(sprintf('%s::%s()', $name, $method->name), $message);
                     }
                 }
             }
+
+            if (in_array(strtolower($refl->getShortName()), self::$php7Reserved)) {
+                @trigger_error(sprintf('The "%s" class uses the reserved name "%s", it will break on PHP 7 and higher', $name, $refl->getShortName()), E_USER_DEPRECATED);
+            }
         }
 
         if ($file) {
@@ -361,4 +363,31 @@ public function loadClass($class)
             return true;
         }
     }
+
+    /**
+     * `class_implements` includes interfaces from the parents so we have to manually exclude them.
+     *
+     * @param string       $class
+     * @param string|false $parent
+     *
+     * @return string[]
+     */
+    private function getOwnInterfaces($class, $parent)
+    {
+        $ownInterfaces = class_implements($class, false);
+
+        if ($parent) {
+            foreach (class_implements($parent, false) as $interface) {
+                unset($ownInterfaces[$interface]);
+            }
+        }
+
+        foreach ($ownInterfaces as $interface) {
+            foreach (class_implements($interface) as $interface) {
+                unset($ownInterfaces[$interface]);
+            }
+        }
+
+        return $ownInterfaces;
+    }
 }

src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php

@@ -313,6 +313,28 @@ class_exists(__NAMESPACE__.'\\Fixtures\\ExtendedFinalMethod', true);
         $this->assertSame($xError, $lastError);
     }
 
+    public function testExtendedDeprecatedMethod()
+    {
+        set_error_handler(function () { return false; });
+        $e = error_reporting(0);
+        trigger_error('', E_USER_NOTICE);
+
+        class_exists('Test\\'.__NAMESPACE__.'\\ExtendsAnnotatedClass', true);
+
+        error_reporting($e);
+        restore_error_handler();
+
+        $lastError = error_get_last();
+        unset($lastError['file'], $lastError['line']);
+
+        $xError = array(
+            'type' => E_USER_DEPRECATED,
+            'message' => 'The "Symfony\Component\Debug\Tests\Fixtures\AnnotatedClass::deprecatedMethod()" method is deprecated since version 3.4. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsAnnotatedClass".',
+        );
+
+        $this->assertSame($xError, $lastError);
+    }
+
     public function testInternalsUse()
     {
         $deprecations = array();
@@ -325,9 +347,10 @@ class_exists('Test\\'.__NAMESPACE__.'\\ExtendsInternals', true);
         restore_error_handler();
 
         $this->assertSame($deprecations, array(
+            'The "Symfony\Component\Debug\Tests\Fixtures\InternalTrait" trait is considered internal. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".',
             'The "Symfony\Component\Debug\Tests\Fixtures\InternalClass" class is considered internal since version 3.4. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".',
             'The "Symfony\Component\Debug\Tests\Fixtures\InternalInterface" interface is considered internal. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".',
-            'The "Symfony\Component\Debug\Tests\Fixtures\InternalTrait" trait is considered internal. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".',
+            'The "Symfony\Component\Debug\Tests\Fixtures\InternalClass::internalMethod()" method is considered internal since version 3.4. It may change without further notice. You should not use it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".',
         ));
     }
 }
@@ -371,9 +394,15 @@ public function findFile($class)
             eval('namespace Test\\'.__NAMESPACE__.'; class Float {}');
         } elseif ('Test\\'.__NAMESPACE__.'\ExtendsFinalClass' === $class) {
             eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsFinalClass extends \\'.__NAMESPACE__.'\Fixtures\FinalClass {}');
+        } elseif ('Test\\'.__NAMESPACE__.'\ExtendsAnnotatedClass' === $class) {
+            eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsAnnotatedClass extends \\'.__NAMESPACE__.'\Fixtures\AnnotatedClass {
+                public function deprecatedMethod() { }
+            }');
         } elseif ('Test\\'.__NAMESPACE__.'\ExtendsInternals' === $class) {
             eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternals extends \\'.__NAMESPACE__.'\Fixtures\InternalClass implements \\'.__NAMESPACE__.'\Fixtures\InternalInterface {
                 use \\'.__NAMESPACE__.'\Fixtures\InternalTrait;
+
+                public function internalMethod() { }
             }');
         }
     }

src/Symfony/Component/Debug/Tests/Fixtures/AnnotatedClass.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Symfony\Component\Debug\Tests\Fixtures;
+
+class AnnotatedClass
+{
+    /**
+     * @deprecated since version 3.4.
+     */
+    public function deprecatedMethod()
+    {
+    }
+}

src/Symfony/Component/Debug/Tests/Fixtures/InternalClass.php

@@ -8,4 +8,8 @@
 class InternalClass
 {
     use InternalTrait2;
+
+    public function usedInInternalClass()
+    {
+    }
 }

src/Symfony/Component/Debug/Tests/Fixtures/InternalTrait2.php

@@ -7,4 +7,17 @@
  */
 trait InternalTrait2
 {
+    /**
+     * @internal since version 3.4
+     */
+    public function internalMethod()
+    {
+    }
+
+    /**
+     * @internal but should not trigger a deprecation.
+     */
+    public function usedInInternalClass()
+    {
+    }
 }