Skip to content

Commit 7f91c5c

Browse files
committed
Config: show user friendly error message when ini change failed
Inspired by discussion 415. Previously, when a PHP ini option which cannot be changed at runtime was passed to PHPCS, it would be silently ignored (by PHP, PHPCS would still try to handle it, but would not report that PHP did not change the value). This commit changes that behaviour by adding a new "ERROR: Ini option %s cannot be set at runtime" error to alert the end-user to the fact that they are passing a PHP ini option which PHPCS cannot change. The new error will be thrown both when the user passes the invalid ini setting via the command line, as well as when it is passed via a custom ruleset. The behaviour when trying to change an ini setting which _doesn't exist_ (typo, extension not available) is unchanged. In that case, the ini directive will still be silently ignored. Includes unit tests to safeguard the new behaviour. Closes 416 Also note: when this error occurs due to an invalid setting being passed via a ruleset, the error will be thrown directly and not collected via the `MessageCollector`. This is due to the error coming from the `Config` class. Once the `MessageCollector` would be implemented in the `Config` class, this can potentially be changed.
1 parent 8ac3cad commit 7f91c5c

File tree

8 files changed

+514
-5
lines changed

8 files changed

+514
-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' :

tests/Core/Config/IniSetTest.php

Lines changed: 306 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,306 @@
1+
<?php
2+
/**
3+
* Tests for overriding the value of a PHP ini setting using CLI arguments.
4+
*
5+
* @copyright 2025 PHPCSStandards Contributors
6+
* @license https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/master/licence.txt BSD Licence
7+
*/
8+
9+
namespace PHP_CodeSniffer\Tests\Core\Config;
10+
11+
use PHP_CodeSniffer\Exceptions\DeepExitException;
12+
use PHP_CodeSniffer\Tests\ConfigDouble;
13+
use PHPUnit\Framework\TestCase;
14+
15+
/**
16+
* Tests for overriding the value of a PHP ini setting using CLI arguments.
17+
*
18+
* @covers \PHP_CodeSniffer\Config::processShortArgument
19+
*/
20+
final class IniSetTest extends TestCase
21+
{
22+
23+
/**
24+
* The name of the current ini setting under test.
25+
*
26+
* @var string
27+
*/
28+
private $currentOption;
29+
30+
/**
31+
* Store the original value of the ini setting under test, so the system can be restored to its
32+
* original state in the tearDown() method.
33+
*
34+
* @var string|false
35+
*/
36+
private $originalValue;
37+
38+
39+
/**
40+
* Reset the ini value which was potentially changed via the test.
41+
*
42+
* @return void
43+
*/
44+
protected function tearDown(): void
45+
{
46+
if (is_string($this->originalValue) === true) {
47+
@ini_set($this->currentOption, $this->originalValue);
48+
}
49+
50+
}//end tearDown()
51+
52+
53+
/**
54+
* Verify that when an ini option is passed on the command-line and the value is the current value of the option
55+
* and can be set at runtime, PHPCS does not throw an exception.
56+
*
57+
* @return void
58+
*/
59+
public function testIniValueHandlingWhenValueIsAlreadyCorrect()
60+
{
61+
$this->currentOption = 'precision';
62+
$this->originalValue = ini_get($this->currentOption);
63+
64+
$this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist");
65+
66+
new ConfigDouble(['-d', "{$this->currentOption}={$this->originalValue}"]);
67+
68+
$this->assertSame($this->originalValue, ini_get($this->currentOption));
69+
70+
}//end testIniValueHandlingWhenValueIsAlreadyCorrect()
71+
72+
73+
/**
74+
* Verify that when an ini option is passed on the command-line without a value and can be set at runtime, PHPCS sets it correctly.
75+
*
76+
* @requires extension mbstring
77+
*
78+
* @return void
79+
*/
80+
public function testIniValueIsUpdatedToTrueWhenNoValuePassed()
81+
{
82+
$this->currentOption = 'precision';
83+
// Set the expectation as the string equivalent to "true" as ini_get() will return a string value.
84+
$expected = '1';
85+
86+
$this->originalValue = ini_get($this->currentOption);
87+
$this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist");
88+
89+
new ConfigDouble(['-d', $this->currentOption]);
90+
91+
$this->assertSame($expected, ini_get($this->currentOption));
92+
93+
}//end testIniValueIsUpdatedToTrueWhenNoValuePassed()
94+
95+
96+
/**
97+
* Test that when an ini value for a Core directive is passed on the command-line and can be set at runtime,
98+
* PHPCS sets it correctly.
99+
*
100+
* @param string $option The name of the ini option.
101+
* @param string $newValue The value to set the ini option to.
102+
*
103+
* @dataProvider dataIniValueIsUpdated
104+
*
105+
* @return void
106+
*/
107+
public function testIniValueIsUpdated($option, $newValue)
108+
{
109+
$this->currentOption = $option;
110+
$this->originalValue = ini_get($option);
111+
112+
$this->assertNotFalse($this->originalValue, "Test is broken: the $option ini directive does not exist");
113+
114+
new ConfigDouble(['-d', "$option=$newValue"]);
115+
116+
$this->assertSame($newValue, ini_get($option));
117+
118+
}//end testIniValueIsUpdated()
119+
120+
121+
/**
122+
* Data provider.
123+
*
124+
* @return array<string, array<string, string>>
125+
*/
126+
public static function dataIniValueIsUpdated()
127+
{
128+
return [
129+
// Core directive, default value is 14.
130+
'INI_ALL option: precision' => [
131+
'option' => 'precision',
132+
'newValue' => '10',
133+
],
134+
];
135+
136+
}//end dataIniValueIsUpdated()
137+
138+
139+
/**
140+
* Test that when an INI_ALL value for an optional extension is passed on the command-line and can be set at runtime,
141+
* PHPCS sets it correctly.
142+
*
143+
* This is tested in a separate method as the BCMath extension may not be available (which would cause the test to fail).
144+
*
145+
* @requires extension bcmath
146+
*
147+
* @return void
148+
*/
149+
public function testIniValueIsUpdatedWhenOptionalBcmathExtensionIsAvailable()
150+
{
151+
// Default value for the bcmath.scale ini setting is 0.
152+
$this->currentOption = 'bcmath.scale';
153+
$newValue = '10';
154+
155+
$this->originalValue = ini_get($this->currentOption);
156+
$this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist");
157+
158+
new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]);
159+
160+
$this->assertSame($newValue, ini_get($this->currentOption));
161+
162+
}//end testIniValueIsUpdatedWhenOptionalBcmathExtensionIsAvailable()
163+
164+
165+
/**
166+
* Test that when an ini value is passed on the command-line and can be set at runtime, PHPCS sets it correctly.
167+
*
168+
* There are only two `INI_USER` options in PHP. The first `tidy.clean_output` cannot be used for this test
169+
* as PHPUnit will send headers before this test runs.
170+
* So `sqlite3.defensive` is the only option we can test with, but this option was an INI_SYSTEM setting
171+
* prior to PHP 8.2, so we can only test it on PHP 8.2 and higher.
172+
*
173+
* It's also unfortunate that it is a boolean option, which makes distinguising "set to false" and
174+
* "not set" difficult.
175+
*
176+
* @requires PHP 8.2
177+
* @requires extension sqlite3
178+
*
179+
* @return void
180+
*/
181+
public function testIniValueIsUpdatedWhenOptionalSqllite3ExtensionIsAvailable()
182+
{
183+
// Default value for the sqlite3.defensive ini setting is 1.
184+
$this->currentOption = 'sqlite3.defensive';
185+
$newValue = '0';
186+
187+
$this->originalValue = ini_get($this->currentOption);
188+
$this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist");
189+
190+
new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]);
191+
192+
$this->assertSame($newValue, ini_get($this->currentOption));
193+
194+
}//end testIniValueIsUpdatedWhenOptionalSqllite3ExtensionIsAvailable()
195+
196+
197+
/**
198+
* Test that when an ini value is for a disabled extension, PHPCS will silently ignore the ini setting.
199+
*
200+
* @return void
201+
*/
202+
public function testIniValueIsSilentlyIgnoredWhenOptionalExtensionIsNotAvailable()
203+
{
204+
if (extension_loaded('mysqli') === true) {
205+
$this->markTestSkipped(
206+
'Skipping as this test needs the MySqli extension to *not* be available.'
207+
);
208+
}
209+
210+
$this->currentOption = 'mysqli.default_port';
211+
$newValue = '1234';
212+
213+
$this->originalValue = ini_get($this->currentOption);
214+
$this->assertFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive exists");
215+
216+
new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]);
217+
218+
$this->assertFalse(ini_get($this->currentOption), 'This should be impossible: an option for a disabled extension cannot be set');
219+
220+
}//end testIniValueIsSilentlyIgnoredWhenOptionalExtensionIsNotAvailable()
221+
222+
223+
/**
224+
* Test that when an ini value is not known to PHP, PHPCS will silently ignore the ini setting.
225+
*
226+
* @return void
227+
*/
228+
public function testIniValueIsSilentlyIgnoredForUnknownIniName()
229+
{
230+
$this->currentOption = 'some.ini_which_doesnt_exist';
231+
$newValue = '1234';
232+
233+
$this->originalValue = ini_get($this->currentOption);
234+
$this->assertFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive exists");
235+
236+
new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]);
237+
238+
$this->assertFalse(ini_get($this->currentOption), 'This should be impossible: an option which isn\'t known to PHP cannot be set');
239+
240+
}//end testIniValueIsSilentlyIgnoredForUnknownIniName()
241+
242+
243+
/**
244+
* Test that when an ini value is passed on the command-line and can NOT be set at runtime, PHPCS reports this.
245+
*
246+
* @param string $option The name of the ini option.
247+
* @param string $newValue The value to set the ini option to.
248+
* @param string $alternativeValue Alternative value in case the ini option is currently set to the $newValue.
249+
*
250+
* @dataProvider dataIniValueCannotBeUpdatedAtRuntime
251+
*
252+
* @return void
253+
*/
254+
public function testIniValueCannotBeUpdatedAtRuntime($option, $newValue, $alternativeValue='')
255+
{
256+
$this->expectException(DeepExitException::class);
257+
$this->expectExceptionMessage("ERROR: Ini option \"$option\" cannot be changed at runtime.");
258+
259+
$this->currentOption = $option;
260+
$this->originalValue = ini_get($option);
261+
$this->assertNotFalse($this->originalValue, "Test is broken: the $option ini directive does not exist");
262+
263+
if ($this->originalValue === $newValue) {
264+
$newValue = $alternativeValue;
265+
}
266+
267+
new ConfigDouble(['-d', "$option=$newValue"]);
268+
269+
}//end testIniValueCannotBeUpdatedAtRuntime()
270+
271+
272+
/**
273+
* Data provider.
274+
*
275+
* @return array<string, array<string, string>>
276+
*/
277+
public static function dataIniValueCannotBeUpdatedAtRuntime()
278+
{
279+
return [
280+
// Using Core directives available PHP cross-version to prevent the tests failing
281+
// on an unavailable directive or due to an extension not being available.
282+
'php.ini only option: disable_classes' => [
283+
'option' => 'disable_classes',
284+
'newValue' => 'DateTime,DOMComment',
285+
],
286+
'INI_PERDIR option: short_open_tag (bool)' => [
287+
'option' => 'short_open_tag',
288+
'newValue' => '1',
289+
'alternativeValue' => '0',
290+
],
291+
'INI_PERDIR option: max_input_vars (int)' => [
292+
'option' => 'max_input_vars',
293+
'newValue' => '345',
294+
'alternativeValue' => '543',
295+
],
296+
'INI_SYSTEM option: realpath_cache_ttl' => [
297+
'option' => 'realpath_cache_ttl',
298+
'newValue' => '150',
299+
'alternativeValue' => '300',
300+
],
301+
];
302+
303+
}//end dataIniValueCannotBeUpdatedAtRuntime()
304+
305+
306+
}//end class
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?xml version="1.0"?>
2+
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="IniSetTest" xsi:noNamespaceSchemaLocation="phpcs.xsd">
3+
<description>Ruleset to test ini values which can not be changed at runtime will be reported as such when set from the ruleset.</description>
4+
5+
<ini name="disable_classes" value="DateTime,DOMComment"/>
6+
7+
<!-- Prevent a "no sniff were registered" error. -->
8+
<rule ref="Generic.PHP.BacktickOperator"/>
9+
</ruleset>

0 commit comments

Comments
 (0)