Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.1: prevent deprecation notices about missing return types #3400

Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 23, 2021

A plain run against PHP 8.1.0-beta1 currently results in the following deprecation notices:

Deprecated: Return type of PHP_CodeSniffer\Files\FileList::current() should either be compatible with Iterator::current(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 186
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::next() should either be compatible with Iterator::next(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 217
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::key() should either be compatible with Iterator::key(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 204
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::valid() should either be compatible with Iterator::valid(): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 230
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::rewind() should either be compatible with Iterator::rewind(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 173
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::count() should either be compatible with Countable::count(): int, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 247
Deprecated: Return type of PHP_CodeSniffer\Filters\Filter::getChildren() should either be compatible with RecursiveFilterIterator::getChildren(): ?RecursiveFilterIterator, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Filters/Filter.php on line 135
Deprecated: Return type of PHP_CodeSniffer\Filters\Filter::accept() should either be compatible with FilterIterator::accept(): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in path/to/PHP_CodeSniffer/src/Filters/Filter.php on line 93

The run will still succeed.

These deprecation notices relate to the Return types for internal methods RFC in PHP 8.1 and in particular, the change made in PHP PR #7239, which adds return types to the various SPL Iterator interface methods.

Basically, as of PHP 8.1, these methods in classes which implement the Iterator interface are expected to have a return type declared.
The return type can be the same as used in PHP itself or a more specific type. This complies with the Liskov principle of covariance, which allows the return type of a child overloaded method to be more specific than that of the parent.

The problem with this is that return types were only introduced in PHP 7.0 and therefore cannot be used as PHP_CodeSniffer 3.x has a minimum PHP version of 5.4.

Luckily an attribute has been added to silence the deprecation warning.

While attributes are a PHP 8.0+ feature, due to the choice of the #[] syntax, in PHP < 8.0, attributes will just be ignored and treated as comments, so there is no drawback to using the attribute.

Note: fixing these issues by adding the attributes exposed problems with attributes in the various Commenting sniffs. Fixes for those have been pulled separately.

@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 23, 2021

Note: due to the issues in the commenting sniffs, the builds for this PR will fail on the CS check not being able to handle the attributes correctly.

PR #3396, #3397 and PR #3398 should be merged first. Once those are merged, this PR can be rebased and the builds will pass (well, all except for PHP 8.1, but that's a different matter - see #3395 )

A plain run against PHP 8.1.0-beta1 currently results in the following deprecation notices:
```
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::current() should either be compatible with Iterator::current(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 186
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::next() should either be compatible with Iterator::next(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 217
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::key() should either be compatible with Iterator::key(): mixed, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 204
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::valid() should either be compatible with Iterator::valid(): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 230
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::rewind() should either be compatible with Iterator::rewind(): void, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 173
Deprecated: Return type of PHP_CodeSniffer\Files\FileList::count() should either be compatible with Countable::count(): int, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Files/FileList.php on line 247
Deprecated: Return type of PHP_CodeSniffer\Filters\Filter::getChildren() should either be compatible with RecursiveFilterIterator::getChildren(): ?RecursiveFilterIterator, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in in path/to/PHP_CodeSniffer/src/Filters/Filter.php on line 135
Deprecated: Return type of PHP_CodeSniffer\Filters\Filter::accept() should either be compatible with FilterIterator::accept(): bool, or the #[ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in path/to/PHP_CodeSniffer/src/Filters/Filter.php on line 93
```

The run will still succeed.

These deprecation notices relate to the [Return types for internal methods RFC](https://wiki.php.net/rfc/internal_method_return_types) in PHP 8.1 and in particular, the change made in PHP PR #7239, which adds return types to the various SPL Iterator interface methods.

Basically, as of PHP 8.1, these methods in classes which implement the Iterator interface are expected to have a return type declared.
The return type can be the same as used in PHP itself or a more specific type. This complies with the Liskov principle of covariance, which allows the return type of a child overloaded method to be more specific than that of the parent.

The problem with this is that return types were only introduced in PHP 7.0 and therefore cannot be used as PHP_CodeSniffer 3.x has a minimum PHP version of 5.4.

Luckily an attribute has been added to silence the deprecation warning.

While attributes are a PHP 8.0+ feature, due to the choice of the `#[]` syntax, in PHP < 8.0, attributes will just be ignored and treated as comments, so there is no drawback to using the attribute.
@jrfnl jrfnl force-pushed the php81/prevent-returntype-will-change-notices branch from 7d5f38b to be8fe72 Compare July 29, 2021 08:55
@jrfnl
Copy link
Contributor Author

jrfnl commented Jul 29, 2021

Rebased now the attribute/comment PRs have been merged.

@jrfnl
Copy link
Contributor Author

jrfnl commented Aug 16, 2021

Note: the current failures still on PHP 8.1 are unrelated to this PR and the build against this PR shows that the slew of deprecation notices at the start of a run is now gone, which is exactly what this PR is about.

@gsherwood gsherwood added this to the 3.6.1 milestone Aug 17, 2021
gsherwood added a commit that referenced this pull request Aug 26, 2021
@gsherwood gsherwood merged commit 915a694 into squizlabs:master Aug 26, 2021
@gsherwood
Copy link
Member

Thanks for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants