Native return types on interfaces conflict with the BC goal of 3.0

[rimas-kudelis]

Monolog version 3

#1648 has added native return types on two interfaces:

• Monolog\Processor\ProcessorInterface
• Monolog\ResettableInterface

While the latter probably won't cause much problems, the former might and will, because a Processor package which intends to be compatible with both ^2.0 and ^3.0 will have a harder time implementing such compatibility. Furthermore, the UPGRADE.md explicitly mentions that "The interfaces do not require a LogRecord return type even where it would be applicable".

I suppose the change to ProcessorInterface was a simple mass-replacement mistake?

[Seldaek]

The ResettableInterface problem is no problem at all as you can already add the void type in 2.x.

The ProcessorInterface is indeed a tricky one.. As the 2.x one had __invoke(array $record) we are left with these options for 3.x:

• __invoke($record) which should allow it to work everywhere but types nothing, and doens't allow people to use a LogRecord type at all on the parameter, and lacks return type altho implementors can add one
• __invoke(array|LogRecord $record) which requires php8 but should allow it to work everywhere and offers a bit more type safety but it is kinda meaningless in the monolog3 context, and lacks return type altho implementors can add one
• __invoke(LogRecord $record): LogRecord is indeed incompatible but at least allows correct types if you fully embrace 3.x

The problem is now I can't even change it without breaking things further in 3.x, so yes it sucks a bit but I don't think it should change as the current way is better long term.

For the short term there are two workarounds:

• Conditionally declare a base class or trait depending on the Monolog version like I did in symfony-bridge to support both versions: https://github.com/symfony/symfony/blob/6.2/src/Symfony/Bridge/Monolog/Processor/CompatibilityProcessor.php
• Skip ProcessorInterface entirely, and just implement __invoke as you please. Monolog just requires processors to be callables and does not actually care about the ProcessorInterface, so if you don't need it for autowiring things in a Symfony context for example I think this is the better way.

[rimas-kudelis]

Hi, and thanks for the response! What you said here does conflict with the statements in UPGRADE.md though.

Could you elaborate how implementing option 2 would "break things further"? I do agree it would be kinda suboptimal from purely Monolog3 POV, but that I don't see how it would break anything.

[Seldaek]

I mean I cannot change this anymore in 3.x without breaking symfony code for example, as narrowing the parameter types in implementations is not allowed https://3v4l.org/BCSSi

What seems like a viable option tho is to remove the return type only, that would allow you to use LogRecord only on both param and return types, or you can widen it to LogRecord|array if your min php version is 8+. See https://3v4l.org/L97Z5

That people write Monolog 2/3 compatible ProcessorInterface impl as long as they require PHP 8.

[rimas-kudelis]

Oh yeah, I was specifically talking about return types only.

[Seldaek]

Indeed, sorry for the confusion ;) I'll get this in the next release then.

[rimas-kudelis]

Thank you!