Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
bug #23844 [Debug] Correctly detect methods not from the same vendor …
…(GuilhemN)

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 #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 #23816 (comment), 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
  • Loading branch information
nicolas-grekas committed Aug 9, 2017
2 parents d90742e + 08d352a commit 01d1563
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 13 deletions.
27 changes: 18 additions & 9 deletions src/Symfony/Component/Debug/DebugClassLoader.php
Expand Up @@ -232,19 +232,28 @@ public function loadClass($class)
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);
}
}
}

Expand All @@ -256,7 +265,7 @@ public function loadClass($class)
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);
}
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
Expand Up @@ -347,10 +347,10 @@ class_exists('Test\\'.__NAMESPACE__.'\\ExtendsInternals', true);
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".',
));
}
}
Expand Down Expand Up @@ -399,11 +399,13 @@ public function findFile($class)
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 { }');
}
}
}

0 comments on commit 01d1563

Please sign in to comment.