Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Debug] Fixed erroneous deprecation notice for extended Interfaces#17320
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Tobion commentedJan 10, 2016
I think this implementation is too specific. It should be solved generically. Something like ignore deprecated interfaces if the parent interface is from the Symfony namespace as well. |
peterrehm commentedJan 10, 2016
@Tobion Updated accordingly. I don't know if this might cause any unwanted side effects. |
Tobion commentedJan 10, 2016
Lets see what@nicolas-grekas thinks |
nicolas-grekas commentedJan 11, 2016
There is an issue, and I think we should not restrict the fix to --- a/src/Symfony/Component/Debug/DebugClassLoader.php+++ b/src/Symfony/Component/Debug/DebugClassLoader.php@@ -200,8 +200,23 @@ class DebugClassLoader @trigger_error(sprintf('The %s class extends %s that is deprecated %s', $name, $parent->name, self::$deprecated[$parent->name]), E_USER_DEPRECATED); }+ $parentInterfaces = array();+ $deprecatedInterfaces = array();+ if ($parent) {+ foreach ($parent->getInterfaceNames() as $interface) {+ $parentInterfaces[$interface] = 1;+ }+ } foreach ($refl->getInterfaceNames() as $interface) {- if (isset(self::$deprecated[$interface]) && strncmp($ns, $interface, $len) && !($parent && $parent->implementsInterface($interface))) {+ if (isset(self::$deprecated[$interface]) && strncmp($ns, $interface, $len)) {+ $deprecatedInterfaces[] = $interface;+ }+ foreach (class_implements($interface) as $interface) {+ $parentInterfaces[$interface] = 1;+ }+ }+ 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); } } @peterrehm could you please also squash your PR and fix the commit message (and PR title)? |
Tobion commentedJan 11, 2016
Needs tests IMO |
stof commentedJan 11, 2016
Can you add a test covering this ? |
peterrehm commentedJan 11, 2016
Not sure yet, working on it. |
peterrehm commentedJan 11, 2016
Just added a test, please have a look at the test.@nicolas-grekas Otherwise all your comments should have been incorporated as well... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
please keep this dataProvider near the test using it instead of inserting your new test between them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Updated.
nicolas-grekas commentedJan 12, 2016
Thank you@peterrehm. |
…terfaces (peterrehm)This PR was merged into the 2.8 branch.Discussion----------[Debug] Fixed erroneous deprecation notice for extended Interfaces| Q | A| ------------- | ---| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#16643,#16775| License | MIT| Doc PR | -Replaces#16775.Commits-------5f4e968 Fixed erroneous deprecation notice for extended Interfaces
Tobion commentedJan 12, 2016
So when a user-land interface extends a deprecated symfony interface, there are no deprecation messages anymore? |
nicolas-grekas commentedJan 12, 2016
yes there is, when the user interface is loaded, isn't it? |
Tobion commentedJan 12, 2016
Ok I just wanted to make sure. |
Replaces#16775.