From 46c1d3e1c8e56c19c80b5bcc7d92901418d9dad3 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 4 May 2025 18:01:46 +0200 Subject: [PATCH] Config: bug fix - "reports" could get set incorrectly when in CBF mode There are three CLI arguments related to the reporting: * `--report=[comma separated list]` * `--report-file=[path to file]` * `--report-=[path to file]` The `--report-file...` option is ignored when in CBF mode. Whether that is the correct behaviour or not, is outside the scope of the current bug fix. In the `Config::processLongArgument()` method, the `--report-file` option is handled _before_ the `--report-` option to ensure that `--report-file` is handled correctly, as the handling of the `--report-` argument is based on seeing the prefix `--report-`, which overlaps with `--report-file`. However, this logic was broken in CBF mode, as in that case, the `--report-file` CLI arg would still fall through to the `--report-` handling, which would break the setting of the `$reports`. Fixed now. Includes tests specifically for this issue. --- src/Config.php | 4 +- tests/Core/Config/ReportArgsTest.php | 64 ++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 tests/Core/Config/ReportArgsTest.php diff --git a/src/Config.php b/src/Config.php index df56ecf92d..22c75b7e28 100644 --- a/src/Config.php +++ b/src/Config.php @@ -1010,8 +1010,8 @@ public function processLongArgument($arg, $pos) } self::$overriddenDefaults['stdinPath'] = true; - } else if (PHP_CODESNIFFER_CBF === false && substr($arg, 0, 12) === 'report-file=') { - if (isset(self::$overriddenDefaults['reportFile']) === true) { + } else if (substr($arg, 0, 12) === 'report-file=') { + if (PHP_CODESNIFFER_CBF === true || isset(self::$overriddenDefaults['reportFile']) === true) { break; } diff --git a/tests/Core/Config/ReportArgsTest.php b/tests/Core/Config/ReportArgsTest.php new file mode 100644 index 0000000000..60b5a7c46e --- /dev/null +++ b/tests/Core/Config/ReportArgsTest.php @@ -0,0 +1,64 @@ +markTestSkipped('This test needs CS mode to run'); + } + + $config = new ConfigDouble(['--report-file='.__DIR__.'/report.txt']); + + $this->assertTrue(is_string($config->reportFile)); + $this->assertStringEndsWith('/report.txt', $config->reportFile); + $this->assertSame(['full' => null], $config->reports); + + }//end testReportFileDoesNotSetReportsCs() + + + /** + * [CBF mode] Verify that passing `--report-file` does not influence *which* reports get activated. + * + * @group CBF + * + * @return void + */ + public function testReportFileDoesNotSetReportsCbf() + { + if (PHP_CODESNIFFER_CBF === false) { + $this->markTestSkipped('This test needs CBF mode to run'); + } + + $config = new ConfigDouble(['--report-file='.__DIR__.'/report.txt']); + + $this->assertNull($config->reportFile); + $this->assertSame(['full' => null], $config->reports); + + }//end testReportFileDoesNotSetReportsCbf() + + +}//end class