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:
Please don't compare us, tool for improving codebase, with a tool that degrades code quality by removing its important parts 😬. Fixer would rather add final and readonly, than remove them. We even have auto-review tests to ensure our classes are final 🤷♂️. Final is good! 😎— PHP-CS-Fixer (@PHPCSFixer) May 6, 2023
“Next week” was actually few months later, and
unfinalize was released. What happened later is one hell of a PHP story!
unfinalize was released I created pull request to #PHP-CS-Fixer and sent a message on our core members’ Messenger chat:
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…
This is hilarious! They actually did it! 🎉 They blocked Unfinalize inside of PHP CS Fixer itself! 😂— Steve Bauman (@ste_bau) October 3, 2023
I consider this a badge of honour friends 👏
Time to fork https://t.co/Erkk2Qklvc
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.
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
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 🤷♂️.
dg/bypass-finals does the same
Yes, and no.
There is a huge difference between bypassing `final` in your own classes for testing purposes (dg/bypass-finals), and trimming `final` from vendor code to hijack it (stevebauman/unfinalize). Also a huge difference in the authors' approach ("marketing").— Greg Korba ➡️ @[email protected] (@_Codito_) September 30, 2023
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:
conflictintroduced by my PR effectively started to work in Fixer v3.32.0, while for
unfinalizeit 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).
unfinalizeis 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
conflictentry 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
conflictentry 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.
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.