Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A tree builder without a root node is deprecated since Symfony 4.2 #251

Closed
belinde opened this issue Jan 7, 2019 · 5 comments
Closed

A tree builder without a root node is deprecated since Symfony 4.2 #251

belinde opened this issue Jan 7, 2019 · 5 comments

Comments

@belinde
Copy link

belinde commented Jan 7, 2019

In DependencyInjection\Configuration::getConfigTreeBuilder() a TreeBuilder is istantiated without arguments, and so triggers an E_USER_DEPRECATED error on Symfony 4.2. The problem is in the first two rows of method:

        $treeBuilder = new TreeBuilder();
        $rootNode    = $treeBuilder->root('doctrine_migrations', 'array');

I think it could be safe put the parameters ALSO in the constructor, so the error is suppressed in Symfony 4.2 but the code still works for previous versions:

        $treeBuilder = new TreeBuilder('doctrine_migrations', 'array');
        if (!is_callable([$treeBuilder, 'getRootNode'])) { // hack for backward compatibility
            $rootNode = $treeBuilder->root('doctrine_migrations', 'array');
        }

In this way new versions get the arguments in the constructor, and old versions silently ignore them. I assume that getRootNode() has been introduced in the same time of the deprecation, so it could be a good and almost inexpensive check.

@greg0ire
Copy link
Member

greg0ire commented Jan 7, 2019

Can you make a PR? Here is an example: sonata-project/SonataAdminBundle#5360

@alcaeus
Copy link
Member

alcaeus commented Jan 7, 2019

Yes, a PR would be great. Please make sure to send it to the 1.3 branch as we want to fix the error not only in 2.0 (which is master) but 1.3 as well. 👍

@belinde
Copy link
Author

belinde commented Jan 7, 2019

I wasn't sure about the quality of the fix... Sure, I'll do tomorrow, I don't have a PC available right now.

@belinde
Copy link
Author

belinde commented Jan 8, 2019

In branch 1.3 the issue has been fixed in commit e16ce0c by user Chalas, in date 2018-12-01, so it has just to be merged in master.

@jwage
Copy link
Member

jwage commented Jan 9, 2019

This was merged to master and is in 2.0

@jwage jwage closed this as completed Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants