class_alias() doesn't work really well for deprecation layers because the notice is thrown depending on the loading order. Deprecating the service in favor of test.browser might be better?

It is true thatclass_alias is limited. However I am not sure if I understand your suggestion. The library I am using has the following class definition:

<?phpdeclare(strict_types=1);namespaceFriendsOfBehat\SymfonyExtension\Driver;useBehat\Mink\Driver\BrowserKitDriver;useSymfony\Component\BrowserKit\Client;useSymfony\Component\HttpKernel\KernelInterface;finalclass SymfonyDriverextends BrowserKitDriver{publicfunction__construct(KernelInterface$kernel,string$baseUrl)    {if (!$kernel->getContainer()->has('test.client')) {thrownew \RuntimeException(sprintf('Kernel "%s" used by Behat with "%s" environment and debug %s does not have "test.client" service.' ."\n" .'Please make sure the kernel is using "test" environment or have "framework.test" configuration option enabled.',get_class($kernel),$kernel->getEnvironment(),$kernel->isDebug() ?'enabled' :'disabled'            ));        }$testClient =$kernel->getContainer()->get('test.client');if (!$testClientinstanceof Client) {thrownew \RuntimeException(sprintf('Service "test.client" should be an instance of "%s", "%s" given.',                Client::class,get_class($testClient)            ));        }parent::__construct($testClient,$baseUrl);    }}

The servicetest.client could be deprecated. But still theinstanceof check should succeed. I don't see how to do the latter withoutclass_alias (or rearranging the class hierarchy).

Or do you mean that inaddition to theclass_alias to define a new servicetest.browser and deprecatingtest.client?

I was hopping that creating a new service would be enough but it wouldn't allow passing legacy type hints.
The best solution then is to make AbstractBrowser extend from Client.

If I am not mistaken, the Client should also have the same public interface as AbstractBrowser then. I'm not sure if that's worth it? Wouldn't it be against the intent of the original PR?

It is possible though, rename AbstractBrowser back to Client. Create a new empty AbstractBrowser extending from Client. Then upon removal of Client drop AbstractBrowser and rename Client to AbstractBrowser. Bit cumbersome, but perhaps the safest way.

It does have the same public interface, see its implementation, unless I missed something?

I meant that all methods from AbstractBrowser have to be moved to Client if Client is the top of the hierarchy.

Anyway, I am willing to do this. However I am not sure how deprecation will work. We cannot trigger an error in Client then as AbstractBrowser will depend on it?

We will have to remove the deprecation notice and keep the annotation. DebugClassLoader will still warn ppl that extend Client directly.

Sounds good. I think I have one last question concerning the deprecation messages. Currently the tests expect them on AbstractBrowser methods.

Calling the "Symfony\Component\BrowserKit\AbstractBrowser::getInternalRequest()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.

If I alter the class hierarchy__METHOD__ will resolve to methods onClient and yield this:

 Calling the "Symfony\Component\BrowserKit\Client::getInternalRequest()" method before the "request()" one is deprecated since Symfony 4.1 and will throw an exception in 5.0.

I can do three things:

1. Alter the tests to expect the latter deprecation instead of the first. But people might be confused to see deprecated Client appear the message while they don't depend on it themselves.
2. Hardcode the deprecation messages such that they refer to AbstractBrowser instead.
3. Usestatic::class orget_called_class to generate deprecation messages. This means that the message contains always the concrete instance type. For exampleKernelBrowser::getInternalRequest().

I don't know how you guys usually solve this. I don't see evidence of variant 2 or 3 in the code base so far. Solution 1 is the way to go?

Thanks for bearing with me and my questions 👍

Option 1. makes the most sense to me: all existing code usesClient - and not yet written code should not use the method anyway.

You are right :). I have pushed a separate commit with the changes we discussed.

Thank you@Devristo.