8000 [Debug] Detect internal and deprecated methods by GuilhemN · Pull Request #23816 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Debug] Detect internal and deprecated methods #23816

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

[Debug] Detect internal and deprecated methods #23816

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
47c8632
[Debug] Detect internal and deprecated methods
GuilhemN Aug 7, 2017
87c39fb
Fixes
GuilhemN Aug 8, 2017
a69c60d
fixes
GuilhemN Aug 8, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[Debug] Detect internal and deprecated methods
  • Loading branch information
@GuilhemN
GuilhemN committed Aug 7, 2017
commit 47c86328ee455ded560eb37ff0e9bcf1f10d741a
161 changes: 95 additions & 66 deletions src/Symfony/Component/Debug/DebugClassLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));

Expand Down Expand Up @@ -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;
Expand All @@ -228,40 +183,88 @@ 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]) : '';
}
}
}

if ($parent = get_parent_class($class)) {
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);
}
}

$traits = class_uses($name, false);
Copy link
Member
@nicolas-grekas nicolas-grekas Aug 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$traits = array($parent) + class_uses($name, false);
Then below foreach ($interfaces + $traits to remove the array_merge() (directly $traits after, maybe renamed btw)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

array($parent) + $interfaces; could cause conflicts so I did $parentAndTraits = ($parent ? array($parent => $parent) : array()) + class_uses($name, false); instead.

$interfaces = $this->getOwnInterfaces($name);

// Detect if the parent is annotated
foreach (array_merge(array($parent), $interfaces, $traits) 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 (array_merge(array($parent), $traits) 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]);
}
}
}

$isClass = class_exists($name, false);
foreach ($refl->getMethods(\ReflectionMethod::IS_PUBLIC | \ReflectionMethod::IS_PROTECTED) as $method) {
if ($method->class !== $name) {
continue;
}

$parentInterfaces = array();
$deprecatedInterfaces = array();
if ($parent) {
foreach (class_implements($parent) as $interface) {
$parentInterfaces[$interface] = 1;
}
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 ($refl->getInterfaceNames() as $interface) {
if (isset(self::$deprecated[$interface]) && strncmp($ns, $interface, $len)) {
$deprecatedInterfaces[] = $interface;
foreach (array_merge(array($parent), $traits) 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
$doc = $method->getDocComment();
Copy link
Member
@nicolas-grekas nicolas-grekas Aug 8, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be merged with next line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if (false === $doc) {
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) {
Expand Down Expand Up @@ -361,4 +364,30 @@ public function loadClass($class)
return true;
}
}

/**
* `class_implements` includes interfaces from the parents so we have to
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can configure your IDE to auto-break lines at 120, not 80 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

* manually exclude them.
*
* @param string $class
*
* @return string[]
*/
private function getOwnInterfaces($class)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

        $ownInterfaces = class_implements($class, false);

        if ($parent = get_parent_class($class)) {
            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;

{
$parentInterfaces = array();
if ($parent = get_parent_class($class)) {
foreach (class_implements($parent, false) as $interface) {
$parentInterfaces[$interface] = true;
}
}

foreach (class_implements($class, false) as $interface) {
foreach (class_implements($interface) as $interface) {
$parentInterfaces[$interface] = true;
}
}

return array_keys(array_diff_key(class_implements($class, false), $parentInterfaces));
}
}
29 changes: 29 additions & 0 deletions src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -328,6 +350,7 @@ class_exists('Test\\'.__NAMESPACE__.'\\ExtendsInternals', true);
'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".',
));
}
}
Expand Down Expand Up @@ -371,9 +394,15 @@ public function findFile($class)
eval('namespace Test\\'.__NAMESPACE__.'; class Float {}');
} elseif ('Test\\'.__NAMESPACE__ 10000 .'\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() { }
}');
}
}
Expand Down
13 changes: 13 additions & 0 deletions src/Symfony/Component/Debug/Tests/Fixtures/AnnotatedClass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

namespace Symfony\Component\Debug\Tests\Fixtures;

class AnnotatedClass
{
/**
* @deprecated since version 3.4.
*/
public function deprecatedMethod()
{
}
}
4 changes: 4 additions & 0 deletions src/Symfony/Component/Debug/Tests/Fixtures/InternalClass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@
class InternalClass
{
use InternalTrait2;

public function usedInInternalClass()
{
}
}
13 changes: 13 additions & 0 deletions src/Symfony/Component/Debug/Tests/Fixtures/InternalTrait2.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,17 @@
*/
trait InternalTrait2
{
/**
* @internal since version 3.4
*/
public function internalMethod()
{
}

/**
* @internal but should not trigger a deprecation.
*/
public function usedInInternalClass()
{
}
}
0