Sonataadminbundle: BC-break Compile Error: Cannot override final method Sonata\AdminBundle\Controller\CRUDController::redirectToList()

[Koc]

Environment

Sonata packages

$ composer show --latest 'sonata-project/*'
sonata-project/admin-bundle              3.28.0 3.28.0 The missing Symfony Admin Generator
sonata-project/block-bundle              3.8.0  3.8.0  Symfony SonataBlockBundle
sonata-project/cache                     2.0.0  2.0.0  Cache library
sonata-project/core-bundle               3.7.1  3.7.1  Symfony SonataCoreBundle
sonata-project/datagrid-bundle           2.3.0  2.3.0  Symfony SonataDatagridBundle
sonata-project/doctrine-extensions       1.0.2  1.0.2  Doctrine2 behavioral extensions
sonata-project/doctrine-orm-admin-bundle 3.2.0  3.2.0  Symfony Sonata / Integrate Doctrine ORM into the SonataAdminBundle
sonata-project/easy-extends-bundle       2.3.0  2.3.0  Symfony SonataEasyExtendsBundle
sonata-project/exporter                  1.8.0  1.8.0  Lightweight Exporter library
sonata-project/intl-bundle               2.2.4  2.4.0  Symfony SonataIntlBundle
sonata-project/notification-bundle       3.2.0  3.2.0  Symfony SonataNotificationBundle

Symfony packages

$ composer show --latest 'symfony/*'
symfony/assetic-bundle     v2.8.2  v2.8.2 Integrates Assetic into Symfony2
symfony/cache              v3.2.14 v3.4.0 Symfony implementation of PSR-6
symfony/monolog-bundle     v2.12.1 v3.1.2 Symfony MonologBundle
symfony/polyfill-apcu      v1.6.0  v1.6.0 Symfony polyfill backporting apcu_* functions to lower PHP versions
symfony/polyfill-intl-icu  v1.6.0  v1.6.0 Symfony polyfill for intl's ICU-related data and classes
symfony/polyfill-mbstring  v1.6.0  v1.6.0 Symfony polyfill for the Mbstring extension
symfony/polyfill-php54     v1.6.0  v1.6.0 Symfony polyfill backporting some PHP 5.4+ features to lower PHP versions
symfony/polyfill-php55     v1.6.0  v1.6.0 Symfony polyfill backporting some PHP 5.5+ features to lower PHP versions
symfony/polyfill-php56     v1.6.0  v1.6.0 Symfony polyfill backporting some PHP 5.6+ features to lower PHP versions
symfony/polyfill-php70     v1.6.0  v1.6.0 Symfony polyfill backporting some PHP 7.0+ features to lower PHP versions
symfony/polyfill-util      v1.6.0  v1.6.0 Symfony utilities for portability of PHP codes
symfony/security-acl       v3.0.0  v3.0.0 Symfony Security Component - ACL (Access Control List)
symfony/swiftmailer-bundle v2.6.7  v3.1.6 Symfony SwiftmailerBundle
symfony/symfony            v2.8.31 v3.4.0 The Symfony PHP framework

PHP version

$ php -v
PHP 7.1.11-1+ubuntu16.04.1+deb.sury.org+1 (cli) (built: Oct 27 2017 13:49:56) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.1.0, Copyright (c) 1998-2017 Zend Technologies
    with Zend OPcache v7.1.11-1+ubuntu16.04.1+deb.sury.org+1, Copyright (c) 1999-2017, by Zend Technologies
    with Xdebug v2.5.5, Copyright (c) 2002-2017, by Derick Rethans

Subject

Looks like #4741 caused BC-break.

Steps to reproduce

1. Create custom controller wich extends from CRUDController and define own method redirectToList.
2. Update to the latest version

Expected results

Minior version do not break my code.

Actual results

Compile Error: Cannot override final method Sonata\AdminBundle\Controller\CRUDController::redirectToList()

[greg0ire]

lol @peter-gribanov had the same issue. Why did you both had the idea to name a method exactly like that? Is that bad luck or is there something I am missing?

[Koc]

It's the most logical name for this method. Looks like we think alike

[greg0ire]

It makes sense.

@sonata-project/contributors this is why inheritance is bad. Not sure we should really cover this with our BC promise though.

[Koc]

We can use annotation and make it real final on next major. https://github.com/symfony/debug/blob/master/DebugClassLoader.php#L229 handles @final annotations and report violations in profiler.

[greg0ire]

@Koc if you implemented it differently than we did, you want it to break now. You don't want something else to happen when we call redirectToList. It should really break, not warn.

[Koc]

I have same signature, without any arguments.

[greg0ire]

I know but what if someone else hasn't? Also, what do you do in your implementation that we don't? And should it be done every time we call redirectToList?

[peter-gribanov]

The basic implementation is the same as for you, but in one of the CRUD controllers it is redefined.

Example:

protected function redirectToList()
{
    if ($return_url = $this->getRequestReturnUrl()) {
        return $this->redirect($return_url);
    }

    if ($return_url = $this->getSessionReturnUrl()) {
        return $this->redirect($return_url);
    }

    return parent::redirectToList();
}

I need go to the return_url page if exists and we need to do this in any action, including the batch actions.

Now it is impossible to redefine it in batch actions.

[greg0ire]

That's not the right extension point though: you are not redirecting to a list at all. I think it would make more sense to rename the method to redirectAfterPost

[core23]

Adding new methods is always a BC break. Also if we haven't made this method final it could result as a BC break, e.g. if someone has required parameters

[jordisala1991]

I add here too, just in case someone else had the same problem:

http://symfony.com/doc/current/contributing/code/bc.html

We follow this BC promise. We are allowed to add protected methods to a class. If you extend a class and add a method we do not guarantee BC compatibility. As Symfony does the same

[Koc]

Agree with this BC promise

[Koc]

In any case we got a very funny issue with Peter)

[greg0ire]

Nice xkcd @core23 ;)