Skip to content

Consistent removal of deprecations and reporting a fatal where a deprecation was previously reported #21569

@kkmuffme

Description

@kkmuffme

Description

While some things reported as PHP Deprecated seems to get removed (= fatal) in the subsequent version, there are others where that only happens in the next minor version after/randomly after some minor versions or in the next major version.

Especially with compile time errors - which by default will only get reported once on startup if opcache is enabled - this means that lots of deprecations go unnoticed for a long time and then possibly result in a fatal error at the "wrong" place (= the fatal is reported for the caller/"user" instead of for the code that is deprecated) or result in previously ignored errors (in various error reporting tools, e.g. Sentry) to again show up if the error message changed.

e.g.

  1. deprecated in 8.0, fatal in 8.1 (why does it still report a deprecation though on the declaration instead of a fatal directly?)
    https://3v4l.org/nHkd1#v8.1.34
<?php
function some_lib(array $a = [], string $b) {
    echo $b . PHP_EOL;
}

some_lib( b: 'hello' );

deprecated in 8.4, not a fatal in 8.5

function some_lib(array $a = null, string $b) {
    echo $b . PHP_EOL;
}

some_lib( b: 'hello', a: null );
  1. deprecated in PHP 8.0, due to changes in default error message in PHP 8.4, this fatal is reported as 2 separate errors. The fatal however should be already at the declaration of some_lib
    https://3v4l.org/nHkd1
<?php
function some_lib(array $a = [], string $b) {
    echo $b . PHP_EOL;
}

some_lib( b: 'hello' );
  1. while there are obviously other cases where you would end up with this fatal error, this specific one would report the fatal on the deprecated default null instead of on the assignment.

https://3v4l.org/0orf9#v8.5.3

<?php

abstract class BaseLib {
    abstract protected function process( $a );
}

class MyLib extends BaseLib {
    //protected $a;
    protected string $a;
    
    public function run( string $a ) {
        $this->process( $a );
    }
    
    protected function process( $a ) {
        $this->a = $a;
    }
}

class ThirdParty extends MyLib {
    public function run( string $a = null ) {
        $this->process( $a );
    }
}


$third_party_code = new ThirdParty();
$third_party_code->run();

Assume you are the author of MyLib, which you distribute with your software but, since it's open source, is also used by other software.
In a new release you changed protected $a; to protected string $a; since you only assigned this to a string in all cases in your own use case and that new version also requires PHP 8.5 as minimum.
You cannot add a type hint, since the BaseLib abstract method, does not have one either (assume the BaseLib is a CMS class you do not manage)

That CMS (and/or that popular hosting company) suppresses E_DEPRECATED error, which means the CMS users never see

Deprecated: ThirdParty::run(): Implicitly marking parameter $a as nullable is deprecated, the explicit nullable type must be used instead in /in/0orf9 on line 21

However, they see:

Fatal error: Uncaught TypeError: Cannot assign null to property MyLib::$a of type string in /in/0orf9:16

What happens now:
Since it's a CMS used by lots of people who are not software developers, lots of people start spamming your company email, your github, and posting in various places that they have a fatal bc of a class, where the namespace/class name contains your company name.

However: the issue isn't in your code. You could obviously add a condition to handle this, but even then (e.g. a trigger_error) it would end up reporting on your class.
If however, the deprecation of the nullable default would lead to a fatal error instead of a deprecation, it would report the fatal in ThirdParty class instead
(I know they could just set the param type to string|null and we end up with the same issue, but that's unrelated to deprecations)


Summary:
What is reported as deprecated, should be changed to a fatal in the next version in all cases at the location of where the deprecated was reported previously.
Otherwise it will end up reporting a fatal at a place that did not emit any deprecated before (see case 1).
This means that instead of getting 1 fatal in that 1 place that has deprecated code, you could suddenly get 100s of different fatal errors in 100 different places that call/use that function/method that uses deprecated behavior.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions