8000 bug #23844 [Debug] Correctly detect methods not from the same vendor … · symfony/symfony@03fce7e · GitHub
[go: up one dir, main page]
Skip to content

Commit 03fce7e

Browse files
nicolas-grekasjderusse
nicolas-grekas
authored and
jderusse
committed
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
2 parents d90742e + 08d352a commit 03fce7e

File tree

4 files changed

+38
-19
lines changed

4 files changed

+38
-19
lines changed

‎.travis.yml

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -92,12 +92,9 @@ before_install:
9292
fi
9393
9494
# Matrix lines for intermediate PHP versions are skipped for pull requests
95-
if [[ ! $deps && ! $PHP = ${MIN_PHP%.*} && ! $PHP = hhvm* && $TRAVIS_PULL_REQUEST != false ]]; then
96-
deps=skip
97-
skip=1
98-
else
99-
COMPONENTS=$(find src/Symfony -mindepth 3 -type f -name phpunit.xml.dist -printf '%h\n')
100-
fi
95+
# COMPONENTS=$(printf 'src/Symfony/Bridge/Doctrine\nsrc/Symfony/Bundle/FrameworkBundle\nsrc/Symfony/Component/Cache\nsrc/Symfony/Component/HttpFoundation\nsrc/Symfony/Component/Lock\n')
96+
# COMPONENTS=$(find src/Symfony -mindepth 3 -type f -name phpunit.xml.dist -printf '%h\n'|grep Lock)
97+
COMPONENTS=$(find src/Symfony -mindepth 3 -type f -name phpunit.xml.dist -printf '%h\n')
10198
10299
- |
103100
# Install sigchild-enabled PHP to test the Process component on the lowest PHP matrix line

‎src/Symfony/Component/Debug/DebugClassLoader.php

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -232,19 +232,28 @@ public function loadClass($class)
232232
continue;
233233
}
234234

235+
// Method from a trait
236+
if ($method->getFilename() !== $refl->getFileName()) {
237+
continue;
238+
}
239+
235240
if ($isClass && $parent && isset(self::$finalMethods[$parent][$method->name])) {
236-
list($methodShortName, $message) = self::$finalMethods[$parent][$method->name];
237-
@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);
241+
list($declaringClass, $message) = self::$finalMethods[$parent][$method->name];
242+
@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);
238243
}
239244

240245
foreach ($parentAndTraits as $use) {
241-
if (isset(self::$deprecatedMethods[$use][$method->name]) && strncmp($ns, $use, $len)) {
242-
list($methodShortName, $message) = self::$deprecatedMethods[$use][$method->name];
243-
@trigger_error(sprintf('The "%s" method is deprecated%s. You should not extend it from "%s".', $methodShortName, $message, $name), E_USER_DEPRECATED);
246+
if (isset(self::$deprecatedMethods[$use][$method->name])) {
247+
list($declaringClass, $message) = self::$deprecatedMethods[$use][$method->name];
248+
if (strncmp($ns, $declaringClass, $len)) {
249+
@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);
250+
}
244251
}
245-
if (isset(self::$internalMethods[$use][$method->name]) && strncmp($ns, $use, $len)) {
246-
list($methodShortName, $message) = self::$internalMethods[$use][$method->name];
247-
@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);
252+
if (isset(self::$internalMethods[$use][$method->name])) {
253+
list($declaringClass, $message) = self::$internalMethods[$use][$method->name];
254+
if (strncmp($ns, $declaringClass, $len)) {
255+
@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+
}
248257
}
249258
}
250259

@@ -256,7 +265,7 @@ public function loadClass($class)
256265
foreach (array('final', 'deprecated', 'internal') as $annotation) {
257266
if (false !== strpos($doc, '@'.$annotation) && preg_match('#\n\s+\* @'.$annotation.'(?:( .+?)\.?)?\r?\n\s+\*(?: @|/$)#s', $doc, $notice)) {
258267
$message = isset($notice[1]) ? preg_replace('#\s*\r?\n \* +#', ' ', $notice[1]) : '';
259-
self::${$annotation.'Methods'}[$name][$method->name] = array(sprintf('%s::%s()', $name, $method->name), $message);
268+
self::${$annotation.'Methods'}[$name][$method->name] = array($name, $message);
260269
}
261270
}
262271
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -347,10 +347,10 @@ class_exists('Test\\'.__NAMESPACE__.'\\ExtendsInternals', true);
347347
restore_error_handler();
348348

349349
$this->assertSame($deprecations, array(
350+
'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".',
351+
'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".',
350352
'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".',
351-
'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".',
352-
'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".',
353-
'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".',
353+
'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".',
354354
));
355355
}
356356
}
@@ -399,11 +399,13 @@ public function findFile($class)
399399
public function deprecatedMethod() { }
400400
}');
401401
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsInternals' === $class) {
402-
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternals extends \\'.__NAMESPACE__.'\Fixtures\InternalClass implements \\'.__NAMESPACE__.'\Fixtures\InternalInterface {
402+
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternals extends ExtendsInternalsParent {
403403
use \\'.__NAMESPACE__.'\Fixtures\InternalTrait;
404404
405405
public function internalMethod() { }
406406
}');
407+
} elseif ('Test\\'.__NAMESPACE__.'\ExtendsInternalsParent' === $class) {
408+
eval('namespace Test\\'.__NAMESPACE__.'; class ExtendsInternalsParent extends \\'.__NAMESPACE__.'\Fixtures\InternalClass implements \\'.__NAMESPACE__.'\Fixtures\InternalInterface { }');
407409
}
408410
}
409411
}

‎src/Symfony/Component/Lock/Tests/Store/AbstractStoreTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,9 @@ public function testSave()
3939
$this->assertFalse($store->exists($key));
4040
}
4141

42+
/**
43+
* @dataProvider lotOfData
44+
*/
4245
public function testSaveWithDifferentResources()
4346
{
4447
$store = $this->getStore();
@@ -60,6 +63,9 @@ public function testSaveWithDifferentResources()
6063
$this->assertFalse($store->exists($key2));
6164
}
6265

66+
/**
67+
* @dataProvider lotOfData
68+
*/
6369
public function testSaveWithDifferentKeysOnSameResources()
6470
{
6571
$store = $this->getStore();
@@ -109,4 +115,9 @@ public function testSaveTwice()
109115

110116
$store->delete($key);
111117
}
118+
119+
public function lotOfData()
120+
{
121+
return array_fill(0, 1000, []);
122+
}
112123
}

0 commit comments

Comments
 (0)
0