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

YoastCS: drop support PHP < 7.2 & modernize the codebase #324

Merged
merged 5 commits into from
Nov 4, 2023

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Nov 3, 2023

Drop support for PHP < 7.2

The majority of the Yoast codebases now have a minimum of PHP 7.2.

Additionally, the YoastCS checks should give the same results independently of the PHP version on which the CS check is being run, which means that generally speaking, the CS check is run on a high PHP version, both locally as well as in CI.

With this in mind, there is no reason to keep the YoastCS minimum supported PHP version at PHP 5.4, so let's bring it in line with the other Yoast codebases and let it have a PHP 7.2 minimum.

Note: PHP_CodeSniffer itself still has a PHP 5.4 minimum. PHPCS 4.x is expected to have a PHP 7.2 minimum and this change is in line with that.

YoastCS/Modernize: add explicit visibility to all OO constants

YoastCS/Modernize: use constant arrays

Prior to PHP 5.6 class constants couldn't contain arrays. As support for PHP < 7.2 has now been dropped, unchangable properties containing arrays should be changed to constants.

YoastCS/Modernize: selectively use null coalescing

PHP 7.0 introduced the null coalesce operator. This commit implements the use of the operator in select places.

YoastCS/Modernize - Tests: add PHP native return type declarations

As support for PHP < 7.2 has now been dropped, PHP native parameter and return type declarations can be used.

There are, however, a couple of caveats:

  1. Return type declarations can be freely used as long as the type needed is available in PHP 7.2.
  2. Parameter type declarations can not be freely used as parameter types are contravariant, which means they can only be wider than the original type, which means parameter type declarations can only be added for YoastCS native methods and not for methods which implement a PHPCS native interface method or overload a PHPCS native sniff method, as in that case, the original method will rarely have a type declaration, which means that adding one in YoastCS would violate LSP.

Also note that a TypeError thrown by a sniff, would stop the scan of a file dead, so let's be conservative with adding type declarations in the sniffs for now, but add them freely to the tests.

jrfnl added 5 commits November 4, 2023 00:38
The majority of the Yoast codebases now have a minimum of PHP 7.2.

Additionally, the YoastCS checks should give the same results independently of the PHP version on which the CS check is being run, which means that generally speaking, the CS check is run on a high PHP version, both locally as well as in CI.

With this in mind, there is no reason to keep the YoastCS minimum supported PHP version at PHP 5.4, so let's bring it in line with the other Yoast codebases and let it have a PHP 7.2 minimum.

Note: PHP_CodeSniffer itself still has a PHP 5.4 minimum. PHPCS 4.x is expected to have a PHP 7.2 minimum and this change is in line with that.
Prior to PHP 5.6 class constants couldn't contain arrays. As support for PHP < 7.2 has now been dropped, unchangable properties containing arrays should be changed to constants.
PHP 7.0 introduced the null coalesce operator. This commit implements the use of the operator in select places.
As support for PHP < 7.2 has now been dropped, PHP native parameter and return type declarations can be used.

There are, however, a couple of caveats:
1. Return type declarations can be freely used as long as the type needed is available in PHP 7.2.
2. Parameter type declarations can **_not_** be freely used as parameter types are contravariant, which means they can only be _wider_ than the original type, which means parameter type declarations can only be added for YoastCS native methods and not for methods which implement a PHPCS native interface method or overload a PHPCS native sniff method, as in that case, the original method will rarely have a type declaration, which means that adding one in YoastCS would violate LSP.

Also note that a `TypeError` thrown by a sniff, would stop the scan of a file dead, so let's be conservative with adding type declarations in the sniffs for now, but add them freely to the tests.
@coveralls
Copy link

Coverage Status

coverage: 97.146% (-0.5%) from 97.644%
when pulling c4f44a9 on JRF/yoastcs-drop-support-php-lt-7.2
into 1acd027 on develop.

@jrfnl jrfnl merged commit da20b9b into develop Nov 4, 2023
25 checks passed
@jrfnl jrfnl deleted the JRF/yoastcs-drop-support-php-lt-7.2 branch November 4, 2023 00:10
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.

2 participants