Signature change of ArrayAccess/Iterator causes a deprecation warning

[aik099]

Is your feature request related to a problem?

Some time ago the declarations of the iterator-related interfaces (e.g. Interator, ArrayAccess and likely others) were changed to enforce return type. After the change on PHP 8.1 I'm getting deprecation notices like these:

Deprecated: Return type of kMySQLQuery::current() should either be compatible with Iterator::current(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1243

Deprecated: Return type of kMySQLQuery::next() should either be compatible with Iterator::next(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1267

Deprecated: Return type of kMySQLQuery::key() should either be compatible with Iterator::key(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1255

Deprecated: Return type of kMySQLQuery::valid() should either be compatible with Iterator::valid(): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1279

Deprecated: Return type of kMySQLQuery::rewind() should either be compatible with Iterator::rewind(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1231

Deprecated: Return type of kMySQLQuery::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1291

Deprecated: Return type of kMySQLQuery::seek($position) should either be compatible with SeekableIterator::seek(int $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1315

Deprecated: Return type of kMySQLQuery::current() should either be compatible with Iterator::current(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1243

Deprecated: Return type of kMySQLQuery::next() should either be compatible with Iterator::next(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in file.php on line 1267

for a code like this:

class myIterator implements Iterator {
    public function rewind() {

    }

    public function current() {

    }

    public function key() {

    }

    public function next() {

    }

    public function valid() {

    }
}

Describe the solution you'd like

I have no idea what RFC did this, sorry. As a solution I would expect such cases to be reported as needs fixing by the sniffs of this project.

Additional context (optional)

• I intend to create a pull request to implement this feature.

[jrfnl]

Thanks for opening this issue @aik099 .

The related RFC is https://wiki.php.net/rfc/internal_method_return_types

It is going to be tricky to sniff this, though probably doable for the most common cases.

Notes for when a sniff will be developed for this:

• Check class declaration signature for extends and implements and check if the extended classes/implemented interfaces are PHP native ones (will need namespace/use statement resolution).
  => This is the first point of failure/source of false negatives. If the PHP native class/interface is further down the inheritance chain we will not be able to detect it.
• Check for each method in the class if it is an overload of a method in the parent class / implementation of an interface method.
• Check if it has a return type and if so if it is compatible with the PHP native return type.
  -> This is the second potential point of failure as it will require co-variance determination to be added to PHPCompatibility (or rather PHPCSUtils).
• If there is no return type, check if there is a `#[ReturnTypeWillChange] attribute.
• If not, throw a warning.

Does that sound right to you ?

[aik099]

Sounds right. Thank you.

Looking at RFC I wasn't able to figure out what internal classes/interfaces were affected by it. Probably sniff just needs to look if the extended class/implemented interface is internal in general and then do array_intersect on its methods and methods of a class to get the scope to process.

[jrfnl]

AFAIK it affects all internals where return types could be added, i.e. when the return type is not resource or a type which can't be expressed - very few of those left -.
In most cases, this doesn't affect userland code. Think: global functions, final or private class methods in PHP native classes.

It affects userland code where the methods are implemented in userland code (PHP native interfaces) or non-final, non-private methods are overloaded in a class extending a PHP native class.