Skip to content

Commit 744da95

Browse files
jrfnljrfnl
committed
Refactor the requirements check
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
1 parent 842c7f2 commit 744da95

File tree

10 files changed

+533
-53
lines changed

10 files changed

+533
-53
lines changed

‎.github/workflows/test-requirements-check.yml‎

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
name: Test requirements check
2+
3+
on:
4+
# Run on pushes to the main branches and on pull requests which touch files related to the requirements check.
5+
# No need to run this workflow when there are only irrelevant changes.
6+
push:
7+
branches:
8+
- 4.x
9+
tags:
10+
- '**'
11+
paths:
12+
- '.github/workflows/test-requirements-check.yml'
13+
- '.github/workflows/reusable-build-phar.yml'
14+
- 'bin/phpcs'
15+
- 'bin/phpcbf'
16+
- 'phpcs.xml.dist'
17+
- 'requirements.php'
18+
- 'scripts/**'
19+
pull_request:
20+
paths:
21+
- '.github/workflows/test-requirements-check.yml'
22+
- '.github/workflows/reusable-build-phar.yml'
23+
- 'bin/phpcs'
24+
- 'bin/phpcbf'
25+
- 'phpcs.xml.dist'
26+
- 'requirements.php'
27+
- 'scripts/**'
28+
29+
# Allow manually triggering the workflow.
30+
workflow_dispatch:
31+
32+
# Cancels all previous workflow runs for the same branch that have not yet completed.
33+
concurrency:
34+
# The concurrency group contains the workflow name and the branch name.
35+
group: ${{ github.workflow }}-${{ github.ref }}
36+
cancel-in-progress: true
37+
38+
jobs:
39+
# Make sure that the files involved in the requirements check don't contain parse errors
40+
# for the PHP versions supported by the requirements check to prevent the tests being run
41+
# failing on the parse errors instead of on the requirements check
42+
# (which would easily go unnoticed).
43+
lint:
44+
runs-on: ubuntu-latest
45+
46+
strategy:
47+
matrix:
48+
php: ['5.3', '5.4', '5.5', '5.6', '7.0', '7.1']
49+
50+
name: "Lint: PHP ${{ matrix.php }}"
51+
52+
steps:
53+
- name: Checkout code
54+
uses: actions/checkout@v4
55+
56+
- name: Install PHP
57+
uses: shivammathur/setup-php@v2
58+
with:
59+
php-version: ${{ matrix.php }}
60+
coverage: none
61+
62+
- name: "Lint bin/phpcs"
63+
run: php -l ./bin/phpcs
64+
65+
- name: "Lint bin/phpcbf"
66+
run: php -l ./bin/phpcbf
67+
68+
- name: "Lint requirements.php"
69+
run: php -l ./requirements.php
70+
71+
# The matrix for these tests should be the same for the "plain" file test as for the PHAR test.
72+
# This matrix, however, is quite complex, making maintaining it manually pretty error prone.
73+
# So this job uses a PHP script to generate the matrix based on a fixed set of variables.
74+
#
75+
# The resulting matrix contains builds which combine the following variables:
76+
# - os: ubuntu / windows
77+
# - cmd: phpcs / phpcbf
78+
# - php: 7.2 (minimum PHP version for 4.x), latest and nightly with the required extensions (should pass the check).
79+
# - php: 5.3 (minimum for the requirements check) and 7.1 with the required extension (should fail the PHP version check).
80+
# - php: 7.2 (minimum PHP version for 4.x), latest and nightly WITHOUT required extensions (should fail the check for extensions).
81+
#
82+
# Each combination also contains a "expect" key to set the expectations for success / failure.
83+
#
84+
# The scripts involved in generating the matrix can be found in the `/scripts/` directory.
85+
prepare-matrix:
86+
needs: lint
87+
88+
name: Get test matrix
89+
runs-on: ubuntu-latest
90+
91+
outputs:
92+
matrix: ${{ steps.set-matrix.outputs.MATRIX }}
93+
94+
steps:
95+
- name: Checkout code
96+
uses: actions/checkout@v4
97+
98+
- name: Install PHP
99+
uses: shivammathur/setup-php@v2
100+
with:
101+
php-version: 'latest'
102+
ini-values: 'error_reporting=-1, display_errors=On'
103+
coverage: none
104+
105+
- name: Set matrix
106+
id: set-matrix
107+
run: echo "MATRIX=$(php scripts/get-requirements-check-matrix.php)" >> "$GITHUB_OUTPUT"
108+
109+
- name: "DEBUG: show generated matrix"
110+
run: echo ${{ steps.set-matrix.outputs.MATRIX }}
111+
112+
# Test that the requirements check works correctly when using a Composer install or git clone of PHPCS.
113+
test-plain:
114+
needs: prepare-matrix
115+
116+
strategy:
117+
fail-fast: false
118+
matrix: ${{ fromJson(needs.prepare-matrix.outputs.MATRIX) }}
119+
120+
# yamllint disable-line rule:line-length
121+
name: "Plain: ${{ matrix.cmd == 'phpcs' && 'cs' || 'cbf' }} / ${{ matrix.php }} / ${{ matrix.name }} (${{ matrix.os == 'ubuntu-latest' && 'nix' || 'Win' }})"
122+
123+
continue-on-error: ${{ matrix.php == 'nightly' }}
124+
125+
runs-on: ${{ matrix.os }}
126+
127+
steps:
128+
- name: Checkout code
129+
uses: actions/checkout@v4
130+
131+
- name: Install PHP
132+
uses: shivammathur/setup-php@v2
133+
with:
134+
php-version: ${{ matrix.php }}
135+
ini-values: 'error_reporting=-1, display_errors=On'
136+
extensions: ${{ matrix.extensions }}
137+
coverage: none
138+
env:
139+
fail-fast: true
140+
141+
# We're only testing the requirements check here, so we just need to verify that PHPCS/PHPCBF starts. Nothing more.
142+
- name: Run the test
143+
id: check
144+
continue-on-error: true
145+
run: php "bin/${{ matrix.cmd }}" --version
146+
147+
- name: Check the result of a successful test against expectation
148+
if: ${{ steps.check.outcome == 'success' && matrix.expect == 'fail' }}
149+
run: exit 1
150+
151+
- name: Check the result of a failed test against expectation
152+
if: ${{ steps.check.outcome != 'success' && matrix.expect == 'success' }}
153+
run: exit 1
154+
155+
build-phars:
156+
needs: lint
157+
158+
name: "Build Phar on PHP 8.0"
159+
160+
uses: ./.github/workflows/reusable-build-phar.yml
161+
with:
162+
uploadArtifacts: true
163+
164+
# Test that the requirements check works correctly when using a PHPCS/PHPCBF PHAR file.
165+
test-phar:
166+
needs: [prepare-matrix, build-phars]
167+
168+
strategy:
169+
fail-fast: false
170+
matrix: ${{ fromJson(needs.prepare-matrix.outputs.MATRIX) }}
171+
172+
# yamllint disable-line rule:line-length
173+
name: "PHAR: ${{ matrix.cmd == 'phpcs' && 'cs' || 'cbf' }} / ${{ matrix.php }} / ${{ matrix.name }} (${{ matrix.os == 'ubuntu-latest' && 'nix' || 'Win' }})"
174+
175+
continue-on-error: ${{ matrix.php == 'nightly' }}
176+
177+
runs-on: ${{ matrix.os }}
178+
179+
steps:
180+
- name: Checkout code
181+
uses: actions/checkout@v4
182+
183+
- name: Install PHP
184+
uses: shivammathur/setup-php@v2
185+
with:
186+
php-version: ${{ matrix.php }}
187+
ini-values: 'error_reporting=-1, display_errors=On'
188+
extensions: ${{ matrix.extensions }}
189+
coverage: none
190+
env:
191+
fail-fast: true
192+
193+
- name: Download the phar
194+
uses: actions/download-artifact@v4
195+
with:
196+
name: ${{ matrix.cmd }}-phar
197+
198+
# We're only testing the requirements check here, so we just need to verify that PHPCS/PHPCBF starts. Nothing more.
199+
- name: Run the test
200+
id: check
201+
continue-on-error: true
202+
run: php ${{ matrix.cmd }}.phar --version
203+
204+
- name: Check the result of a successful test against expectation
205+
if: ${{ steps.check.outcome == 'success' && matrix.expect == 'fail' }}
206+
run: exit 1
207+
208+
- name: Check the result of a failed test against expectation
209+
if: ${{ steps.check.outcome != 'success' && matrix.expect == 'success' }}
210+
run: exit 1

‎bin/phpcbf‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,28 @@
33
/**
44
* PHP Code Beautifier and Fixer fixes violations of a defined coding standard.
55
*
6+
* :WARNING:
7+
* This file MUST stay cross-version compatible with older PHP versions (min: PHP 5.3) to allow
8+
* for the requirements check to work correctly.
9+
*
10+
* The PHP 5.3 minimum is set as the previous PHPCS major (3.x) already had a PHP 5.4 minimum
11+
* requirement and didn't take parse errors caused due to the use of namespaces into account
12+
* in its requirements check, so running PHPCS 3.x on PHP < 5.3 would have failed with a parse
13+
* error already anyway, so PHP 5.3 seems reasonable to keep as the minimum for this.
14+
* :WARNING:
15+
*
616
* @author Greg Sherwood <[email protected]>
17+
* @author Juliette Reinders Folmer <[email protected]>
718
* @copyright 2006-2015 Squiz Pty Ltd (ABN 77 084 670 600)
19+
* @copyright 2024 PHPCSStandards and contributors
820
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
921
*/
1022

11-
require_once __DIR__.'/../autoload.php';
23+
// Check if the PHP version and extensions comply with the minimum requirements before anything else.
24+
require_once dirname(__DIR__).'/requirements.php';
25+
PHP_CodeSniffer\checkRequirements();
26+
27+
require_once dirname(__DIR__).'/autoload.php';
1228

1329
$runner = new PHP_CodeSniffer\Runner();
1430
$exitCode = $runner->runPHPCBF();

‎bin/phpcs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,28 @@
33
/**
44
* PHP_CodeSniffer detects violations of a defined coding standard.
55
*
6+
* :WARNING:
7+
* This file MUST stay cross-version compatible with older PHP versions (min: PHP 5.3) to allow
8+
* for the requirements check to work correctly.
9+
*
10+
* The PHP 5.3 minimum is set as the previous PHPCS major (3.x) already had a PHP 5.4 minimum
11+
* requirement and didn't take parse errors caused due to the use of namespaces into account
12+
* in its requirements check, so running PHPCS 3.x on PHP < 5.3 would have failed with a parse
13+
* error already anyway, so PHP 5.3 seems reasonable to keep as the minimum for this.
14+
* :WARNING:
15+
*
616
* @author Greg Sherwood <[email protected]>
17+
* @author Juliette Reinders Folmer <[email protected]>
718
* @copyright 2006-2015 Squiz Pty Ltd (ABN 77 084 670 600)
19+
* @copyright 2024 PHPCSStandards and contributors
820
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
921
*/
1022

11-
require_once __DIR__.'/../autoload.php';
23+
// Check if the PHP version and extensions comply with the minimum requirements before anything else.
24+
require_once dirname(__DIR__).'/requirements.php';
25+
PHP_CodeSniffer\checkRequirements();
26+
27+
require_once dirname(__DIR__).'/autoload.php';
1228

1329
$runner = new PHP_CodeSniffer\Runner();
1430
$exitCode = $runner->runPHPCS();

‎phpcs.xml.dist‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
<description>The coding standard for PHP_CodeSniffer itself.</description>
44

55
<file>autoload.php</file>
6+
<file>requirements.php</file>
67
<file>bin</file>
78
<file>scripts</file>
89
<file>src</file>
@@ -58,7 +59,11 @@
5859
<rule ref="Squiz.WhiteSpace.MemberVarSpacing"/>
5960
<rule ref="Squiz.WhiteSpace.OperatorSpacing"/>
6061
<rule ref="Squiz.WhiteSpace.SuperfluousWhitespace"/>
61-
<rule ref="Generic.Arrays.DisallowLongArraySyntax"/>
62+
<rule ref="Generic.Arrays.DisallowLongArraySyntax">
63+
<!-- Files involved in the requirements check should stay compatible with PHP 5.3. -->
64+
<exclude-pattern>bin/phpc(s|bf)$</exclude-pattern>
65+
<exclude-pattern>requirements\.php$</exclude-pattern>
66+
</rule>
6267
<rule ref="Generic.Commenting.Todo"/>
6368
<rule ref="Generic.ControlStructures.DisallowYodaConditions"/>
6469
<rule ref="Generic.ControlStructures.InlineControlStructure"/>

‎phpstan.neon.dist‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ parameters:
22
phpVersion: 70100 # Should be 50400, but PHPStan only accepts 70100 or higher...
33
level: 0
44
paths:
5+
- requirements.php
56
- src
67
bootstrapFiles:
78
- tests/bootstrap.php

‎requirements.php‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
<?php
2+
/**
3+
* Requirements check for PHP_CodeSniffer.
4+
*
5+
* :WARNING:
6+
* This file MUST stay cross-version compatible with older PHP versions (min: PHP 5.3) to allow
7+
* for the requirements check to work correctly.
8+
*
9+
* The PHP 5.3 minimum is set as the previous PHPCS major (3.x) already had a PHP 5.4 minimum
10+
* requirement and didn't take parse errors caused due to the use of namespaces into account
11+
* in its requirements check, so running PHPCS 3.x on PHP < 5.3 would have failed with a parse
12+
* error already anyway, so PHP 5.3 seems reasonable to keep as the minimum for this.
13+
* :WARNING:
14+
*
15+
* @author Greg Sherwood <[email protected]>
16+
* @author Juliette Reinders Folmer <[email protected]>
17+
* @copyright 2006-2015 Squiz Pty Ltd (ABN 77 084 670 600)
18+
* @copyright 2024 PHPCSStandards and contributors
19+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
20+
*/
21+
22+
namespace PHP_CodeSniffer;
23+
24+
25+
/**
26+
* Exits if the minimum requirements of PHP_CodeSniffer are not met.
27+
*
28+
* @return void
29+
*/
30+
function checkRequirements()
31+
{
32+
$exitCode = 3;
33+
34+
// Check the PHP version.
35+
if (PHP_VERSION_ID < 70200) {
36+
echo 'ERROR: PHP_CodeSniffer requires PHP version 7.2.0 or greater.'.PHP_EOL;
37+
exit($exitCode);
38+
}
39+
40+
$requiredExtensions = array(
41+
'tokenizer',
42+
'xmlwriter',
43+
'SimpleXML',
44+
);
45+
$missingExtensions = array();
46+
47+
foreach ($requiredExtensions as $extension) {
48+
if (extension_loaded($extension) === false) {
49+
$missingExtensions[] = $extension;
50+
}
51+
}
52+
53+
if (empty($missingExtensions) === false) {
54+
$last = array_pop($requiredExtensions);
55+
$required = implode(', ', $requiredExtensions);
56+
$required .= ' and '.$last;
57+
58+
if (count($missingExtensions) === 1) {
59+
$missing = $missingExtensions[0];
60+
} else {
61+
$last = array_pop($missingExtensions);
62+
$missing = implode(', ', $missingExtensions);
63+
$missing .= ' and '.$last;
64+
}
65+
66+
$error = 'ERROR: PHP_CodeSniffer requires the %s extensions to be enabled. Please enable %s.'.PHP_EOL;
67+
printf($error, $required, $missing);
68+
exit($exitCode);
69+
}
70+
71+
}//end checkRequirements()

0 commit comments

Comments
 (0)