diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ba7a345b1f..8917590929 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -65,6 +65,15 @@ jobs: - php: '8.4' skip_tests: true + # Run a couple of builds with custom extensions to allow for testing ini handling within PHPCS. + # Ref: https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/416 + - php: '7.3' + os: 'ubuntu-latest' + extensions: ':mysqli' # Run with mysqli disabled. + - php: '8.2' + os: 'ubuntu-latest' + extensions: ':mysqli' # Run with mysqli disabled. + # The default libxml library on Ubuntu images is a little out of date. # To safeguard support for the latest libxml we need to update the library on the fly. # This only needs to be tested with one PHP version for each libxml minor to verify support. @@ -183,6 +192,7 @@ jobs: with: php-version: ${{ matrix.php }} ini-values: ${{ steps.set_ini.outputs.PHP_INI }} + extensions: ${{ matrix.extensions }} coverage: none tools: cs2pr @@ -269,10 +279,12 @@ jobs: custom_ini: [false] include: - # Also run one coverage build with custom ini settings. + # Also run one coverage build with custom ini settings for testing the DisallowShortOpenTag sniff. + # Also run with a disabled extension for testing the handling of unsettable ini settings by PHPCS. - php: '8.1' os: 'ubuntu-latest' custom_ini: true + extensions: ':mysqli' # Run with mysqli disabled. # yamllint disable-line rule:line-length name: "Coverage: ${{ matrix.php }} ${{ matrix.custom_ini && ' with custom ini settings' || '' }} (${{ matrix.os == 'ubuntu-latest' && 'Linux' || 'Win' }})" @@ -297,6 +309,7 @@ jobs: with: php-version: ${{ matrix.php }} ini-values: error_reporting=-1, display_errors=On${{ steps.set_ini.outputs.PHP_INI }} + extensions: ${{ matrix.extensions }} coverage: xdebug # Install dependencies and handle caching in one go. diff --git a/src/Config.php b/src/Config.php index 1c8d8dbdd1..c0494dbabc 100644 --- a/src/Config.php +++ b/src/Config.php @@ -739,10 +739,23 @@ public function processShortArgument($arg, $pos) case 'd' : $ini = explode('=', $this->cliArgs[($pos + 1)]); $this->cliArgs[($pos + 1)] = ''; - if (isset($ini[1]) === true) { - ini_set($ini[0], $ini[1]); - } else { - ini_set($ini[0], true); + if (isset($ini[1]) === false) { + // Set to true. + $ini[1] = '1'; + } + + $current = ini_get($ini[0]); + if ($current === false) { + // Ini setting which doesn't exist, or is from an unavailable extension. + // Silently ignore it. + break; + } + + $changed = ini_set($ini[0], $ini[1]); + if ($changed === false && ini_get($ini[0]) !== $ini[1]) { + $error = sprintf('ERROR: Ini option "%s" cannot be changed at runtime.', $ini[0]).PHP_EOL; + $error .= $this->printShortUsage(true); + throw new DeepExitException($error, 3); } break; case 'n' : diff --git a/tests/Core/Config/IniSetTest.php b/tests/Core/Config/IniSetTest.php new file mode 100644 index 0000000000..d9977f6d3a --- /dev/null +++ b/tests/Core/Config/IniSetTest.php @@ -0,0 +1,328 @@ +originalValue) === true) { + @ini_set($this->currentOption, $this->originalValue); + } + + }//end tearDown() + + + /** + * Verify that when an ini option is passed on the command-line and the value is the current value of the option + * and can be set at runtime, PHPCS does not throw an exception. + * + * @return void + */ + public function testIniValueHandlingWhenValueIsAlreadyCorrect() + { + $this->currentOption = 'precision'; + $this->originalValue = ini_get($this->currentOption); + + $this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist"); + + new ConfigDouble(['-d', "{$this->currentOption}={$this->originalValue}"]); + + $this->assertSame($this->originalValue, ini_get($this->currentOption)); + + }//end testIniValueHandlingWhenValueIsAlreadyCorrect() + + + /** + * 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. + * + * @requires extension mbstring + * + * @return void + */ + public function testIniValueIsUpdatedToTrueWhenNoValuePassed() + { + $this->currentOption = 'precision'; + // Set the expectation as the string equivalent to "true" as ini_get() will return a string value. + $expected = '1'; + + $this->originalValue = ini_get($this->currentOption); + + if ($this->originalValue === $expected) { + $this->markTestSkipped( + 'Skipping as original ini value on the system on which the test is run, is the same as the intended "new" value.' + ); + } + + $this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist"); + + new ConfigDouble(['-d', $this->currentOption]); + + $this->assertSame($expected, ini_get($this->currentOption)); + + }//end testIniValueIsUpdatedToTrueWhenNoValuePassed() + + + /** + * Test that when an ini value for a Core directive is passed on the command-line and can be set at runtime, + * PHPCS sets it correctly. + * + * @param string $option The name of the ini option. + * @param string $newValue The value to set the ini option to. + * @param string $alternativeValue Alternative value if the newValue would happen to coincide with the original value. + * + * @dataProvider dataIniValueIsUpdated + * + * @return void + */ + public function testIniValueIsUpdated($option, $newValue, $alternativeValue) + { + $this->currentOption = $option; + $this->originalValue = ini_get($option); + + if ($this->originalValue === $newValue) { + $newValue = $alternativeValue; + } + + $this->assertNotFalse($this->originalValue, "Test is broken: the $option ini directive does not exist"); + + new ConfigDouble(['-d', "$option=$newValue"]); + + $this->assertSame($newValue, ini_get($option)); + + }//end testIniValueIsUpdated() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataIniValueIsUpdated() + { + return [ + // Core directive, default value is 14. + 'INI_ALL option: precision' => [ + 'option' => 'precision', + 'newValue' => '10', + 'alternativeValue' => '20', + ], + ]; + + }//end dataIniValueIsUpdated() + + + /** + * Test that when an INI_ALL value for an optional extension is passed on the command-line and can be set at runtime, + * PHPCS sets it correctly. + * + * This is tested in a separate method as the BCMath extension may not be available (which would cause the test to fail). + * + * @requires extension bcmath + * + * @return void + */ + public function testIniValueIsUpdatedWhenOptionalBcmathExtensionIsAvailable() + { + // Default value for the bcmath.scale ini setting is 0. + $this->currentOption = 'bcmath.scale'; + $newValue = '10'; + + $this->originalValue = ini_get($this->currentOption); + if ($this->originalValue === $newValue) { + $newValue = '20'; + } + + $this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist"); + + new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]); + + $this->assertSame($newValue, ini_get($this->currentOption)); + + }//end testIniValueIsUpdatedWhenOptionalBcmathExtensionIsAvailable() + + + /** + * Test that when an ini value is passed on the command-line and can be set at runtime, PHPCS sets it correctly. + * + * There are only two `INI_USER` options in PHP. The first `tidy.clean_output` cannot be used for this test + * as PHPUnit will send headers before this test runs. + * So `sqlite3.defensive` is the only option we can test with, but this option was an INI_SYSTEM setting + * prior to PHP 8.2, so we can only test it on PHP 8.2 and higher. + * + * It's also unfortunate that it is a boolean option, which makes distinguising "set to false" and + * "not set" difficult. + * + * @requires PHP 8.2 + * @requires extension sqlite3 + * + * @return void + */ + public function testIniValueIsUpdatedWhenOptionalSqllite3ExtensionIsAvailable() + { + // Default value for the sqlite3.defensive ini setting is 1. + $this->currentOption = 'sqlite3.defensive'; + $newValue = '0'; + + $this->originalValue = ini_get($this->currentOption); + if ($this->originalValue === $newValue) { + $newValue = '1'; + } + + $this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist"); + + new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]); + + $this->assertSame($newValue, ini_get($this->currentOption)); + + }//end testIniValueIsUpdatedWhenOptionalSqllite3ExtensionIsAvailable() + + + /** + * Test that when an ini value is for a disabled extension, PHPCS will silently ignore the ini setting. + * + * @return void + */ + public function testIniValueIsSilentlyIgnoredWhenOptionalExtensionIsNotAvailable() + { + if (extension_loaded('mysqli') === true) { + $this->markTestSkipped( + 'Skipping as this test needs the MySqli extension to *not* be available.' + ); + } + + $this->currentOption = 'mysqli.default_port'; + $newValue = '1234'; + + $this->originalValue = ini_get($this->currentOption); + $this->assertFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive exists"); + + new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]); + + $this->assertFalse(ini_get($this->currentOption), 'This should be impossible: an option for a disabled extension cannot be set'); + + }//end testIniValueIsSilentlyIgnoredWhenOptionalExtensionIsNotAvailable() + + + /** + * Test that when an ini value is not known to PHP, PHPCS will silently ignore the ini setting. + * + * @return void + */ + public function testIniValueIsSilentlyIgnoredForUnknownIniName() + { + $this->currentOption = 'some.ini_which_doesnt_exist'; + $newValue = '1234'; + + $this->originalValue = ini_get($this->currentOption); + $this->assertFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive exists"); + + new ConfigDouble(['-d', "{$this->currentOption}=$newValue"]); + + $this->assertFalse(ini_get($this->currentOption), 'This should be impossible: an option which isn\'t known to PHP cannot be set'); + + }//end testIniValueIsSilentlyIgnoredForUnknownIniName() + + + /** + * Test that when an ini value is passed on the command-line and can NOT be set at runtime, PHPCS reports this. + * + * @param string $option The name of the ini option. + * @param string $newValue The value to set the ini option to. + * @param string $alternativeValue Alternative value in case the ini option is currently set to the $newValue. + * + * @dataProvider dataIniValueCannotBeUpdatedAtRuntime + * + * @return void + */ + public function testIniValueCannotBeUpdatedAtRuntime($option, $newValue, $alternativeValue='') + { + $this->expectException(DeepExitException::class); + $this->expectExceptionMessage("ERROR: Ini option \"$option\" cannot be changed at runtime."); + + $this->currentOption = $option; + $this->originalValue = ini_get($option); + $this->assertNotFalse($this->originalValue, "Test is broken: the $option ini directive does not exist"); + + if ($this->originalValue === $newValue) { + $newValue = $alternativeValue; + } + + new ConfigDouble(['-d', "$option=$newValue"]); + + }//end testIniValueCannotBeUpdatedAtRuntime() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataIniValueCannotBeUpdatedAtRuntime() + { + return [ + // Using Core directives available PHP cross-version to prevent the tests failing + // on an unavailable directive or due to an extension not being available. + 'php.ini only option: disable_classes' => [ + 'option' => 'disable_classes', + 'newValue' => 'DateTime,DOMComment', + 'alternativeValue' => 'DOMComment,DateTime', + ], + 'INI_PERDIR option: short_open_tag (bool)' => [ + 'option' => 'short_open_tag', + 'newValue' => '1', + 'alternativeValue' => '0', + ], + 'INI_PERDIR option: max_input_vars (int)' => [ + 'option' => 'max_input_vars', + 'newValue' => '345', + 'alternativeValue' => '543', + ], + 'INI_SYSTEM option: realpath_cache_ttl' => [ + 'option' => 'realpath_cache_ttl', + 'newValue' => '150', + 'alternativeValue' => '300', + ], + ]; + + }//end dataIniValueCannotBeUpdatedAtRuntime() + + +}//end class diff --git a/tests/Core/Ruleset/IniSetFailIniOnlyTest.xml b/tests/Core/Ruleset/IniSetFailIniOnlyTest.xml new file mode 100644 index 0000000000..73476fdfb3 --- /dev/null +++ b/tests/Core/Ruleset/IniSetFailIniOnlyTest.xml @@ -0,0 +1,9 @@ + + + Ruleset to test ini values which can not be changed at runtime will be reported as such when set from the ruleset. + + + + + + diff --git a/tests/Core/Ruleset/IniSetFailIniPerDirTest.xml b/tests/Core/Ruleset/IniSetFailIniPerDirTest.xml new file mode 100644 index 0000000000..87b9eecaf9 --- /dev/null +++ b/tests/Core/Ruleset/IniSetFailIniPerDirTest.xml @@ -0,0 +1,9 @@ + + + Ruleset to test ini values which can not be changed at runtime will be reported as such when set from the ruleset. + + + + + + diff --git a/tests/Core/Ruleset/IniSetFailIniSystemTest.xml b/tests/Core/Ruleset/IniSetFailIniSystemTest.xml new file mode 100644 index 0000000000..292cbf7bf1 --- /dev/null +++ b/tests/Core/Ruleset/IniSetFailIniSystemTest.xml @@ -0,0 +1,9 @@ + + + Ruleset to test ini values which can not be changed at runtime will be reported as such when set from the ruleset. + + + + + + diff --git a/tests/Core/Ruleset/IniSetSuccessTest.xml b/tests/Core/Ruleset/IniSetSuccessTest.xml new file mode 100644 index 0000000000..81284e1ea2 --- /dev/null +++ b/tests/Core/Ruleset/IniSetSuccessTest.xml @@ -0,0 +1,9 @@ + + + Ruleset to test ini values can be changed from the ruleset. + + + + + + diff --git a/tests/Core/Ruleset/ProcessRulesetIniSetTest.php b/tests/Core/Ruleset/ProcessRulesetIniSetTest.php new file mode 100644 index 0000000000..5b1b4cb1fa --- /dev/null +++ b/tests/Core/Ruleset/ProcessRulesetIniSetTest.php @@ -0,0 +1,139 @@ +originalValue) === true) { + @ini_set($this->currentOption, $this->originalValue); + } + + }//end tearDown() + + + /** + * Test that when an ini value is set in a custom ruleset and can be set at runtime, PHPCS sets it correctly. + * + * @return void + */ + public function testIniValueIsUpdated() + { + $this->currentOption = 'precision'; + $expected = '10'; + + $this->originalValue = ini_get($this->currentOption); + $this->assertNotFalse($this->originalValue, "Test is broken: the {$this->currentOption} ini directive does not exist"); + + new Ruleset(new ConfigDouble(['.', '--standard='.__DIR__.'/IniSetSuccessTest.xml'])); + + $this->assertSame($expected, ini_get($this->currentOption)); + + }//end testIniValueIsUpdated() + + + /** + * Test that when an ini value is set in a custom ruleset and can NOT be set at runtime, PHPCS reports this. + * + * @param string $standard The standard to use for the test. + * @param string $option The name of the ini option. + * @param mixed $expected The value to expect the ini option to be set to if it could have been changed. + * + * @dataProvider dataIniValueCannotBeUpdatedAtRuntime + * + * @return void + */ + public function testIniValueCannotBeUpdatedAtRuntime($standard, $option, $expected) + { + $this->currentOption = $option; + $this->originalValue = ini_get($option); + + if ($this->originalValue === $expected) { + $this->markTestSkipped( + 'Skipping as original ini value on the system on which the test is run, is the same as the intended "new" value.' + ); + } + + $this->assertNotFalse($this->originalValue, "Test is broken: the $option ini directive does not exist"); + + $this->expectException(DeepExitException::class); + $this->expectExceptionMessage("ERROR: Ini option \"$option\" cannot be changed at runtime."); + + new Ruleset(new ConfigDouble(["--standard=$standard"])); + + // Make sure the value didn't get set. + $this->assertNotSame($expected, ini_get($option), 'Setting the ini value should not have worked, the test is broken'); + + }//end testIniValueCannotBeUpdatedAtRuntime() + + + /** + * Data provider. + * + * @return array> + */ + public static function dataIniValueCannotBeUpdatedAtRuntime() + { + return [ + // Using Core directives available PHP cross-version to prevent the tests failing + // on an unavailable directive or due to an extension not being available. + 'php.ini only option: disable_classes' => [ + 'standard' => __DIR__.'/IniSetFailIniOnlyTest.xml', + 'option' => 'disable_classes', + 'expected' => 'DateTime,DOMComment', + ], + 'INI_PERDIR option: short_open_tag' => [ + 'standard' => __DIR__.'/IniSetFailIniPerDirTest.xml', + 'option' => 'short_open_tag', + 'expected' => '1', + ], + 'INI_SYSTEM option: realpath_cache_ttl' => [ + 'standard' => __DIR__.'/IniSetFailIniSystemTest.xml', + 'option' => 'realpath_cache_ttl', + 'expected' => '200', + ], + ]; + + }//end dataIniValueCannotBeUpdatedAtRuntime() + + +}//end class