Recently I’ve created pull request to #PHP-CS-Fixer that aimed to prevent installing stevebauman/unfinalize along with friendsofphp/php-cs-fixer, effectively blocking Fixer to be installed as a dev dependency for that tool. That escalated quickly šŸ˜…!

Some time ago in a galaravelaxy not so far away

In #PHP world, discussion about final is everlasting. Every time it pops out somewhere, it engages a lot of people (including me), but it’s mostly wasting everyone’s time, since both sides rather have well-formed opinion about it and are not open to change it. One of these takes sparked initial conflict of interests:

“Next week” was actually few months later, and unfinalize was released. What happened later is one hell of a PHP story!

Half-joke, half-intention

Shortly after unfinalize was released I created pull request to #PHP-CS-Fixer and sent a message on our core members’ Messenger chat:

Easter egg at the lowest cost

Translation: ā€žEaster egg at the lowest costā€Ÿ

Because unfinalize was some kind of trolling (but with all the seriousness under the hood, when it comes to getting rid of final keyword and giving “freedom” to developers), I thought we can continue this way. Of course, making fun was only part of that, because I was also dead serious in the PR’s description. Maybe the fact I described it with that particular tone was decisive when it came to general reception by the community? Probably I should keep that pastiche trend and write some kind of Robin Under-the-Hood speech šŸ˜‰.

Anyway, it probably wouldn’t escalate so quick if I hadn’t commented this PR and hadn’t got involved in the discussion, that drove Ghostwriter mad šŸ˜…. He was so disgusted by my “act of sabotage”, that he changed implementation of unfinalize to be Fixer-agnostic (which I found totally valid and I think it should be merged anyway). This PR has put the change made into Fixer straight under unfinalize’s author nose…

The Drama

Yeah, it clicked šŸ˜†. From this point the amount of dislikes on my PR got rising curve, people started to calling me names (examples: 1, 2), and discussion became šŸŒ¶ļø hot.

I did my best to not get provoked by any of the comments, to provide arguments and reasoning, but it was mostly tilting at windmills. Some people came there only to call me “petty”, without even the slightest will to understand what happened. Not like it surprised me in any way, but still šŸ˜….

In the end my pull request was reverted and Keradus took the blame on himself, which I really respect, but honestly: it was my initiative, I’ve created the pull request and it was me who advocated for it on private chat.

Reasons behind the pull request

Let’s ignore the joke part of that PR for now, and focus on all the serious reasons behind it. If you have the will and time, you can read original PR’s and revert PR’s comments, along with several tweets/threads linked here and there. Even though I think reverting this change was the best we could do, I still stand behind my reasoning.

Modifying vendor code is NOT a coding standard

#PHP-CS-Fixer is a tool which aims for raising standards in PHP codebases. It was created to let developers apply fixes to their code in an automated way. As unfinalize has showed, it’s also technically possible to use Fixer as an engine for applying changes to vendor code. The fact you could do it does not mean you should. Modifying vendor code is not a part of any coding standard and that’s why we don’t like the fact it is used for that.

Additionally, unfinalize is a CLI tool built with Laravel Zero, that bundles its dependencies into executable PHAR file, hiding them from end users. I believe that even though Fixer is free to use and modify under its licence, bundling it without any really visible credit (not counting GitHub’s readme and composer.json) is not fair.

Enabling disallowed inheritance may be harmful

Software design is hard. Open source maintenance is hard. Combine these two and you get ticking bomb.

I truly believe that “final by default” is a great approach (more here and here). It does not mean developers should put final everywhere blindly, it just means that it’s more safe to keep your classes final, and open them only when needed. Developers should design their code with users in their mind, so proper extension points should be provided, which would make extending not required (because Open-Closed principle is not about inheritance).

Having that in mind, I think that unfinalize may be harmful for its users. Removing final from vendor code breaks the contract between the libraries’ authors and the users, who do it. You have to remember, that from authors’ point of view those classes are still final, and they maintain and develop them with totally different mindset than classes that were explicitly opened for inheritance. If you unfinalize vendor code and extend classes that supposed to be non-extendable, you may encounter breaking changes on any Composer update. Moreover: you can’t expect the support from the authors, because you’re using their code not in a way they designed it.

Example: you wanted to have some method, but it wasn’t there, so you removed final, extended the class and added that method, but now it’s added in the upstream, it has different signature and PHP throws fatal error… It’s of course recoverable, but do you really want to waste your time on adapting to upstream’s changes instead of implementing proper solution in the first place? You should get familiar with what vendor code offers, try to customise its behaviour through existing, public contract, and if your use case is not covered, first thing you should do is raising an issue in the project’s repository. Applying modifications to vendor code should be last thing to do, and even if you really need it, you can do it using several approaches that were already available before unfinalize was created.

I can’t even imagine the level of potential harmfulness when this idea gets implemented… šŸ™„

Unfinalize is a serious joke

Last and least important: I really dislike the way how unfinalize is marketed. Release notes are hilarious and make you consider the whole thing as a joke, but as I already said, seriousness is there. Steve truly believes that using final in OSS code is wrong, and maintainers are servants. All that “bring back the freedom” blabbering really irritates me, because final does not take freedom from the users, it only keeps control on maintainers’ side (as it should be, since they’re responsible for the package). Actual freedom for users can be provided in many other ways, not only through inheritance.

There were many people stating that my pull request was based on personal preferences. Steve even suggested it’s a vendetta šŸ˜‚. All of these people have focused on the wrong thing. PR’s description was clear: it was about Fixer being used as an engine for unfinalize. Talking about “personal preference” is simply laughable.

Frequently raised counter-arguments

Composer patches has been around for years

Yes, and I really think it’s a great tool. The fact it already existed makes unfinalize redundant and all the drama could be avoided just by… not implementing it šŸ¤·ā€ā™‚ļø.

But, but… dg/bypass-finals does the same

Yes, and no.

dg/bypass-finals aims for making devs’ lives easier when it comes to testing. As we all know, when class is final, PHPUnit’s mocking engine doesn’t work, that’s why if you want to mark class as final, but at the same time you have the need to mock it in your tests, with simple DG\BypassFinals::enable(); in your tests’ bootstrap you can make these classes mockable again.

Of course, nothing prevents you from adding it to app’s main bootstrap and bypass all the finals in the production code. Just like nobody prevents you from putting the nail into electrical outlet or drink gasoline…

The actual impact of my pull request

Now let’s go back to the fun part. It was pretty interesting to observe people’s reactions, considering how much they did not think about an actual impact of introducing that conflict entry. Many comments suggested that I “killed the project”, but here are some facts:

  • conflict introduced by my PR effectively started to work in Fixer v3.32.0, while for unfinalize it wasn’t needed to bump to that version. Project could still use older versions for building PHAR and everything would work without any change on their side (because Composer would resolve dependencies automatically and just wouldn’t install newer, “conflicting” version).
  • Since unfinalize is provided as pre-built CLI tool that can be downloaded directly from release pages, it’s not even required to install it through Composer, so conflict entry does not matter from end users’ perspective.
  • Large part of unfinalize’s potential users are from Laravel community, that does not use Fixer, but Pint (well, effectively they actually use Fixer, but they don’t know about it, they waited for Pint their whole lives šŸ˜‚). Since Pint also bundles Fixer in executable file, there’s no conflict with unfinalize.
  • The conflict entry itself was naive approach that could be easily worked around in several ways.

I’ll put it straight: the impact was almost non-existent šŸ˜†. The only scenario where this conflict would seriously impact users: Fixer installed as project’s direct dependency with the need to install unfinalize. I believe it would be marginal subset of projects, which means the whole drama and the witch hunt, that part of the PHP community carried out, was totally unnecessary.

Lessons learned

The most important lesson from this story is that open source is not a playground or circus arena. There are real users, with real projects, that expect upstream packages to be viable, stable and trustworthy. Even though the actual impact of my pull request was really negligible, it could affect some developers and waste their time on trying to work around the conflict, which in fact is not their problem. After all the discussions and based on my own reflections, I would like to say sorry for the potential trouble my PR could make. It does not mean I’ve changed my mind about unfinalize - no, I stand firm behind all I wrote and said about the tool itself, I just only think it did not make much sense to create that easter egg and to fight joke with a joke, using widely used, mature tool like a PHP-CS-Fixer as a platform.

Another lesson is much more mundane: Larafolks have no chill šŸ˜‚. It’s not the first situation where they show double standards, and at this point it does not even surprise me. #GoodVibesOnly looks great, but in practice some of these people are just toxic.

The last lesson is simple: don’t waste your time on trying to convince someone that doesn’t want to listen. I’m always open for discussion, but arguing with close-minded people is only throwing peas against the wall.