diogo/symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

bug #23844 [Debug] Correctly detect methods not from the same vendor (GuilhemN)

Browse Source 
This PR was merged into the 3.4 branch.

Discussion
----------

[Debug] Correctly detect methods not from the same vendor

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no <!-- 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 there were two issues in https://github.com/symfony/symfony/pull/23816:

* when a class extended another class from the same vendor itself extending a class from another vendor containing deprecated methods, the deprecation wasn't triggered in case the first class extended a deprecated method from the last class because the parent from the same vendor was used for "the same vendor check".

* As stated in https://github.com/symfony/symfony/pull/23816#issuecomment-320661894, ReflectionMethod::$class doesn't contain the right value when using a trait so if you extended a trait containing methods from another vendor you would have always get deprecations. I fixed this by comparing the method file to the filename of the class using the trait.

@nicolas-grekas are you aware of any issue of `ReflectionMethod::getFilename()`?

Commits
-------

08d352a [Debug] Correctly detect methods not from the same vendor
This commit is contained in:
Nicolas Grekas 2017-08-09 19:12:38 +02:00
commit 01d1563df4
2 changed files with 24 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/Symfony/Component/Debug/DebugClassLoader.php
View File

@ -232,19 +232,28 @@ class DebugClassLoader
continue;
}
// Method from a trait
if ($method->getFilename() !== $refl->getFileName()) {
continue;
}
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);
list($declaringClass, $message) = self::$finalMethods[$parent][$method->name];
@trigger_error(sprintf('The "%s::%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".', $declaringClass, $method->name, $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);
if (isset(self::$deprecatedMethods[$use][$method->name])) {
list($declaringClass, $message) = self::$deprecatedMethods[$use][$method->name];
if (strncmp($ns, $declaringClass, $len)) {
@trigger_error(sprintf('The "%s::%s()" method is deprecated%s. You should not extend it from "%s".', $declaringClass, $method->name, $message, $name), E_USER_DEPRECATED);
}
}
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);
if (isset(self::$internalMethods[$use][$method->name])) {
list($declaringClass, $message) = self::$internalMethods[$use][$method->name];
if (strncmp($ns, $declaringClass, $len)) {
@trigger_error(sprintf('The "%s::%s()" method is considered internal%s. It may change without further notice. You should not extend it from "%s".', $declaringClass, $method->name, $message, $name), E_USER_DEPRECATED);
}
}
}
@ -256,7 +265,7 @@ class DebugClassLoader
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);
self::${$annotation.'Methods'}[$name][$method->name] = array($name, $message);
}
}
}

10
src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
View File

@ -347,10 +347,10 @@ class DebugClassLoaderTest extends TestCase
restore_error_handler();
$this->assertSame($deprecations, array(
'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\ExtendsInternalsParent".',
'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\ExtendsInternalsParent".',
'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\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".',
'The "Symfony\Component\Debug\Tests\Fixtures\InternalTrait2::internalMethod()" method is considered internal since version 3.4. It may change without further notice. You should not extend it from "Test\Symfony\Component\Debug\Tests\ExtendsInternals".',
));
}
}
@ -399,11 +399,13 @@ class ClassLoader
public function deprecatedMethod() { }
}');
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsInternals' === $class) {
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternals extends \\'.__NAMESPACE__.'\Fixtures\InternalClass implements \\'.__NAMESPACE__.'\Fixtures\InternalInterface {
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternals extends ExtendsInternalsParent {
use \\'.__NAMESPACE__.'\Fixtures\InternalTrait;
public function internalMethod() { }
}');
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsInternalsParent' === $class) {
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternalsParent extends \\'.__NAMESPACE__.'\Fixtures\InternalClass implements \\'.__NAMESPACE__.'\Fixtures\InternalInterface { }');
}
}
}