Skip to content

Commit 04f4286

Browse files
authored
Merge pull request #1024 from PHPCSStandards/phpcs-4.0/feature/416-improve-ini-set-error-handling
Config: show user friendly error message when ini change failed
2 parents 8ac3cad + 09a96d9 commit 04f4286

File tree

8 files changed

+534
-5
lines changed

8 files changed

+534
-5
lines changed

.github/workflows/test.yml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,15 @@ jobs:
6565
- php: '8.4'
6666
skip_tests: true
6767

68+
# Run a couple of builds with custom extensions to allow for testing ini handling within PHPCS.
69+
# Ref: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/416
70+
- php: '7.3'
71+
os: 'ubuntu-latest'
72+
extensions: ':mysqli' # Run with mysqli disabled.
73+
- php: '8.2'
74+
os: 'ubuntu-latest'
75+
extensions: ':mysqli' # Run with mysqli disabled.
76+
6877
# The default libxml library on Ubuntu images is a little out of date.
6978
# To safeguard support for the latest libxml we need to update the library on the fly.
7079
# This only needs to be tested with one PHP version for each libxml minor to verify support.
@@ -183,6 +192,7 @@ jobs:
183192
with:
184193
php-version: ${{ matrix.php }}
185194
ini-values: ${{ steps.set_ini.outputs.PHP_INI }}
195+
extensions: ${{ matrix.extensions }}
186196
coverage: none
187197
tools: cs2pr
188198

@@ -269,10 +279,12 @@ jobs:
269279
custom_ini: [false]
270280

271281
include:
272-
# Also run one coverage build with custom ini settings.
282+
# Also run one coverage build with custom ini settings for testing the DisallowShortOpenTag sniff.
283+
# Also run with a disabled extension for testing the handling of unsettable ini settings by PHPCS.
273284
- php: '8.1'
274285
os: 'ubuntu-latest'
275286
custom_ini: true
287+
extensions: ':mysqli' # Run with mysqli disabled.
276288

277289
# yamllint disable-line rule:line-length
278290
name: "Coverage: ${{ matrix.php }} ${{ matrix.custom_ini && ' with custom ini settings' || '' }} (${{ matrix.os == 'ubuntu-latest' && 'Linux' || 'Win' }})"
@@ -297,6 +309,7 @@ jobs:
297309
with:
298310
php-version: ${{ matrix.php }}
299311
ini-values: error_reporting=-1, display_errors=On${{ steps.set_ini.outputs.PHP_INI }}
312+
extensions: ${{ matrix.extensions }}
300313
coverage: xdebug
301314

302315
# Install dependencies and handle caching in one go.

src/Config.php

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -739,10 +739,23 @@ public function processShortArgument($arg, $pos)
739739
case 'd' :
740740
$ini = explode('=', $this->cliArgs[($pos + 1)]);
741741
$this->cliArgs[($pos + 1)] = '';
742-
if (isset($ini[1]) === true) {
743-
ini_set($ini[0], $ini[1]);
744-
} else {
745-
ini_set($ini[0], true);
742+
if (isset($ini[1]) === false) {
743+
// Set to true.
744+
$ini[1] = '1';
745+
}
746+
747+
$current = ini_get($ini[0]);
748+
if ($current === false) {
749+
// Ini setting which doesn't exist, or is from an unavailable extension.
750+
// Silently ignore it.
751+
break;
752+
}
753+
754+
$changed = ini_set($ini[0], $ini[1]);
755+
if ($changed === false && ini_get($ini[0]) !== $ini[1]) {
756+
$error = sprintf('ERROR: Ini option "%s" cannot be changed at runtime.', $ini[0]).PHP_EOL;
757+
$error .= $this->printShortUsage(true);
758+
throw new DeepExitException($error, 3);
746759
}
747760
break;
748761
case 'n' :

0 commit comments

Comments
 (0)