Straightening the PHP APIs / Deprecations


(Benni Mack) #1

Clearing up PHP APIs for 9.0

In the past few months we heard a lot of discussions (also on decisions.typo3.org) that TYPO3 Core development is too slow or too fast. After giving some thoughts to it, I think part of it is related to removing / breaking code during minor releases which is necessary but kind of in the grey zone of “according to the deprecation strategy” (= break if it is not possible, or if it unused, or does not fit the new concept) - in the future I’d like to set up a clearer deprecation strategy. However, introducing semver now would keep us from a lot of legacy code (15ys+) which still needs to be redone.

So, before coming up with a final deprecation process (which will certainly go closer to the semver approach), I think we need to make one “major” breaking change for 9.0. That is, to streamline the PHP API once and for all.

This is how it would look like

PHP classes

  • Any PHP class which should be considered “final” from the TYPO3 Core, should also be declared final.
  • Renaming classes can only happen via class aliases, which will stay until the next major version.
  • Removing classes is not possible within minor versions.

PHP interfaces (tbd)

  • Interfaces can only change by either introducing new interfaces or removing methods from the interface.
  • Instead, new interfaces should be introduced to superseed the original interface.

PHP methods / functions

  • Introduce “private” instead of marking everything as “protected” where it really makes sense
  • All PHP methods which are certainly only useful for a PHP class itself, should be put to “private” instead of “protected”
  • We still keep a lot of public methods which, just by marking them protected, would be breaking. Go over all public methods and mark them as protected (for XCLASSing)
  • We have a lot of PHP methods where their method signature changes (= changes behaviour). When the old behaviour can not be kept, this is breaking, and should not be allowed in a minor release.

PHP class members / properties

  • All properties which still come from PHP4 times should be migrated to private or protected at least, to ensure the scope.
  • This might probably the heaviest load work to do.

Additional phpdoc functionality

All PHP code which should only used within TYPO3 Core (aka within sysexts) should get an @internal or @private (for public properties) and are excluded from the deprecation approach. Remove @api annotations

Further definitions

We still have some other open topics (mostly regarding global arrays and GET/POST variables which might be changed) but this will come at a later stage.

All work will run through me (I know, it’s a huge task, but should be done in one / two weeks full-time), and needs to be done before 9.0. I will make time available for end of september / october to do this task.

Impact

  • 9.0 will be released after this change has gone through (which might be after a decision and after my work)
  • Some extensions might break / XCLASSing might have impact

Possible Migrations

  • If we accidentally changed a property / method from public/protected to protected/private, we can change that back again in 9.1, 9.2 etc. which gives enough time to test.

Pro

  • A lot less breaking changes in the future in terms of PHP API
  • A clear PHP API that is exposed to extension authors, which can be documented properly
  • Officially allow private class members / methods in the TYPO3 Core (= is not meant to be XCLASSed).

Con

  • Breaking for all random edge cases that people might use TYPO3 for.
  • Quite some work for me, but I’d love to do that.

Notes

I have some more proposals for the API in a separate doc, which I want to prepare as “deprecation strategy starting with 9.0” which I will continue working on if we find a general consensus on this idea.

Organizational

Topic Initiator: Benni
Topic Mentor: Benni


(Markus Klein) #2

While this is very clean in terms of API and code maintenance it can be a real blocker for necessary workarounds of core bugs fore real life projects as this completely denies any way of extending a class to replace it via XCLASS.
This needs an extra careful thought!

Is that compatible with phpunit and mocking functions? (My brain tells me something like “once it was not possible to mock private methods”.) [quote=“benni, post:1, topic:261”]
All work will run through me (I know, it’s a huge task, but should be done in one / two weeks full-time), and needs to be done before 9.0. I will make time available for end of september / october to do this task.
[/quote]

But we definitely need some reviewers with at least the same time budget to properly review things. Small mistakes may happen easily with such “mass changes”.


(Jigal Van Hemert) #3

The technical details are way too premature for me. There are a lot of things just mentioned here that are concepts by themselves.

Semantic versioning

This makes sense for libraries but less for applications. The fact that extensions have dependencies on the core is not because the extensions are the application that consumes the API of the core but because they are smaller or bigger modifications that rely on certain characteristics of that core. The most practical way to describe and manage those characteristics (such as function X exists, functionality Y behaves in this way) is to specify a version or range of versions.

The current schema with minor versions towards an LTS is a nice compromise between rolling releases and massive releases every 18 months.

Final classes and private members

Preventing XCLASSing would limit the way TYPO3 functionality can be extended. Hooks and signal/slots are very nice but in some cases XCLASSing provides a nice solution. If a function is protected instead of private the XCLASS can easily call the original function and modify some things before or after this call. This makes the modification more likely to be compatible for a longer time.
Final classes are just a pain.

Extensibility as a feature

It has always been a great feature of TYPO3 that almost anything could be changed in the system with more or less effort. By registering components, using hooks or signal/slots and ultimately XCLASSing the core functionality the CMS could be adjusted to the needs of the project.

Stable API for extensions

After the release of an LTS version the API should stay stable (unless security issues require breaking changes). Before that release there is really no need to keep it from changing. Yes, it would look cool if an extension works with core 4.5 until 8.7 but with the namespace changes it became apparent that this would not be a feasible situation. Nowadays I only make extensions compatible with one or perhaps two releases. Installations with current stable or old stable cores do not often use new features of extensions. It would in most cases not matter if no new features would be added at all to the included extensions. In fact, it would make life easy for maintainers of a website if newer (compatible) versions of extensions had no breaking changes (period).

I would propose a practical and pragmatic approach with as much flexibility as possible over an approach with technically clean implementations and rules. We already have a few cases where the technically clean solution reduces usability (one example: each tab in the link wizard is now separated which leads to weird side effects when switching tabs. Option values change in ways that are not logical to editors.)


(Claus Due) #4

+1 to everything Markus wrote and in particular to this premise that Jigal wrote. I would add that until TYPO3 is perfectly refactored and we program completely to interfaces rather than implementations, we shouldn’t even be considering “final class”.

And to do that, I would focus first on programming to interfaces everywhere that this is currently possible, then adding new interfaces where that’s reasonable and start programming to those instead of implementations. And finally once those are in place, consider making our implementations final only when we have a very good reason to do so. If that’s done well, the need for XCLASS pretty much disappears.

Side note: smaller interfaces means less to implement if you can’t subclass our implementations, so that should probably be a goal in itself.

This also needs very careful consideration especially if it might break function signature compatibility.


(Benni Mack) #5

Thanks for your input.

Final classes

I don’t expect to add a lot of “final classes” at all, but I think we should add the concept to our code base. The only ones I can currently think of would things like Bootstrap and SystemEnvironmentBuilder.

Private instead protected

Yes, this is a limitation for mocking with phpunit.

Work load

Before starting on the migration, the new deprecation / breaking “rules” need to be in place (and run through here) with detailled information on what will be breaking and what wouldn’t. And yes, we need more man power than just me :wink:


(Benni Mack) #6

Hey Claus + Jigal,

thanks for your feedback on the concept to code against interfaces, which would be a good strategy to continue adding new features.

What I consider one of the most pressing topics where I’d like to radically go with “protected” instead of “public” is within e.g. TypoScriptFrontendController (see the public properties within https://review.typo3.org/#/c/52799/ which is breaking, and happens to georg and me on a regular basis when taking on streamlining existing APIs) - the changes I would actually go with private instead of protected for the cache properties.

So, private would mainly,

I can also create a first draft on what to consider private/protected instead of public right now - you will see that this is mostly related to code that has been there > 8ys - for code like the Caching Framework or Extbase/Fluid I do not see a need to touch it at all (the phpdoc annotations would strive in that area).

Just to be clear on that: The main focus will be on doing a “public” => “protected” for most of the parts we’d touch, and around 80% I currently detected would be related to properties instead of methods. I still like the simplicity in XCLASSing and would keep that as max. flexible as possible of course.


(Claus Due) #7

Sounds good to me - I’ll happily review that list and I also think TSFE is a great place to start. That specific class needs a quite long term plan to encapsulate and refactor. I was mostly worried about the “make classes final” perspective and it seems we agree that XCLASS is vital enough that we must be extremely careful before going with final classes.

And if we keep the above plus the program-to-interfaces idea in mind throughout the coming refactoring then I say go for it.


(Claus Due) #8

I would much rather if you didn’t “final” those. I’m actually subclassing Bootstrap in two or three separate packages and expect to need a SystemEnvironmentBuilder subclass in an up-coming idea I have. Sometimes we do need to change the bootstrapping process and do deep manipulation of the environment that gets built.

Might I suggest instead starting much smaller - for example, final’ing some of the simpler Extbase validators which are very unlikely to be subclassed, share a common parent class and all implement the same interface?


(Benni Mack) #9

Hey,

this is a first draft on how you see how much “public” will become “protected” and how much “public” might become “protected”:

https://review.typo3.org/#/c/53630/1

A proper RST should show why decisions were made, as already mentioned.


(Susi Moog) #10

[quote=“liayn, post:2, topic:261, full:true”]

Is that compatible with phpunit and mocking functions? (My brain tells me something like “once it was not possible to mock private methods”.)[/quote]

Unit testing theory tells us that you should not mock private methods as these tests would only “nail down” your implementation and make it near impossible to change it. You should test your full unit // public entry point to ensure functionality, but it should not matter how many private methods you call. You should at any time be able to refactor code to multiple smaller methods, different private methods, or no private methods at all, without having to change the tests. The tests should make sure your API and behaviour stays the same, but should not fixate your implementation. If you notice a strong desire to test private / protected methods, it’s normally a good indicator that the design is maybe not ideal and could be refactored.


(Markus Klein) #11

:wink: I’m aware of all that, but the reality (existing Core tests) tells us a different story, that’s why I brought up that point.


(Benni Mack) #12

This topic was automatically closed after 28 days. New replies are no longer allowed.