Behat: Many final classes miss proper interfaces

[Yogarine]

I'm currently writing an extension for Behat which requires me two extend and/or wrap some specific behat classes. I attempt to do this using the DI, however the fact that almost all of the classes in Behat are declared final makes this near-impossible unless I re-implement (i.e. copy-paste) almost all of the classes in a specific class dependency tree.

I understand very well what the final class keyword is meant for and know of some specific cases where I personally would recommend and use it. However, can someone please explain me the philosophy behind the extensive usage of the final keyword in Behat?

Especially in a project that is designed to be extended through extensions and dependency injection, I would expect to be able to extend the project's classes.

EDIT:
The root problem is, in Behat, many final classes have additional public methods not specified in the interfaces, or do not implement an interface at all, and end up directly referenced in type hints.

This way you lose all the advantages and flexibility composition should bring in Behat.

[Yogarine]

Tagging @everzet because I think his feedback is most valuable.

[ciaranmcnulty]

Can you be more specific about the use cases? For all of the final classes there's an interface and in my experience developing extensions I've been able to use a Decorator most of the time

[Taluu]

What @ciaranmcnulty said : use decoration instead.

[stof]

Inheritance is the worst extension point ever from a maintainer PoV, because almost any change becomes a BC break (non-final classes with non-private visibility are the worst thing in this regard). the fact that the project is meant to be extended by extensions is precisely the reason why we need to care about backward compatibility (as we cannot patch all extensions out there each time Behat changes something).

[Yogarine]

Those are legit reasons for final classes. However, in Behat, many final classes have additional public methods not specified in the interfaces, and end up directly referenced in type hints.

Some examples are:

• Behat\Behat\Output\Statistics\PhaseStatistics::reset() which is directly depended on in JUnitFeaturePrinter et al.
• Behat\Behat\Context\Snippet\Generator\ContextSnippetGenerator::setContextIdentifier et al, which are directly used in ContextSnippetsController etc.
• Behat\Behat\Context\Snippet\ContextSnippet::getUsedClass()
• Behat\Behat\Context\ContextFactory::createContext()
• etc.

I guess in that case the context of this issue changes to "all public methods of final classes should be declared in an interface & final classes shouldn't be directly referenced in type hints".

The case I'm currently dealing with right now is PhaseStatistics. I want to replace it with my own implementation, but the JUnitFeaturePrinter constructor will have nothing of it.

[Yogarine]

Inheritance is the worst extension point ever from a maintainer PoV, because almost any change becomes a BC break (non-final classes with non-private visibility are the worst thing in this regard). the fact that the project is meant to be extended by extensions is precisely the reason why we need to care about backward compatibility (as we cannot patch all extensions out there each time Behat changes something).

That all depends on whether you consider protected methods to be part of you "public" interface, or not. Either way you'll have to conform to an interface for backwards compatibility. (It's also the reason Java has an additional "package" visibility.)
It's true that delegating that interface to actual interface declarations gives more flexibility. However it does mean that an extension writer that wants to make just small modifications to the logic needs to either wrap the object it's extending or implement all the logic themselves.

In this light, I would like to see that Behat delegated logic to traits to make it easier to make smaller extensions. However, considering Behat currently has zero traits, I suspect that the Behat developers don't share my design philosophy.

[Taluu]

At the time, Behat required php 5.3+, which doesn't have traits. And what you want to do, you can decorate the PhaseStatistics service and it should do the trick.

see https://symfony.com/doc/current/service_container/service_decoration.html

[stof]

needs to either wrap the object it's extending

that's the whole point of composition.

That all depends on whether you consider protected methods to be part of you "public" interface, or not. Either way you'll have to conform to an interface for backwards compatibility. (It's also the reason Java has an additional "package" visibility.)

Well, if we don't provide BC on protected APIs, this means that extensions cannot use them safely. This is making the ecosystem even worse (it is basically saying "we expect you to author extensions but we don't care about you").
And package visibility would not help you here: your extension would still be in a separate package.

[Yogarine]

you can decorate the PhaseStatistics service and it should do the trick.

see https://symfony.com/doc/current/service_container/service_decoration.html

Only if the decorator somehow manages to extend the final class PhaseStatistics since that's what's specifically type hinted in JUnitFeaturePrinter:

namespace Behat\Behat\Output\Node\Printer\JUnit;

// [...]

final class JUnitFeaturePrinter implements FeaturePrinter
{

    // [...]

    public function __construct(PhaseStatistics $statistics, JUnitDurationListener $durationListener = null)
    {
        $this->statistics = $statistics;
        $this->durationListener = $durationListener;
    }

I'm not sure if I was clear enough, but what I'm trying to drive home here is that many final classes declare additional methods which aren't defined in the interface(s) they implement, and are then directly type hinted in the places where these methods are used.

If you don't declare all public methods in an interface, and end up directly type hinting the class instead of the interface, you lose all the advantages of composition with final classes.

I haven't tried the Symfony decorator, but even if it manages, through some kind of black magic, to extend the final PhaseStatistics class, this is still not proper usage of final classes. I suspect, however, that the Symfony decorator won't work in this case. (I can try if you really insist.)

[Yogarine]

And package visibility would not help you here: your extension would still be in a separate package.

With package visibility you can differentiate between protected methods, which are part of your interface towards other developers that wish to extend your class, and "package" level methods, which can only be called by classes extending your class in the same package.

"package" visibility allows you to declare methods which don't have to be backwards compatible. So it helps because than you _can_ provide backwards compatibility on protected APIs.

Either way, this is purely hypothetical because PHP doesn't support package visibility.