Skip to content

Refactor the requirements check #971

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

Merged
merged 1 commit into from
Apr 14, 2025

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Apr 14, 2025

Description

The requirements check (minimum PHP version + extensions) needs to stay PHP cross-version compatible to allow it to work properly.

As things were, what with the autoloader being loaded before the requirements check was run and the requirements check being part of the Runner class, this would block any PHP modernizations from being made to these files.

It is my opinion that the requirements check:

  1. Should be run before anything else.
  2. Should involve as few files as possible to prevent the check blocking code modernizations and raising the cognitive load for contributors.

To this end, I'm introducing a new requirements.php file in the project root. This file will be loaded in the bootstrapping of PHPCS/PHPCBF, both when running via the plain files (Composer, git clone), as well as when running via the PHAR file.

This means there will now be only three files which need to stay PHP cross-version compatible:

  • bin/phpcs
  • bin/phpcbf
  • requirements.php

A very explicit warning about the need for these files to stay cross-version compatible has been added to in the file docblock of each of these files.

The PHP minimum for these three files has been set to PHP 5.3 for the following reasons:

  1. The minimum PHP version for PHPCS 3.x was PHP 5.4, so chances of people trying to run PHPCS on a PHP version older than PHP 5.4 are slim.
  2. The requirements check in PHPCS 3.x was already broken due to the use of a) namespace separators in the bin/* files (PHP 5.3+) and b) short arrays in the actual requirements check code (PHP 5.4+).
  3. Testing - and therefore safeguarding - the functionality of the requirements check on PHP versions older than PHP 5.3 is not possible as PHP < 5.3 is not supported by setup-php.

Alongside the changes to the requirement checking, this commit also introduces a new GH Actions workflow to safeguard the requirements check for the future (and two helper scripts for use in the workflow).
This workflow will only run selectively when files involved in the requirements check, and the testing thereof, are changed.

Note: the exit code has not been changed (yet). This will be handled separately when addressing issue #184.

Suggested changelog entry

N/A

Related issues/external references

Fixes #530

@jrfnl jrfnl added this to the 4.0.0 milestone Apr 14, 2025
@jrfnl jrfnl changed the base branch from phpcs-4.0/feature/ghactions-enable-ci to phpcs-4.0/feature/drop-support-php-lt-7.2 April 14, 2025 11:32
@jrfnl jrfnl force-pushed the phpcs-4.0/feature/530-refactor-requirements-check branch from 744da95 to ff711f8 Compare April 14, 2025 11:32
@jrfnl jrfnl force-pushed the phpcs-4.0/feature/drop-support-php-lt-7.2 branch from 8de604b to a7a27b9 Compare April 14, 2025 11:53
Base automatically changed from phpcs-4.0/feature/drop-support-php-lt-7.2 to 4.x April 14, 2025 12:04
The requirements check (minimum PHP version + extensions) needs to stay PHP cross-version compatible to allow it to work properly.

As things were, what with the autoloader being loaded before the requirements check was run and the requirements check being part of the `Runner` class, this would block any PHP modernizations from being made to these files.

It is my opinion that the requirements check:
1. Should be run before anything else.
2. Should involve as few files as possible to prevent the check blocking code modernizations and raising the cognitive load for contributors.

To this end, I'm introducing a new `requirements.php` file in the project root. This file will be loaded in the bootstrapping of PHPCS/PHPCBF, both when running via the plain files (Composer, git clone), as well as when running via the PHAR file.

This means there will now be only three files which need to stay PHP cross-version compatible:
* `bin/phpcs`
* `bin/phpcbf`
* `requirements.php`

A very explicit warning about the need for these files to stay cross-version compatible has been added to in the file docblock of each of these files.

The PHP minimum for these three files has been set to PHP 5.3 for the following reasons:
1. The minimum PHP version for PHPCS 3.x was PHP 5.4, so chances of people trying to run PHPCS on a PHP version older than PHP 5.4 are slim.
2. The requirements check in PHPCS 3.x was already broken due to the use of a) namespace separators in the `bin/*` files (PHP 5.3+) and b) short arrays in the actual requirements check code (PHP 5.4+).
3. Testing - and therefore safeguarding - the functionality of the requirements check on PHP versions older than PHP 5.3 is not possible as PHP < 5.3 is [not supported by setup-php](https://github.com/shivammathur/setup-php#tada-php-support).

Alongside the changes to the requirement checking, this commit also introduces a new GH Actions workflow to safeguard the requirements check for the future (and two helper scripts for use in the workflow).
This workflow will only run selectively when files involved in the requirements check, and the testing thereof, are changed.

Note: the exit code has not been changed (yet). This will be handled separately when addressing issue 184.

Fixes 530
@jrfnl jrfnl force-pushed the phpcs-4.0/feature/530-refactor-requirements-check branch from ff711f8 to 1972495 Compare April 14, 2025 12:05
@jrfnl jrfnl marked this pull request as ready for review April 14, 2025 12:05
@jrfnl jrfnl merged commit 89f142f into 4.x Apr 14, 2025
157 checks passed
@jrfnl jrfnl deleted the phpcs-4.0/feature/530-refactor-requirements-check branch April 14, 2025 12:16
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