From cca5977c64adcccc3905ef79148c95c4f9cedb8d Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Sun, 2 Mar 2025 17:58:23 +0100 Subject: [PATCH 01/10] Add --ignore-new-errors analyse command option --- .../Ignore/IgnoredErrorHelperResult.php | 53 +++++++++++++------ src/Command/AnalyseApplication.php | 4 +- src/Command/AnalyseCommand.php | 9 ++++ src/Command/CommandHelper.php | 2 - src/Command/FixerWorkerCommand.php | 11 +++- src/DependencyInjection/ContainerFactory.php | 2 - src/DependencyInjection/LoaderFactory.php | 5 +- src/DependencyInjection/NeonLoader.php | 35 ------------ tests/PHPStan/Analyser/AnalyserTest.php | 32 ++++++++++- .../AnalyseApplicationIntegrationTest.php | 2 + 10 files changed, 94 insertions(+), 61 deletions(-) delete mode 100644 src/DependencyInjection/NeonLoader.php diff --git a/src/Analyser/Ignore/IgnoredErrorHelperResult.php b/src/Analyser/Ignore/IgnoredErrorHelperResult.php index 529e1ae4a4..4abf05656f 100644 --- a/src/Analyser/Ignore/IgnoredErrorHelperResult.php +++ b/src/Analyser/Ignore/IgnoredErrorHelperResult.php @@ -50,12 +50,15 @@ public function process( bool $onlyFiles, array $analysedFiles, bool $hasInternalErrors, + bool $generateBaseline, + bool $ignoreNewErrors, ): IgnoredErrorHelperProcessedResult { - $unmatchedIgnoredErrors = $this->ignoreErrors; + // if we are generating the baseline, we dont want ignore Errors configuration to affect the errors we output + $unmatchedIgnoredErrors = $generateBaseline ? [] : $this->ignoreErrors; $stringErrors = []; - $processIgnoreError = function (Error $error, int $i, $ignore) use (&$unmatchedIgnoredErrors, &$stringErrors): bool { + $processIgnoreError = function (Error $error, int $i, $ignore) use ($generateBaseline, &$unmatchedIgnoredErrors, &$stringErrors): bool { $shouldBeIgnored = false; if (is_string($ignore)) { $shouldBeIgnored = IgnoredError::shouldIgnore($this->fileHelper, $error, $ignore, null, null); @@ -65,7 +68,8 @@ public function process( } else { if (isset($ignore['path'])) { $shouldBeIgnored = IgnoredError::shouldIgnore($this->fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, $ignore['path']); - if ($shouldBeIgnored) { + // only ignore errors when not generating the baseline, because it need to contain all errors + if ($shouldBeIgnored && !$generateBaseline) { if (isset($ignore['count'])) { $realCount = $unmatchedIgnoredErrors[$i]['realCount'] ?? 0; $realCount++; @@ -115,12 +119,12 @@ public function process( 'Error message "%s" cannot be ignored, use excludePaths instead.', $error->getMessage(), ); - return true; + return false; } - return false; + return true; } - return true; + return false; }; $ignoredErrors = []; @@ -130,12 +134,16 @@ public function process( foreach ($this->ignoreErrorsByFile[$filePath] as $ignoreError) { $i = $ignoreError['index']; $ignore = $ignoreError['ignoreError']; - $result = $processIgnoreError($error, $i, $ignore); - if (!$result) { + $isIgnoreMatch = $processIgnoreError($error, $i, $ignore); + if (!$isIgnoreMatch) { + continue; + } + + if (!$generateBaseline) { unset($errors[$errorIndex]); $ignoredErrors[] = [$error, $ignore]; - continue 2; } + continue 2; } } @@ -146,12 +154,16 @@ public function process( foreach ($this->ignoreErrorsByFile[$normalizedTraitFilePath] as $ignoreError) { $i = $ignoreError['index']; $ignore = $ignoreError['ignoreError']; - $result = $processIgnoreError($error, $i, $ignore); - if (!$result) { + $isIgnoreMatch = $processIgnoreError($error, $i, $ignore); + if (!$isIgnoreMatch) { + continue; + } + + if (!$generateBaseline) { unset($errors[$errorIndex]); $ignoredErrors[] = [$error, $ignore]; - continue 2; } + continue 2; } } } @@ -160,13 +172,24 @@ public function process( $i = $ignoreError['index']; $ignore = $ignoreError['ignoreError']; - $result = $processIgnoreError($error, $i, $ignore); - if (!$result) { + $isIgnoreMatch = $processIgnoreError($error, $i, $ignore); + if (!$isIgnoreMatch) { + continue; + } + + if (!$generateBaseline) { unset($errors[$errorIndex]); $ignoredErrors[] = [$error, $ignore]; - continue 2; } + continue 2; } + + if (!$ignoreNewErrors) { + continue; + } + + // if the error was not ignored, it is a new error, don't return it when $ignoreNewErrors is set + unset($errors[$errorIndex]); } $errors = array_values($errors); diff --git a/src/Command/AnalyseApplication.php b/src/Command/AnalyseApplication.php index 81793c6ba0..88bf86b1a1 100644 --- a/src/Command/AnalyseApplication.php +++ b/src/Command/AnalyseApplication.php @@ -51,6 +51,8 @@ public function analyse( ?string $projectConfigFile, ?array $projectConfigArray, InputInterface $input, + bool $generateBaseline, + bool $ignoreNewErrors, ): AnalysisResult { $isResultCacheUsed = false; @@ -142,7 +144,7 @@ public function analyse( } } - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $files, $hasInternalErrors); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $files, $hasInternalErrors, $generateBaseline, $ignoreNewErrors); $fileSpecificErrors = $ignoredErrorHelperProcessedResult->getNotIgnoredErrors(); $notFileSpecificErrors = $ignoredErrorHelperProcessedResult->getOtherIgnoreMessages(); $collectedData = $analyserResult->getCollectedData(); diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index a81115e108..234a6a0853 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -102,6 +102,7 @@ protected function configure(): void new InputOption('watch', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('pro', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('fail-without-result-cache', null, InputOption::VALUE_NONE, 'Return non-zero exit code when result cache is not used'), + new InputOption('ignore-new-errors', null, InputOption::VALUE_NONE, 'Ignore new errors when generating the baseline.'), ]); } @@ -136,6 +137,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $debugEnabled = (bool) $input->getOption('debug'); $fix = (bool) $input->getOption('fix') || (bool) $input->getOption('watch') || (bool) $input->getOption('pro'); $failWithoutResultCache = (bool) $input->getOption('fail-without-result-cache'); + $ignoreNewErrors = (bool) $input->getOption('ignore-new-errors'); /** @var string|false|null $generateBaselineFile */ $generateBaselineFile = $input->getOption('generate-baseline'); @@ -182,6 +184,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); } + if ($generateBaselineFile === null && $ignoreNewErrors) { + $inceptionResult->getStdOutput()->getStyle()->error('You must pass the --generate-baseline option alongside --ignore-new-errors.'); + return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); + } + $errorOutput = $inceptionResult->getErrorOutput(); $errorFormat = $input->getOption('error-format'); @@ -290,6 +297,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $inceptionResult->getProjectConfigFile(), $inceptionResult->getProjectConfigArray(), $input, + $generateBaselineFile !== null, + $ignoreNewErrors, ); } catch (Throwable $t) { if ($debug) { diff --git a/src/Command/CommandHelper.php b/src/Command/CommandHelper.php index a142ecdca8..26dd89fb99 100644 --- a/src/Command/CommandHelper.php +++ b/src/Command/CommandHelper.php @@ -225,10 +225,8 @@ public static function begin( } $loader = (new LoaderFactory( - $currentWorkingDirectoryFileHelper, $containerFactory->getRootDirectory(), $containerFactory->getCurrentWorkingDirectory(), - $generateBaselineFile, ))->createLoader(); try { diff --git a/src/Command/FixerWorkerCommand.php b/src/Command/FixerWorkerCommand.php index 70a7624453..2ad988fbee 100644 --- a/src/Command/FixerWorkerCommand.php +++ b/src/Command/FixerWorkerCommand.php @@ -302,6 +302,8 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use $isOnlyFiles, $inceptionFiles, $hasInternalErrors, + false, + false, ); $ignoreFileErrors = []; foreach ($ignoredErrorHelperProcessedResult->getNotIgnoredErrors() as $error) { @@ -355,7 +357,14 @@ private function transformErrorIntoInternalError(Error $error): InternalError */ private function filterErrors(array $errors, IgnoredErrorHelperResult $ignoredErrorHelperResult, bool $onlyFiles, array $inceptionFiles, bool $hasInternalErrors): array { - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $inceptionFiles, $hasInternalErrors); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process( + $errors, + $onlyFiles, + $inceptionFiles, + $hasInternalErrors, + false, + false, + ); $finalErrors = []; foreach ($ignoredErrorHelperProcessedResult->getNotIgnoredErrors() as $error) { if ($error->getIdentifier() === null) { diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index c28e08a77c..e2d426e52d 100644 --- a/src/DependencyInjection/ContainerFactory.php +++ b/src/DependencyInjection/ContainerFactory.php @@ -114,10 +114,8 @@ public function create( ); $configurator = new Configurator(new LoaderFactory( - $this->fileHelper, $this->rootDirectory, $this->currentWorkingDirectory, - $generateBaselineFile, ), $this->journalContainer); $configurator->defaultExtensions = [ 'php' => PhpExtension::class, diff --git a/src/DependencyInjection/LoaderFactory.php b/src/DependencyInjection/LoaderFactory.php index 6053ca5828..742f20abcd 100644 --- a/src/DependencyInjection/LoaderFactory.php +++ b/src/DependencyInjection/LoaderFactory.php @@ -3,24 +3,21 @@ namespace PHPStan\DependencyInjection; use Nette\DI\Config\Loader; -use PHPStan\File\FileHelper; use function getenv; final class LoaderFactory { public function __construct( - private FileHelper $fileHelper, private string $rootDir, private string $currentWorkingDirectory, - private ?string $generateBaselineFile, ) { } public function createLoader(): Loader { - $loader = new NeonLoader($this->fileHelper, $this->generateBaselineFile); + $loader = new Loader(); $loader->addAdapter('dist', NeonAdapter::class); $loader->addAdapter('neon', NeonAdapter::class); $loader->setParameters([ diff --git a/src/DependencyInjection/NeonLoader.php b/src/DependencyInjection/NeonLoader.php deleted file mode 100644 index 010b80cfab..0000000000 --- a/src/DependencyInjection/NeonLoader.php +++ /dev/null @@ -1,35 +0,0 @@ -generateBaselineFile === null) { - return parent::load($file, $merge); - } - - $normalizedFile = $this->fileHelper->normalizePath($file); - if ($this->generateBaselineFile === $normalizedFile) { - return []; - } - - return parent::load($file, $merge); - } - -} diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index bacd85a967..8e31f8dc23 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -52,6 +52,27 @@ public function testReturnErrorIfIgnoredMessagesDoesNotOccur(): void ], $result); } + public function testDontReturnErrorIfIgnoredMessagesDoesNotOccurWhenGeneratingBaseline(): void + { + $result = $this->runAnalyser(['#Unknown error#'], false, __DIR__ . '/data/empty/empty.php', false, true); + + $this->assertEmpty($result); + } + + public function testDontReturnErrorOnNewErrorWhenIgnoringNewErrors(): void + { + $result = $this->runAnalyser(['#Unknown error#'], false, __DIR__ . '/data/bootstrap-error.php', false, true, true); + + $this->assertEmpty($result); + } + + public function testReturnErrorOnIgnoredErrorWhenIgnoringNewErrorsAndGeneratingBaseline(): void + { + $result = $this->runAnalyser(['#Fail\.#'], false, __DIR__ . '/data/bootstrap-error.php', false, true, true); + + $this->assertNotEmpty($result); + } + public function testDoNotReturnErrorIfIgnoredMessagesDoesNotOccurWithReportUnmatchedIgnoredErrorsOff(): void { $result = $this->runAnalyser(['#Unknown error#'], false, __DIR__ . '/data/empty/empty.php', false); @@ -644,6 +665,8 @@ private function runAnalyser( bool $reportUnmatchedIgnoredErrors, $filePaths, bool $onlyFiles, + bool $generateBaseline = false, + bool $ignoreNewErrors = false, ): array { $analyser = $this->createAnalyser(); @@ -679,7 +702,14 @@ private function runAnalyser( ); $analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult(); - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($analyserResult->getErrors(), $onlyFiles, $normalizedFilePaths, $analyserResult->hasReachedInternalErrorsCountLimit()); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process( + $analyserResult->getErrors(), + $onlyFiles, + $normalizedFilePaths, + $analyserResult->hasReachedInternalErrorsCountLimit(), + $generateBaseline, + $ignoreNewErrors, + ); $errors = $ignoredErrorHelperProcessedResult->getNotIgnoredErrors(); $errors = array_merge($errors, $ignoredErrorHelperProcessedResult->getOtherIgnoreMessages()); if ($analyserResult->hasReachedInternalErrorsCountLimit()) { diff --git a/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php b/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php index e84ac78e28..5504958eb8 100644 --- a/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php +++ b/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php @@ -85,6 +85,8 @@ private function runPath(string $path, int $expectedStatusCode): string null, null, $this->createMock(InputInterface::class), + false, + false, ); $statusCode = $errorFormatter->formatErrors($analysisResult, $symfonyOutput); From 4076f4af95743a973c1184d239a7a1ae15f2f24f Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Mon, 3 Mar 2025 21:13:15 +0100 Subject: [PATCH 02/10] Revert "Add --ignore-new-errors analyse command option" This reverts commit 389b6d1eb8456d9eb2e86929ff483cf792bab461. --- .../Ignore/IgnoredErrorHelperResult.php | 53 ++++++------------- src/Command/AnalyseApplication.php | 4 +- src/Command/AnalyseCommand.php | 9 ---- src/Command/CommandHelper.php | 2 + src/Command/FixerWorkerCommand.php | 11 +--- src/DependencyInjection/ContainerFactory.php | 2 + src/DependencyInjection/LoaderFactory.php | 5 +- src/DependencyInjection/NeonLoader.php | 35 ++++++++++++ tests/PHPStan/Analyser/AnalyserTest.php | 32 +---------- .../AnalyseApplicationIntegrationTest.php | 2 - 10 files changed, 61 insertions(+), 94 deletions(-) create mode 100644 src/DependencyInjection/NeonLoader.php diff --git a/src/Analyser/Ignore/IgnoredErrorHelperResult.php b/src/Analyser/Ignore/IgnoredErrorHelperResult.php index 4abf05656f..529e1ae4a4 100644 --- a/src/Analyser/Ignore/IgnoredErrorHelperResult.php +++ b/src/Analyser/Ignore/IgnoredErrorHelperResult.php @@ -50,15 +50,12 @@ public function process( bool $onlyFiles, array $analysedFiles, bool $hasInternalErrors, - bool $generateBaseline, - bool $ignoreNewErrors, ): IgnoredErrorHelperProcessedResult { - // if we are generating the baseline, we dont want ignore Errors configuration to affect the errors we output - $unmatchedIgnoredErrors = $generateBaseline ? [] : $this->ignoreErrors; + $unmatchedIgnoredErrors = $this->ignoreErrors; $stringErrors = []; - $processIgnoreError = function (Error $error, int $i, $ignore) use ($generateBaseline, &$unmatchedIgnoredErrors, &$stringErrors): bool { + $processIgnoreError = function (Error $error, int $i, $ignore) use (&$unmatchedIgnoredErrors, &$stringErrors): bool { $shouldBeIgnored = false; if (is_string($ignore)) { $shouldBeIgnored = IgnoredError::shouldIgnore($this->fileHelper, $error, $ignore, null, null); @@ -68,8 +65,7 @@ public function process( } else { if (isset($ignore['path'])) { $shouldBeIgnored = IgnoredError::shouldIgnore($this->fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, $ignore['path']); - // only ignore errors when not generating the baseline, because it need to contain all errors - if ($shouldBeIgnored && !$generateBaseline) { + if ($shouldBeIgnored) { if (isset($ignore['count'])) { $realCount = $unmatchedIgnoredErrors[$i]['realCount'] ?? 0; $realCount++; @@ -119,12 +115,12 @@ public function process( 'Error message "%s" cannot be ignored, use excludePaths instead.', $error->getMessage(), ); - return false; + return true; } - return true; + return false; } - return false; + return true; }; $ignoredErrors = []; @@ -134,16 +130,12 @@ public function process( foreach ($this->ignoreErrorsByFile[$filePath] as $ignoreError) { $i = $ignoreError['index']; $ignore = $ignoreError['ignoreError']; - $isIgnoreMatch = $processIgnoreError($error, $i, $ignore); - if (!$isIgnoreMatch) { - continue; - } - - if (!$generateBaseline) { + $result = $processIgnoreError($error, $i, $ignore); + if (!$result) { unset($errors[$errorIndex]); $ignoredErrors[] = [$error, $ignore]; + continue 2; } - continue 2; } } @@ -154,16 +146,12 @@ public function process( foreach ($this->ignoreErrorsByFile[$normalizedTraitFilePath] as $ignoreError) { $i = $ignoreError['index']; $ignore = $ignoreError['ignoreError']; - $isIgnoreMatch = $processIgnoreError($error, $i, $ignore); - if (!$isIgnoreMatch) { - continue; - } - - if (!$generateBaseline) { + $result = $processIgnoreError($error, $i, $ignore); + if (!$result) { unset($errors[$errorIndex]); $ignoredErrors[] = [$error, $ignore]; + continue 2; } - continue 2; } } } @@ -172,24 +160,13 @@ public function process( $i = $ignoreError['index']; $ignore = $ignoreError['ignoreError']; - $isIgnoreMatch = $processIgnoreError($error, $i, $ignore); - if (!$isIgnoreMatch) { - continue; - } - - if (!$generateBaseline) { + $result = $processIgnoreError($error, $i, $ignore); + if (!$result) { unset($errors[$errorIndex]); $ignoredErrors[] = [$error, $ignore]; + continue 2; } - continue 2; } - - if (!$ignoreNewErrors) { - continue; - } - - // if the error was not ignored, it is a new error, don't return it when $ignoreNewErrors is set - unset($errors[$errorIndex]); } $errors = array_values($errors); diff --git a/src/Command/AnalyseApplication.php b/src/Command/AnalyseApplication.php index 88bf86b1a1..81793c6ba0 100644 --- a/src/Command/AnalyseApplication.php +++ b/src/Command/AnalyseApplication.php @@ -51,8 +51,6 @@ public function analyse( ?string $projectConfigFile, ?array $projectConfigArray, InputInterface $input, - bool $generateBaseline, - bool $ignoreNewErrors, ): AnalysisResult { $isResultCacheUsed = false; @@ -144,7 +142,7 @@ public function analyse( } } - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $files, $hasInternalErrors, $generateBaseline, $ignoreNewErrors); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $files, $hasInternalErrors); $fileSpecificErrors = $ignoredErrorHelperProcessedResult->getNotIgnoredErrors(); $notFileSpecificErrors = $ignoredErrorHelperProcessedResult->getOtherIgnoreMessages(); $collectedData = $analyserResult->getCollectedData(); diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index 234a6a0853..a81115e108 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -102,7 +102,6 @@ protected function configure(): void new InputOption('watch', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('pro', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('fail-without-result-cache', null, InputOption::VALUE_NONE, 'Return non-zero exit code when result cache is not used'), - new InputOption('ignore-new-errors', null, InputOption::VALUE_NONE, 'Ignore new errors when generating the baseline.'), ]); } @@ -137,7 +136,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int $debugEnabled = (bool) $input->getOption('debug'); $fix = (bool) $input->getOption('fix') || (bool) $input->getOption('watch') || (bool) $input->getOption('pro'); $failWithoutResultCache = (bool) $input->getOption('fail-without-result-cache'); - $ignoreNewErrors = (bool) $input->getOption('ignore-new-errors'); /** @var string|false|null $generateBaselineFile */ $generateBaselineFile = $input->getOption('generate-baseline'); @@ -184,11 +182,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); } - if ($generateBaselineFile === null && $ignoreNewErrors) { - $inceptionResult->getStdOutput()->getStyle()->error('You must pass the --generate-baseline option alongside --ignore-new-errors.'); - return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); - } - $errorOutput = $inceptionResult->getErrorOutput(); $errorFormat = $input->getOption('error-format'); @@ -297,8 +290,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int $inceptionResult->getProjectConfigFile(), $inceptionResult->getProjectConfigArray(), $input, - $generateBaselineFile !== null, - $ignoreNewErrors, ); } catch (Throwable $t) { if ($debug) { diff --git a/src/Command/CommandHelper.php b/src/Command/CommandHelper.php index 26dd89fb99..a142ecdca8 100644 --- a/src/Command/CommandHelper.php +++ b/src/Command/CommandHelper.php @@ -225,8 +225,10 @@ public static function begin( } $loader = (new LoaderFactory( + $currentWorkingDirectoryFileHelper, $containerFactory->getRootDirectory(), $containerFactory->getCurrentWorkingDirectory(), + $generateBaselineFile, ))->createLoader(); try { diff --git a/src/Command/FixerWorkerCommand.php b/src/Command/FixerWorkerCommand.php index 2ad988fbee..70a7624453 100644 --- a/src/Command/FixerWorkerCommand.php +++ b/src/Command/FixerWorkerCommand.php @@ -302,8 +302,6 @@ function (array $errors, array $locallyIgnoredErrors, array $analysedFiles) use $isOnlyFiles, $inceptionFiles, $hasInternalErrors, - false, - false, ); $ignoreFileErrors = []; foreach ($ignoredErrorHelperProcessedResult->getNotIgnoredErrors() as $error) { @@ -357,14 +355,7 @@ private function transformErrorIntoInternalError(Error $error): InternalError */ private function filterErrors(array $errors, IgnoredErrorHelperResult $ignoredErrorHelperResult, bool $onlyFiles, array $inceptionFiles, bool $hasInternalErrors): array { - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process( - $errors, - $onlyFiles, - $inceptionFiles, - $hasInternalErrors, - false, - false, - ); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($errors, $onlyFiles, $inceptionFiles, $hasInternalErrors); $finalErrors = []; foreach ($ignoredErrorHelperProcessedResult->getNotIgnoredErrors() as $error) { if ($error->getIdentifier() === null) { diff --git a/src/DependencyInjection/ContainerFactory.php b/src/DependencyInjection/ContainerFactory.php index e2d426e52d..c28e08a77c 100644 --- a/src/DependencyInjection/ContainerFactory.php +++ b/src/DependencyInjection/ContainerFactory.php @@ -114,8 +114,10 @@ public function create( ); $configurator = new Configurator(new LoaderFactory( + $this->fileHelper, $this->rootDirectory, $this->currentWorkingDirectory, + $generateBaselineFile, ), $this->journalContainer); $configurator->defaultExtensions = [ 'php' => PhpExtension::class, diff --git a/src/DependencyInjection/LoaderFactory.php b/src/DependencyInjection/LoaderFactory.php index 742f20abcd..6053ca5828 100644 --- a/src/DependencyInjection/LoaderFactory.php +++ b/src/DependencyInjection/LoaderFactory.php @@ -3,21 +3,24 @@ namespace PHPStan\DependencyInjection; use Nette\DI\Config\Loader; +use PHPStan\File\FileHelper; use function getenv; final class LoaderFactory { public function __construct( + private FileHelper $fileHelper, private string $rootDir, private string $currentWorkingDirectory, + private ?string $generateBaselineFile, ) { } public function createLoader(): Loader { - $loader = new Loader(); + $loader = new NeonLoader($this->fileHelper, $this->generateBaselineFile); $loader->addAdapter('dist', NeonAdapter::class); $loader->addAdapter('neon', NeonAdapter::class); $loader->setParameters([ diff --git a/src/DependencyInjection/NeonLoader.php b/src/DependencyInjection/NeonLoader.php new file mode 100644 index 0000000000..010b80cfab --- /dev/null +++ b/src/DependencyInjection/NeonLoader.php @@ -0,0 +1,35 @@ +generateBaselineFile === null) { + return parent::load($file, $merge); + } + + $normalizedFile = $this->fileHelper->normalizePath($file); + if ($this->generateBaselineFile === $normalizedFile) { + return []; + } + + return parent::load($file, $merge); + } + +} diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 8e31f8dc23..bacd85a967 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -52,27 +52,6 @@ public function testReturnErrorIfIgnoredMessagesDoesNotOccur(): void ], $result); } - public function testDontReturnErrorIfIgnoredMessagesDoesNotOccurWhenGeneratingBaseline(): void - { - $result = $this->runAnalyser(['#Unknown error#'], false, __DIR__ . '/data/empty/empty.php', false, true); - - $this->assertEmpty($result); - } - - public function testDontReturnErrorOnNewErrorWhenIgnoringNewErrors(): void - { - $result = $this->runAnalyser(['#Unknown error#'], false, __DIR__ . '/data/bootstrap-error.php', false, true, true); - - $this->assertEmpty($result); - } - - public function testReturnErrorOnIgnoredErrorWhenIgnoringNewErrorsAndGeneratingBaseline(): void - { - $result = $this->runAnalyser(['#Fail\.#'], false, __DIR__ . '/data/bootstrap-error.php', false, true, true); - - $this->assertNotEmpty($result); - } - public function testDoNotReturnErrorIfIgnoredMessagesDoesNotOccurWithReportUnmatchedIgnoredErrorsOff(): void { $result = $this->runAnalyser(['#Unknown error#'], false, __DIR__ . '/data/empty/empty.php', false); @@ -665,8 +644,6 @@ private function runAnalyser( bool $reportUnmatchedIgnoredErrors, $filePaths, bool $onlyFiles, - bool $generateBaseline = false, - bool $ignoreNewErrors = false, ): array { $analyser = $this->createAnalyser(); @@ -702,14 +679,7 @@ private function runAnalyser( ); $analyserResult = $finalizer->finalize($analyserResult, $onlyFiles, false)->getAnalyserResult(); - $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process( - $analyserResult->getErrors(), - $onlyFiles, - $normalizedFilePaths, - $analyserResult->hasReachedInternalErrorsCountLimit(), - $generateBaseline, - $ignoreNewErrors, - ); + $ignoredErrorHelperProcessedResult = $ignoredErrorHelperResult->process($analyserResult->getErrors(), $onlyFiles, $normalizedFilePaths, $analyserResult->hasReachedInternalErrorsCountLimit()); $errors = $ignoredErrorHelperProcessedResult->getNotIgnoredErrors(); $errors = array_merge($errors, $ignoredErrorHelperProcessedResult->getOtherIgnoreMessages()); if ($analyserResult->hasReachedInternalErrorsCountLimit()) { diff --git a/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php b/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php index 5504958eb8..e84ac78e28 100644 --- a/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php +++ b/tests/PHPStan/Command/AnalyseApplicationIntegrationTest.php @@ -85,8 +85,6 @@ private function runPath(string $path, int $expectedStatusCode): string null, null, $this->createMock(InputInterface::class), - false, - false, ); $statusCode = $errorFormatter->formatErrors($analysisResult, $symfonyOutput); From 0f6eaebf96ffecde7876f8f1997c8494bdae1480 Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Tue, 11 Mar 2025 23:24:37 +0100 Subject: [PATCH 03/10] Add --ignore-new-errors option for baseline generation --- build/phpstan.neon | 1 + phpcs.xml | 1 + src/Command/AnalyseCommand.php | 183 +++++++++++++++++- tests/PHPStan/Command/AnalyseCommandTest.php | 77 +++++++- .../data-ignore-new-errors-baseline/A.php | 16 ++ .../empty.neon | 0 .../Command/data-ignore-new-errors/A.php | 23 +++ .../Command/data-ignore-new-errors/B.php | 16 ++ .../Command/data-ignore-new-errors/C.php | 16 ++ .../Command/data-ignore-new-errors/empty.neon | 0 10 files changed, 327 insertions(+), 6 deletions(-) create mode 100644 tests/PHPStan/Command/data-ignore-new-errors-baseline/A.php create mode 100644 tests/PHPStan/Command/data-ignore-new-errors-baseline/empty.neon create mode 100644 tests/PHPStan/Command/data-ignore-new-errors/A.php create mode 100644 tests/PHPStan/Command/data-ignore-new-errors/B.php create mode 100644 tests/PHPStan/Command/data-ignore-new-errors/C.php create mode 100644 tests/PHPStan/Command/data-ignore-new-errors/empty.neon diff --git a/build/phpstan.neon b/build/phpstan.neon index 1b1bf800f9..3fa881294d 100644 --- a/build/phpstan.neon +++ b/build/phpstan.neon @@ -24,6 +24,7 @@ parameters: checkMissingCallableSignature: true excludePaths: - ../tests/*/data/* + - ../tests/*/data-*/* - ../tests/tmp/* - ../tests/PHPStan/Analyser/nsrt/* - ../tests/PHPStan/Analyser/traits/* diff --git a/phpcs.xml b/phpcs.xml index fa9198745f..c9fc515426 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -209,6 +209,7 @@ tests/*/Fixture/ tests/*/cache/ tests/*/data/ + tests/*/data-*/ tests/*/traits/ tests/PHPStan/Analyser/nsrt/ tests/e2e/anon-class/ diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index a81115e108..1b01a2bb15 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -2,7 +2,11 @@ namespace PHPStan\Command; +use Nette\DI\Config\Loader; +use Nette\FileNotFoundException; +use Nette\InvalidStateException; use OndraM\CiDetector\CiDetector; +use PHPStan\Analyser\Ignore\IgnoredError; use PHPStan\Analyser\InternalError; use PHPStan\Command\ErrorFormatter\BaselineNeonErrorFormatter; use PHPStan\Command\ErrorFormatter\BaselinePhpErrorFormatter; @@ -102,6 +106,7 @@ protected function configure(): void new InputOption('watch', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('pro', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('fail-without-result-cache', null, InputOption::VALUE_NONE, 'Return non-zero exit code when result cache is not used'), + new InputOption('ignore-new-errors', null, InputOption::VALUE_NONE, 'Ignore new errors when generating the baseline.'), ]); } @@ -136,6 +141,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $debugEnabled = (bool) $input->getOption('debug'); $fix = (bool) $input->getOption('fix') || (bool) $input->getOption('watch') || (bool) $input->getOption('pro'); $failWithoutResultCache = (bool) $input->getOption('fail-without-result-cache'); + $ignoreNewErrors = (bool) $input->getOption('ignore-new-errors'); /** @var string|false|null $generateBaselineFile */ $generateBaselineFile = $input->getOption('generate-baseline'); @@ -182,6 +188,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); } + if ($generateBaselineFile === null && $ignoreNewErrors) { + $inceptionResult->getStdOutput()->getStyle()->error('You must pass the --generate-baseline option alongside --ignore-new-errors.'); + return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); + } + $errorOutput = $inceptionResult->getErrorOutput(); $errorFormat = $input->getOption('error-format'); @@ -411,7 +422,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $inceptionResult->handleReturn(1, $analysisResult->getPeakMemoryUsageBytes(), $this->analysisStartTime); } - return $this->generateBaseline($generateBaselineFile, $inceptionResult, $analysisResult, $output, $allowEmptyBaseline, $baselineExtension, $failWithoutResultCache); + return $this->generateBaseline($generateBaselineFile, $inceptionResult, $analysisResult, $output, $allowEmptyBaseline, $baselineExtension, $failWithoutResultCache, $ignoreNewErrors, $container); } /** @var ErrorFormatter $errorFormatter */ @@ -587,8 +598,13 @@ private function getMessageFromInternalError(FileHelper $fileHelper, InternalErr return $message; } - private function generateBaseline(string $generateBaselineFile, InceptionResult $inceptionResult, AnalysisResult $analysisResult, OutputInterface $output, bool $allowEmptyBaseline, string $baselineExtension, bool $failWithoutResultCache): int + private function generateBaseline(string $generateBaselineFile, InceptionResult $inceptionResult, AnalysisResult $analysisResult, OutputInterface $output, bool $allowEmptyBaseline, string $baselineExtension, bool $failWithoutResultCache, bool $ignoreNewErrors, Container $container): int { + $baselineFileDirectory = dirname($generateBaselineFile); + $fileHelper = $container->getByType(FileHelper::class); + $baselinePathHelper = new ParentDirectoryRelativePathHelper($baselineFileDirectory); + $analysisResult = $this->processFileSpecificErrorsFromAnalysisResult($analysisResult, $ignoreNewErrors, $generateBaselineFile, $inceptionResult, $fileHelper, $baselinePathHelper); + if (!$allowEmptyBaseline && !$analysisResult->hasErrors()) { $inceptionResult->getStdOutput()->getStyle()->error('No errors were found during the analysis. Baseline could not be generated.'); $inceptionResult->getStdOutput()->writeLineFormatted('To allow generating empty baselines, pass --allow-empty-baseline option.'); @@ -599,7 +615,6 @@ private function generateBaseline(string $generateBaselineFile, InceptionResult $streamOutput = $this->createStreamOutput(); $errorConsoleStyle = new ErrorsConsoleStyle(new StringInput(''), $streamOutput); $baselineOutput = new SymfonyOutput($streamOutput, new SymfonyStyle($errorConsoleStyle)); - $baselineFileDirectory = dirname($generateBaselineFile); $baselinePathHelper = new ParentDirectoryRelativePathHelper($baselineFileDirectory); if ($baselineExtension === 'php') { @@ -674,6 +689,63 @@ private function generateBaseline(string $generateBaselineFile, InceptionResult return $inceptionResult->handleReturn($exitCode, $analysisResult->getPeakMemoryUsageBytes(), $this->analysisStartTime); } + private function processFileSpecificErrorsFromAnalysisResult(AnalysisResult $analysisResult, bool $ignoreNewErrors, string $generateBaselineFile, InceptionResult $inceptionResult, FileHelper $fileHelper, RelativePathHelper $baselinePathHelper): AnalysisResult + { + $fileSpecificErrors = $analysisResult->getFileSpecificErrors(); + if (!$ignoreNewErrors) { + return $analysisResult; + } + + $baselineIgnoreErrors = $this->getCurrentBaselineIgnoreErrors($generateBaselineFile, $inceptionResult); + $ignoreErrorsByFile = $this->mapIgnoredErrors($baselineIgnoreErrors, $fileHelper); + + foreach ($fileSpecificErrors as $errorIndex => $error) { + $filePath = $baselinePathHelper->getRelativePath($error->getFilePath()); + if (isset($ignoreErrorsByFile[$filePath])) { + foreach ($ignoreErrorsByFile[$filePath] as $ignoreError) { + $ignore = $ignoreError['ignoreError']; + $shouldIgnore = IgnoredError::shouldIgnore($fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, null); + if ($shouldIgnore) { + continue 2; + } + } + } + + $traitFilePath = $error->getTraitFilePath(); + if ($traitFilePath !== null) { + $normalizedTraitFilePath = $baselinePathHelper->getRelativePath($traitFilePath); + if (isset($ignoreErrorsByFile[$normalizedTraitFilePath])) { + foreach ($ignoreErrorsByFile[$normalizedTraitFilePath] as $ignoreError) { + $ignore = $ignoreError['ignoreError']; + $shouldIgnore = IgnoredError::shouldIgnore($fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, null); + if ($shouldIgnore) { + continue 2; + } + } + } + } + + // the error was not matched in the baseline, making it a new error, new errors should be ignored here + unset($fileSpecificErrors[$errorIndex]); + } + + $fileSpecificErrors = array_values($fileSpecificErrors); + + return new AnalysisResult( + $fileSpecificErrors, + $analysisResult->getNotFileSpecificErrors(), + $analysisResult->getInternalErrorObjects(), + $analysisResult->getWarnings(), + $analysisResult->getCollectedData(), + $analysisResult->isDefaultLevelUsed(), + $analysisResult->getProjectConfigFile(), + $analysisResult->isResultCacheSaved(), + $analysisResult->getPeakMemoryUsageBytes(), + $analysisResult->isResultCacheUsed(), + $analysisResult->getChangedProjectExtensionFilesOutsideOfAnalysedPaths(), + ); + } + /** * @param string[] $files */ @@ -716,4 +788,109 @@ private function runDiagnoseExtensions(Container $container, Output $errorOutput } } + private function getCurrentBaselineIgnoreErrors(string $generateBaselineFile, InceptionResult $inceptionResult): mixed + { + $loader = new Loader(); + try { + $currentBaselineConfig = $loader->load($generateBaselineFile); + $baselineIgnoreErrors = $currentBaselineConfig['parameters']['ignoreErrors'] ?? []; + } catch (FileNotFoundException) { + // currently no baseline file -> empty config + $baselineIgnoreErrors = []; + } catch (InvalidStateException $invalidStateException) { + $inceptionResult->getErrorOutput()->writeLineFormatted($invalidStateException->getMessage()); + throw $invalidStateException; + } + return $baselineIgnoreErrors; + } + + /** + * @param (string|mixed[])[] $baselineIgnoreErrors + * @return mixed[][] + * @throws ShouldNotHappenException + */ + private function mapIgnoredErrors(array $baselineIgnoreErrors, FileHelper $fileHelper): array + { + $ignoreErrorsByFile = []; + + $expandedIgnoreErrors = []; + foreach ($baselineIgnoreErrors as $ignoreError) { + if (!is_array($ignoreError)) { + throw new ShouldNotHappenException('Baseline should not have ignore error strings'); + } + + if (!isset($ignoreError['message']) && !isset($ignoreError['messages']) && !isset($ignoreError['identifier'])) { + continue; + } + if (isset($ignoreError['messages'])) { + foreach ($ignoreError['messages'] as $message) { + $expandedIgnoreError = $ignoreError; + unset($expandedIgnoreError['messages']); + $expandedIgnoreError['message'] = $message; + $expandedIgnoreErrors[] = $expandedIgnoreError; + } + } else { + $expandedIgnoreErrors[] = $ignoreError; + } + } + $uniquedExpandedIgnoreErrors = []; + foreach ($expandedIgnoreErrors as $ignoreError) { + if (!isset($ignoreError['message']) && !isset($ignoreError['identifier'])) { + $uniquedExpandedIgnoreErrors[] = $ignoreError; + continue; + } + if (!isset($ignoreError['path'])) { + $uniquedExpandedIgnoreErrors[] = $ignoreError; + continue; + } + + $key = $ignoreError['path']; + if (isset($ignoreError['message'])) { + $key = sprintf("%s\n%s", $key, $ignoreError['message']); + } + if (isset($ignoreError['identifier'])) { + $key = sprintf("%s\n%s", $key, $ignoreError['identifier']); + } + if ($key === '') { + throw new ShouldNotHappenException(); + } + + if (!array_key_exists($key, $uniquedExpandedIgnoreErrors)) { + $uniquedExpandedIgnoreErrors[$key] = $ignoreError; + continue; + } + + $uniquedExpandedIgnoreErrors[$key] = [ + 'message' => $ignoreError['message'] ?? null, + 'path' => $ignoreError['path'], + 'identifier' => $ignoreError['identifier'] ?? null, + 'count' => ($uniquedExpandedIgnoreErrors[$key]['count'] ?? 1) + ($ignoreError['count'] ?? 1), + 'reportUnmatched' => false, + ]; + } + $expandedIgnoreErrors = array_values($uniquedExpandedIgnoreErrors); + + foreach ($expandedIgnoreErrors as $i => $ignoreError) { + $ignoreErrorEntry = [ + 'index' => $i, + 'ignoreError' => $ignoreError, + ]; + + if (!isset($ignoreError['message']) && !isset($ignoreError['identifier'])) { + continue; + } + if (!isset($ignoreError['path'])) { + throw new ShouldNotHappenException('Baseline should not have ignore errors without path'); + } + + $normalizedPath = $fileHelper->normalizePath($ignoreError['path']); + $ignoreError['path'] = $normalizedPath; + $ignoreErrorsByFile[$normalizedPath][] = $ignoreErrorEntry; + $ignoreError['realPath'] = $normalizedPath; + $expandedIgnoreErrors[$i] = $ignoreError; + } + + return $ignoreErrorsByFile; + } + } diff --git a/tests/PHPStan/Command/AnalyseCommandTest.php b/tests/PHPStan/Command/AnalyseCommandTest.php index f47edea747..dee9bbe24e 100644 --- a/tests/PHPStan/Command/AnalyseCommandTest.php +++ b/tests/PHPStan/Command/AnalyseCommandTest.php @@ -6,11 +6,14 @@ use PHPStan\Testing\PHPStanTestCase; use Symfony\Component\Console\Tester\CommandTester; use Throwable; +use function array_merge; use function chdir; use function getcwd; use function microtime; use function realpath; +use function rename; use function sprintf; +use function unlink; use const DIRECTORY_SEPARATOR; use const PHP_EOL; @@ -71,6 +74,74 @@ public function testValidAutoloadFile(): void } } + public function testGenerateBaselineIgnoreNewErrorsRemoveFile(): void + { + $baselineFile = __DIR__ . '/data-ignore-new-errors/baseline.neon'; + $this->runCommand(0, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php', __DIR__ . '/data-ignore-new-errors/B.php'], + '--configuration' => __DIR__ . '/data-ignore-new-errors/empty.neon', + '--level' => '9', + '--generate-baseline' => $baselineFile, + ]); + + $output = $this->runCommand(0, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors/B.php', __DIR__ . '/data-ignore-new-errors/C.php'], + '--configuration' => $baselineFile, + '--level' => '9', + '--generate-baseline' => $baselineFile, + '--ignore-new-errors' => true, + ]); + @unlink($baselineFile); + + $this->assertStringContainsString('[OK] Baseline generated with 1 error', $output); + } + + public function testGenerateBaselineIgnoreNewErrorsChangeFile(): void + { + $baselineFile = __DIR__ . '/data-ignore-new-errors-baseline/baseline.neon'; + $baselineFileSecondRun = __DIR__ . '/data-ignore-new-errors/baseline.neon'; + $this->runCommand(0, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors-baseline/A.php'], + '--configuration' => __DIR__ . '/data-ignore-new-errors-baseline/empty.neon', + '--level' => '9', + '--generate-baseline' => $baselineFile, + ]); + + rename($baselineFile, $baselineFileSecondRun); + $output = $this->runCommand(0, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php'], + '--configuration' => $baselineFileSecondRun, + '--level' => '9', + '--generate-baseline' => $baselineFileSecondRun, + '--ignore-new-errors' => true, + ]); + @unlink($baselineFileSecondRun); + + $this->assertStringContainsString('[OK] Baseline generated with 2 errors', $output); + } + + public function testGenerateBaselineIgnoreNewErrorsEmptyBaseline(): void + { + $baselineFile = __DIR__ . '/data-ignore-new-errors/baseline.neon'; + $this->runCommand(0, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php', __DIR__ . '/data-ignore-new-errors/B.php'], + '--configuration' => __DIR__ . '/data-ignore-new-errors/empty.neon', + '--level' => '9', + '--generate-baseline' => $baselineFile, + ]); + + $output = $this->runCommand(1, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors/C.php'], + '--configuration' => $baselineFile, + '--level' => '9', + '--generate-baseline' => $baselineFile, + '--ignore-new-errors' => true, + ]); + @unlink($baselineFile); + + $this->assertStringContainsString('[ERROR] No errors were found during the analysis. Baseline could not be generated.', $output); + } + /** * @return string[][] */ @@ -117,16 +188,16 @@ public static function autoDiscoveryPathsProvider(): array } /** - * @param array $parameters + * @param array $parameters */ private function runCommand(int $expectedStatusCode, array $parameters = []): string { $commandTester = new CommandTester(new AnalyseCommand([], microtime(true))); - $commandTester->execute([ + $commandTester->execute(array_merge([ 'paths' => [__DIR__ . DIRECTORY_SEPARATOR . 'test'], '--debug' => true, - ] + $parameters, ['debug' => true]); + ], $parameters), ['debug' => true]); $this->assertSame($expectedStatusCode, $commandTester->getStatusCode(), $commandTester->getDisplay()); diff --git a/tests/PHPStan/Command/data-ignore-new-errors-baseline/A.php b/tests/PHPStan/Command/data-ignore-new-errors-baseline/A.php new file mode 100644 index 0000000000..59ae2c9e8e --- /dev/null +++ b/tests/PHPStan/Command/data-ignore-new-errors-baseline/A.php @@ -0,0 +1,16 @@ +> + */ + public function doFoo(): array + { + return [['foo']]; + } + + public function doBar(): void + { + array_key_first(); + } + +} diff --git a/tests/PHPStan/Command/data-ignore-new-errors/B.php b/tests/PHPStan/Command/data-ignore-new-errors/B.php new file mode 100644 index 0000000000..684afd5a8e --- /dev/null +++ b/tests/PHPStan/Command/data-ignore-new-errors/B.php @@ -0,0 +1,16 @@ +> + */ + public function doFoo(): array + { + return [['foo']]; + } + +} diff --git a/tests/PHPStan/Command/data-ignore-new-errors/C.php b/tests/PHPStan/Command/data-ignore-new-errors/C.php new file mode 100644 index 0000000000..7dfac0fb01 --- /dev/null +++ b/tests/PHPStan/Command/data-ignore-new-errors/C.php @@ -0,0 +1,16 @@ +> + */ + public function doFoo(): array + { + return [['foo']]; + } + +} diff --git a/tests/PHPStan/Command/data-ignore-new-errors/empty.neon b/tests/PHPStan/Command/data-ignore-new-errors/empty.neon new file mode 100644 index 0000000000..e69de29bb2 From 6f1584aa5ab0d06faafee00070f5195280b94c33 Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Wed, 12 Mar 2025 22:51:55 +0100 Subject: [PATCH 04/10] Handle more errors than ignore count in --ignore-new-errors --- src/Command/AnalyseCommand.php | 138 +++++++++++---------------------- 1 file changed, 45 insertions(+), 93 deletions(-) diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index 1b01a2bb15..db55a0daa1 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -6,6 +6,7 @@ use Nette\FileNotFoundException; use Nette\InvalidStateException; use OndraM\CiDetector\CiDetector; +use PHPStan\Analyser\Error; use PHPStan\Analyser\Ignore\IgnoredError; use PHPStan\Analyser\InternalError; use PHPStan\Command\ErrorFormatter\BaselineNeonErrorFormatter; @@ -603,7 +604,9 @@ private function generateBaseline(string $generateBaselineFile, InceptionResult $baselineFileDirectory = dirname($generateBaselineFile); $fileHelper = $container->getByType(FileHelper::class); $baselinePathHelper = new ParentDirectoryRelativePathHelper($baselineFileDirectory); - $analysisResult = $this->processFileSpecificErrorsFromAnalysisResult($analysisResult, $ignoreNewErrors, $generateBaselineFile, $inceptionResult, $fileHelper, $baselinePathHelper); + if ($ignoreNewErrors) { + $analysisResult = $this->filterAnalysisResultForExistingErrors($analysisResult, $generateBaselineFile, $inceptionResult, $fileHelper, $baselinePathHelper); + } if (!$allowEmptyBaseline && !$analysisResult->hasErrors()) { $inceptionResult->getStdOutput()->getStyle()->error('No errors were found during the analysis. Baseline could not be generated.'); @@ -689,39 +692,26 @@ private function generateBaseline(string $generateBaselineFile, InceptionResult return $inceptionResult->handleReturn($exitCode, $analysisResult->getPeakMemoryUsageBytes(), $this->analysisStartTime); } - private function processFileSpecificErrorsFromAnalysisResult(AnalysisResult $analysisResult, bool $ignoreNewErrors, string $generateBaselineFile, InceptionResult $inceptionResult, FileHelper $fileHelper, RelativePathHelper $baselinePathHelper): AnalysisResult + private function filterAnalysisResultForExistingErrors(AnalysisResult $analysisResult, string $generateBaselineFile, InceptionResult $inceptionResult, FileHelper $fileHelper, RelativePathHelper $baselinePathHelper): AnalysisResult { $fileSpecificErrors = $analysisResult->getFileSpecificErrors(); - if (!$ignoreNewErrors) { - return $analysisResult; - } $baselineIgnoreErrors = $this->getCurrentBaselineIgnoreErrors($generateBaselineFile, $inceptionResult); $ignoreErrorsByFile = $this->mapIgnoredErrors($baselineIgnoreErrors, $fileHelper); + $ignoreUseCount = []; + foreach ($fileSpecificErrors as $errorIndex => $error) { - $filePath = $baselinePathHelper->getRelativePath($error->getFilePath()); - if (isset($ignoreErrorsByFile[$filePath])) { - foreach ($ignoreErrorsByFile[$filePath] as $ignoreError) { - $ignore = $ignoreError['ignoreError']; - $shouldIgnore = IgnoredError::shouldIgnore($fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, null); - if ($shouldIgnore) { - continue 2; - } - } + $hasMatchingIgnore = $this->checkIgnoreErrorByPath($error->getFilePath(), $ignoreErrorsByFile, $fileHelper, $error, $ignoreUseCount, $baselinePathHelper); + if ($hasMatchingIgnore) { + continue; } $traitFilePath = $error->getTraitFilePath(); if ($traitFilePath !== null) { - $normalizedTraitFilePath = $baselinePathHelper->getRelativePath($traitFilePath); - if (isset($ignoreErrorsByFile[$normalizedTraitFilePath])) { - foreach ($ignoreErrorsByFile[$normalizedTraitFilePath] as $ignoreError) { - $ignore = $ignoreError['ignoreError']; - $shouldIgnore = IgnoredError::shouldIgnore($fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, null); - if ($shouldIgnore) { - continue 2; - } - } + $hasMatchingIgnore = $this->checkIgnoreErrorByPath($traitFilePath, $ignoreErrorsByFile, $fileHelper, $error, $ignoreUseCount, $baselinePathHelper); + if ($hasMatchingIgnore) { + continue; } } @@ -746,6 +736,36 @@ private function processFileSpecificErrorsFromAnalysisResult(AnalysisResult $ana ); } + /** + * @param mixed[][] $ignoreErrorsByFile + * @param int[] $ignoreUseCount map of indexes of ignores and how often they have been "used" to ignore an error + */ + private function checkIgnoreErrorByPath(string $filePath, array $ignoreErrorsByFile, FileHelper $fileHelper, Error $error, array &$ignoreUseCount, RelativePathHelper $baselinePathHelper): bool + { + $relativePath = $baselinePathHelper->getRelativePath($filePath); + if (!isset($ignoreErrorsByFile[$relativePath])) { + return false; + } + + foreach ($ignoreErrorsByFile[$relativePath] as $ignoreError) { + $ignore = $ignoreError['ignoreError']; + $shouldIgnore = IgnoredError::shouldIgnore($fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, null); + if (!$shouldIgnore) { + continue; + } + + $realCount = $ignoreUseCount[$ignoreError['index']] ?? 0; + $realCount++; + $ignoreUseCount[$ignoreError['index']] = $realCount; + + if ($realCount <= $ignore['count']) { + return true; + } + } + + return false; + } + /** * @param string[] $files */ @@ -805,89 +825,21 @@ private function getCurrentBaselineIgnoreErrors(string $generateBaselineFile, In } /** - * @param (string|mixed[])[] $baselineIgnoreErrors + * @param mixed[][] $baselineIgnoreErrors * @return mixed[][] - * @throws ShouldNotHappenException */ private function mapIgnoredErrors(array $baselineIgnoreErrors, FileHelper $fileHelper): array { $ignoreErrorsByFile = []; - $expandedIgnoreErrors = []; - foreach ($baselineIgnoreErrors as $ignoreError) { - if (!is_array($ignoreError)) { - throw new ShouldNotHappenException('Baseline should not have ignore error strings'); - } - - if (!isset($ignoreError['message']) && !isset($ignoreError['messages']) && !isset($ignoreError['identifier'])) { - continue; - } - if (isset($ignoreError['messages'])) { - foreach ($ignoreError['messages'] as $message) { - $expandedIgnoreError = $ignoreError; - unset($expandedIgnoreError['messages']); - $expandedIgnoreError['message'] = $message; - $expandedIgnoreErrors[] = $expandedIgnoreError; - } - } else { - $expandedIgnoreErrors[] = $ignoreError; - } - } - $uniquedExpandedIgnoreErrors = []; - foreach ($expandedIgnoreErrors as $ignoreError) { - if (!isset($ignoreError['message']) && !isset($ignoreError['identifier'])) { - $uniquedExpandedIgnoreErrors[] = $ignoreError; - continue; - } - if (!isset($ignoreError['path'])) { - $uniquedExpandedIgnoreErrors[] = $ignoreError; - continue; - } - - $key = $ignoreError['path']; - if (isset($ignoreError['message'])) { - $key = sprintf("%s\n%s", $key, $ignoreError['message']); - } - if (isset($ignoreError['identifier'])) { - $key = sprintf("%s\n%s", $key, $ignoreError['identifier']); - } - if ($key === '') { - throw new ShouldNotHappenException(); - } - - if (!array_key_exists($key, $uniquedExpandedIgnoreErrors)) { - $uniquedExpandedIgnoreErrors[$key] = $ignoreError; - continue; - } - - $uniquedExpandedIgnoreErrors[$key] = [ - 'message' => $ignoreError['message'] ?? null, - 'path' => $ignoreError['path'], - 'identifier' => $ignoreError['identifier'] ?? null, - 'count' => ($uniquedExpandedIgnoreErrors[$key]['count'] ?? 1) + ($ignoreError['count'] ?? 1), - 'reportUnmatched' => false, - ]; - } - $expandedIgnoreErrors = array_values($uniquedExpandedIgnoreErrors); - - foreach ($expandedIgnoreErrors as $i => $ignoreError) { + foreach ($baselineIgnoreErrors as $i => $ignoreError) { $ignoreErrorEntry = [ 'index' => $i, 'ignoreError' => $ignoreError, ]; - if (!isset($ignoreError['message']) && !isset($ignoreError['identifier'])) { - continue; - } - if (!isset($ignoreError['path'])) { - throw new ShouldNotHappenException('Baseline should not have ignore errors without path'); - } - $normalizedPath = $fileHelper->normalizePath($ignoreError['path']); - $ignoreError['path'] = $normalizedPath; $ignoreErrorsByFile[$normalizedPath][] = $ignoreErrorEntry; - $ignoreError['realPath'] = $normalizedPath; - $expandedIgnoreErrors[$i] = $ignoreError; } return $ignoreErrorsByFile; From bcc78167c7bf0c0f75262df4dfbafcd70af9d75c Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Thu, 13 Mar 2025 20:32:37 +0100 Subject: [PATCH 05/10] Add test for increasing error count --- tests/PHPStan/Command/AnalyseCommandTest.php | 32 ++++++++++++++++--- .../A.php | 0 .../empty.neon | 0 3 files changed, 28 insertions(+), 4 deletions(-) rename tests/PHPStan/Command/{data-ignore-new-errors-baseline => data-ignore-new-errors-compare}/A.php (100%) rename tests/PHPStan/Command/{data-ignore-new-errors-baseline => data-ignore-new-errors-compare}/empty.neon (100%) diff --git a/tests/PHPStan/Command/AnalyseCommandTest.php b/tests/PHPStan/Command/AnalyseCommandTest.php index dee9bbe24e..325af946f9 100644 --- a/tests/PHPStan/Command/AnalyseCommandTest.php +++ b/tests/PHPStan/Command/AnalyseCommandTest.php @@ -96,13 +96,13 @@ public function testGenerateBaselineIgnoreNewErrorsRemoveFile(): void $this->assertStringContainsString('[OK] Baseline generated with 1 error', $output); } - public function testGenerateBaselineIgnoreNewErrorsChangeFile(): void + public function testGenerateBaselineIgnoreNewErrorsReducedErrorCount(): void { - $baselineFile = __DIR__ . '/data-ignore-new-errors-baseline/baseline.neon'; + $baselineFile = __DIR__ . '/data-ignore-new-errors-compare/baseline.neon'; $baselineFileSecondRun = __DIR__ . '/data-ignore-new-errors/baseline.neon'; $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors-baseline/A.php'], - '--configuration' => __DIR__ . '/data-ignore-new-errors-baseline/empty.neon', + 'paths' => [__DIR__ . '/data-ignore-new-errors-compare/A.php'], + '--configuration' => __DIR__ . '/data-ignore-new-errors-compare/empty.neon', '--level' => '9', '--generate-baseline' => $baselineFile, ]); @@ -120,6 +120,30 @@ public function testGenerateBaselineIgnoreNewErrorsChangeFile(): void $this->assertStringContainsString('[OK] Baseline generated with 2 errors', $output); } + public function testGenerateBaselineIgnoreNewErrorsIncreasedErrorCount(): void + { + $baselineFile = __DIR__ . '/data-ignore-new-errors/baseline.neon'; + $baselineFileSecondRun = __DIR__ . '/data-ignore-new-errors-compare/baseline.neon'; + $this->runCommand(0, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php'], + '--configuration' => __DIR__ . '/data-ignore-new-errors/empty.neon', + '--level' => '9', + '--generate-baseline' => $baselineFile, + ]); + + rename($baselineFile, $baselineFileSecondRun); + $output = $this->runCommand(0, [ + 'paths' => [__DIR__ . '/data-ignore-new-errors-compare/A.php'], + '--configuration' => $baselineFileSecondRun, + '--level' => '9', + '--generate-baseline' => $baselineFileSecondRun, + '--ignore-new-errors' => true, + ]); + @unlink($baselineFileSecondRun); + + $this->assertStringContainsString('[OK] Baseline generated with 2 errors', $output); + } + public function testGenerateBaselineIgnoreNewErrorsEmptyBaseline(): void { $baselineFile = __DIR__ . '/data-ignore-new-errors/baseline.neon'; diff --git a/tests/PHPStan/Command/data-ignore-new-errors-baseline/A.php b/tests/PHPStan/Command/data-ignore-new-errors-compare/A.php similarity index 100% rename from tests/PHPStan/Command/data-ignore-new-errors-baseline/A.php rename to tests/PHPStan/Command/data-ignore-new-errors-compare/A.php diff --git a/tests/PHPStan/Command/data-ignore-new-errors-baseline/empty.neon b/tests/PHPStan/Command/data-ignore-new-errors-compare/empty.neon similarity index 100% rename from tests/PHPStan/Command/data-ignore-new-errors-baseline/empty.neon rename to tests/PHPStan/Command/data-ignore-new-errors-compare/empty.neon From cc56f290f5b15f00e198adae17bf3514c4110df8 Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Thu, 13 Mar 2025 20:38:04 +0100 Subject: [PATCH 06/10] Add ignore-new-errors test files to collision ignores --- build/collision-detector.json | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/build/collision-detector.json b/build/collision-detector.json index a687cd3ea4..ad8e022e1b 100644 --- a/build/collision-detector.json +++ b/build/collision-detector.json @@ -15,6 +15,9 @@ "../tests/PHPStan/Rules/Functions/data/define-bug-3349.php", "../tests/PHPStan/Levels/data/stubs/function.php", "../tests/PHPStan/Rules/Properties/data/abstract-final-property-hook-parse-error.php", - "../tests/PHPStan/Rules/Properties/data/final-property-hooks.php" - ] + "../tests/PHPStan/Rules/Properties/data/final-property-hooks.php", + "../tests/PHPStan/Levels/data/stubs/function.php", + "../tests/PHPStan/Command/data-ignore-new-errors/A.php", + "../tests/PHPStan/Command/data-ignore-new-errors-compare/A.php" + ] } From f285fc493927674725b53d205c2cdf3185d12d51 Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Thu, 13 Mar 2025 20:41:35 +0100 Subject: [PATCH 07/10] Shorten assert text of testGenerateBaselineIgnoreNewErrorsEmptyBaseline causes failure in pipeline where less text is rendered on one row --- tests/PHPStan/Command/AnalyseCommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Command/AnalyseCommandTest.php b/tests/PHPStan/Command/AnalyseCommandTest.php index 325af946f9..6bd0286ee6 100644 --- a/tests/PHPStan/Command/AnalyseCommandTest.php +++ b/tests/PHPStan/Command/AnalyseCommandTest.php @@ -163,7 +163,7 @@ public function testGenerateBaselineIgnoreNewErrorsEmptyBaseline(): void ]); @unlink($baselineFile); - $this->assertStringContainsString('[ERROR] No errors were found during the analysis. Baseline could not be generated.', $output); + $this->assertStringContainsString('[ERROR] No errors were found during the analysis.', $output); } /** From 3464d8ab21100e495a3353f3b91159a47baaa2dc Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Wed, 19 Mar 2025 20:56:37 +0100 Subject: [PATCH 08/10] Rename ignore-new-errors option to only-remove-errors --- src/Command/AnalyseCommand.php | 14 +++++++------- tests/PHPStan/Command/AnalyseCommandTest.php | 8 ++++---- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index db55a0daa1..119c9f5111 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -107,7 +107,7 @@ protected function configure(): void new InputOption('watch', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('pro', null, InputOption::VALUE_NONE, 'Launch PHPStan Pro'), new InputOption('fail-without-result-cache', null, InputOption::VALUE_NONE, 'Return non-zero exit code when result cache is not used'), - new InputOption('ignore-new-errors', null, InputOption::VALUE_NONE, 'Ignore new errors when generating the baseline.'), + new InputOption('only-remove-errors', null, InputOption::VALUE_NONE, 'Only remove existing errors from the baseline. Do not add new ones.'), ]); } @@ -142,7 +142,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $debugEnabled = (bool) $input->getOption('debug'); $fix = (bool) $input->getOption('fix') || (bool) $input->getOption('watch') || (bool) $input->getOption('pro'); $failWithoutResultCache = (bool) $input->getOption('fail-without-result-cache'); - $ignoreNewErrors = (bool) $input->getOption('ignore-new-errors'); + $onlyRemoveErrors = (bool) $input->getOption('only-remove-errors'); /** @var string|false|null $generateBaselineFile */ $generateBaselineFile = $input->getOption('generate-baseline'); @@ -189,8 +189,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); } - if ($generateBaselineFile === null && $ignoreNewErrors) { - $inceptionResult->getStdOutput()->getStyle()->error('You must pass the --generate-baseline option alongside --ignore-new-errors.'); + if ($generateBaselineFile === null && $onlyRemoveErrors) { + $inceptionResult->getStdOutput()->getStyle()->error('You must pass the --generate-baseline option alongside --only-remove-errors.'); return $inceptionResult->handleReturn(1, null, $this->analysisStartTime); } @@ -423,7 +423,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int return $inceptionResult->handleReturn(1, $analysisResult->getPeakMemoryUsageBytes(), $this->analysisStartTime); } - return $this->generateBaseline($generateBaselineFile, $inceptionResult, $analysisResult, $output, $allowEmptyBaseline, $baselineExtension, $failWithoutResultCache, $ignoreNewErrors, $container); + return $this->generateBaseline($generateBaselineFile, $inceptionResult, $analysisResult, $output, $allowEmptyBaseline, $baselineExtension, $failWithoutResultCache, $onlyRemoveErrors, $container); } /** @var ErrorFormatter $errorFormatter */ @@ -599,12 +599,12 @@ private function getMessageFromInternalError(FileHelper $fileHelper, InternalErr return $message; } - private function generateBaseline(string $generateBaselineFile, InceptionResult $inceptionResult, AnalysisResult $analysisResult, OutputInterface $output, bool $allowEmptyBaseline, string $baselineExtension, bool $failWithoutResultCache, bool $ignoreNewErrors, Container $container): int + private function generateBaseline(string $generateBaselineFile, InceptionResult $inceptionResult, AnalysisResult $analysisResult, OutputInterface $output, bool $allowEmptyBaseline, string $baselineExtension, bool $failWithoutResultCache, bool $onlyRemoveErrors, Container $container): int { $baselineFileDirectory = dirname($generateBaselineFile); $fileHelper = $container->getByType(FileHelper::class); $baselinePathHelper = new ParentDirectoryRelativePathHelper($baselineFileDirectory); - if ($ignoreNewErrors) { + if ($onlyRemoveErrors) { $analysisResult = $this->filterAnalysisResultForExistingErrors($analysisResult, $generateBaselineFile, $inceptionResult, $fileHelper, $baselinePathHelper); } diff --git a/tests/PHPStan/Command/AnalyseCommandTest.php b/tests/PHPStan/Command/AnalyseCommandTest.php index 6bd0286ee6..f7299f6af3 100644 --- a/tests/PHPStan/Command/AnalyseCommandTest.php +++ b/tests/PHPStan/Command/AnalyseCommandTest.php @@ -89,7 +89,7 @@ public function testGenerateBaselineIgnoreNewErrorsRemoveFile(): void '--configuration' => $baselineFile, '--level' => '9', '--generate-baseline' => $baselineFile, - '--ignore-new-errors' => true, + '--only-remove-errors' => true, ]); @unlink($baselineFile); @@ -113,7 +113,7 @@ public function testGenerateBaselineIgnoreNewErrorsReducedErrorCount(): void '--configuration' => $baselineFileSecondRun, '--level' => '9', '--generate-baseline' => $baselineFileSecondRun, - '--ignore-new-errors' => true, + '--only-remove-errors' => true, ]); @unlink($baselineFileSecondRun); @@ -137,7 +137,7 @@ public function testGenerateBaselineIgnoreNewErrorsIncreasedErrorCount(): void '--configuration' => $baselineFileSecondRun, '--level' => '9', '--generate-baseline' => $baselineFileSecondRun, - '--ignore-new-errors' => true, + '--only-remove-errors' => true, ]); @unlink($baselineFileSecondRun); @@ -159,7 +159,7 @@ public function testGenerateBaselineIgnoreNewErrorsEmptyBaseline(): void '--configuration' => $baselineFile, '--level' => '9', '--generate-baseline' => $baselineFile, - '--ignore-new-errors' => true, + '--only-remove-errors' => true, ]); @unlink($baselineFile); From e69bb2c0007d9f206e450e8e308c2f9def04395f Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Wed, 19 Mar 2025 21:40:17 +0100 Subject: [PATCH 09/10] Extract ignore-new-errors logic into BaselineIgnoredErrorHelper --- conf/config.neon | 3 + .../Ignore/BaselineIgnoredErrorHelper.php | 104 ++++++++++++++++++ src/Command/AnalyseCommand.php | 101 +++-------------- 3 files changed, 123 insertions(+), 85 deletions(-) create mode 100644 src/Analyser/Ignore/BaselineIgnoredErrorHelper.php diff --git a/conf/config.neon b/conf/config.neon index e764d064eb..84f0f0f18e 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -470,6 +470,9 @@ services: ignoreErrors: %ignoreErrors% reportUnmatchedIgnoredErrors: %reportUnmatchedIgnoredErrors% + - + class: PHPStan\Analyser\Ignore\BaselineIgnoredErrorHelper + - class: PHPStan\Analyser\Ignore\IgnoreLexer diff --git a/src/Analyser/Ignore/BaselineIgnoredErrorHelper.php b/src/Analyser/Ignore/BaselineIgnoredErrorHelper.php new file mode 100644 index 0000000000..cc3e721228 --- /dev/null +++ b/src/Analyser/Ignore/BaselineIgnoredErrorHelper.php @@ -0,0 +1,104 @@ + $currentAnalysisErrors errors from the current analysis + * @return list errors from the current analysis which already exit in the baseline + */ + public function removeUnusedIgnoredErrors(array $baselinedErrors, array $currentAnalysisErrors, ParentDirectoryRelativePathHelper $baselinePathHelper): array + { + $ignoreErrorsByFile = $this->mapIgnoredErrorsByFile($baselinedErrors); + + $ignoreUseCount = []; + $nextBaselinedErrors = []; + foreach ($currentAnalysisErrors as $error) { + $hasMatchingIgnore = $this->checkIgnoreErrorByPath($error->getFilePath(), $ignoreErrorsByFile, $error, $ignoreUseCount, $baselinePathHelper); + if ($hasMatchingIgnore) { + $nextBaselinedErrors[] = $error; + continue; + } + + $traitFilePath = $error->getTraitFilePath(); + if ($traitFilePath === null) { + continue; + } + + $hasMatchingIgnore = $this->checkIgnoreErrorByPath($traitFilePath, $ignoreErrorsByFile, $error, $ignoreUseCount, $baselinePathHelper); + if (!$hasMatchingIgnore) { + continue; + } + + $nextBaselinedErrors[] = $error; + } + + return $nextBaselinedErrors; + } + + /** + * @param mixed[][] $ignoreErrorsByFile + * @param int[] $ignoreUseCount map of indexes of ignores and how often they have been "used" to ignore an error + */ + private function checkIgnoreErrorByPath(string $filePath, array $ignoreErrorsByFile, Error $error, array &$ignoreUseCount, RelativePathHelper $baselinePathHelper): bool + { + $relativePath = $baselinePathHelper->getRelativePath($filePath); + if (!isset($ignoreErrorsByFile[$relativePath])) { + return false; + } + + foreach ($ignoreErrorsByFile[$relativePath] as $ignoreError) { + $ignore = $ignoreError['ignoreError']; + $shouldIgnore = IgnoredError::shouldIgnore($this->fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, null); + if (!$shouldIgnore) { + continue; + } + + $realCount = $ignoreUseCount[$ignoreError['index']] ?? 0; + $realCount++; + $ignoreUseCount[$ignoreError['index']] = $realCount; + + if ($realCount <= $ignore['count']) { + return true; + } + } + + return false; + } + + /** + * @param mixed[][] $baselineIgnoreErrors + * @return mixed[][] ignored errors from baseline mapped and grouped by files + */ + private function mapIgnoredErrorsByFile(array $baselineIgnoreErrors): array + { + $ignoreErrorsByFile = []; + + foreach ($baselineIgnoreErrors as $i => $ignoreError) { + $ignoreErrorEntry = [ + 'index' => $i, + 'ignoreError' => $ignoreError, + ]; + + $normalizedPath = $this->fileHelper->normalizePath($ignoreError['path']); + $ignoreErrorsByFile[$normalizedPath][] = $ignoreErrorEntry; + } + + return $ignoreErrorsByFile; + } + +} diff --git a/src/Command/AnalyseCommand.php b/src/Command/AnalyseCommand.php index 119c9f5111..affd28d382 100644 --- a/src/Command/AnalyseCommand.php +++ b/src/Command/AnalyseCommand.php @@ -6,8 +6,7 @@ use Nette\FileNotFoundException; use Nette\InvalidStateException; use OndraM\CiDetector\CiDetector; -use PHPStan\Analyser\Error; -use PHPStan\Analyser\Ignore\IgnoredError; +use PHPStan\Analyser\Ignore\BaselineIgnoredErrorHelper; use PHPStan\Analyser\InternalError; use PHPStan\Command\ErrorFormatter\BaselineNeonErrorFormatter; use PHPStan\Command\ErrorFormatter\BaselinePhpErrorFormatter; @@ -602,10 +601,9 @@ private function getMessageFromInternalError(FileHelper $fileHelper, InternalErr private function generateBaseline(string $generateBaselineFile, InceptionResult $inceptionResult, AnalysisResult $analysisResult, OutputInterface $output, bool $allowEmptyBaseline, string $baselineExtension, bool $failWithoutResultCache, bool $onlyRemoveErrors, Container $container): int { $baselineFileDirectory = dirname($generateBaselineFile); - $fileHelper = $container->getByType(FileHelper::class); $baselinePathHelper = new ParentDirectoryRelativePathHelper($baselineFileDirectory); if ($onlyRemoveErrors) { - $analysisResult = $this->filterAnalysisResultForExistingErrors($analysisResult, $generateBaselineFile, $inceptionResult, $fileHelper, $baselinePathHelper); + $analysisResult = $this->filterAnalysisResultForExistingErrors($analysisResult, $generateBaselineFile, $inceptionResult, $container, $baselinePathHelper); } if (!$allowEmptyBaseline && !$analysisResult->hasErrors()) { @@ -618,7 +616,6 @@ private function generateBaseline(string $generateBaselineFile, InceptionResult $streamOutput = $this->createStreamOutput(); $errorConsoleStyle = new ErrorsConsoleStyle(new StringInput(''), $streamOutput); $baselineOutput = new SymfonyOutput($streamOutput, new SymfonyStyle($errorConsoleStyle)); - $baselinePathHelper = new ParentDirectoryRelativePathHelper($baselineFileDirectory); if ($baselineExtension === 'php') { $baselineErrorFormatter = new BaselinePhpErrorFormatter($baselinePathHelper); @@ -692,37 +689,19 @@ private function generateBaseline(string $generateBaselineFile, InceptionResult return $inceptionResult->handleReturn($exitCode, $analysisResult->getPeakMemoryUsageBytes(), $this->analysisStartTime); } - private function filterAnalysisResultForExistingErrors(AnalysisResult $analysisResult, string $generateBaselineFile, InceptionResult $inceptionResult, FileHelper $fileHelper, RelativePathHelper $baselinePathHelper): AnalysisResult + private function filterAnalysisResultForExistingErrors(AnalysisResult $analysisResult, string $generateBaselineFile, InceptionResult $inceptionResult, Container $container, ParentDirectoryRelativePathHelper $baselinePathHelper): AnalysisResult { - $fileSpecificErrors = $analysisResult->getFileSpecificErrors(); - - $baselineIgnoreErrors = $this->getCurrentBaselineIgnoreErrors($generateBaselineFile, $inceptionResult); - $ignoreErrorsByFile = $this->mapIgnoredErrors($baselineIgnoreErrors, $fileHelper); - - $ignoreUseCount = []; - - foreach ($fileSpecificErrors as $errorIndex => $error) { - $hasMatchingIgnore = $this->checkIgnoreErrorByPath($error->getFilePath(), $ignoreErrorsByFile, $fileHelper, $error, $ignoreUseCount, $baselinePathHelper); - if ($hasMatchingIgnore) { - continue; - } + $currentAnalysisErrors = $analysisResult->getFileSpecificErrors(); - $traitFilePath = $error->getTraitFilePath(); - if ($traitFilePath !== null) { - $hasMatchingIgnore = $this->checkIgnoreErrorByPath($traitFilePath, $ignoreErrorsByFile, $fileHelper, $error, $ignoreUseCount, $baselinePathHelper); - if ($hasMatchingIgnore) { - continue; - } - } + $currentBaselinedErrors = $this->getCurrentBaselinedErrors($generateBaselineFile, $inceptionResult); - // the error was not matched in the baseline, making it a new error, new errors should be ignored here - unset($fileSpecificErrors[$errorIndex]); - } + /** @var BaselineIgnoredErrorHelper $baselineIgnoredErrorsHelper */ + $baselineIgnoredErrorsHelper = $container->getByType(BaselineIgnoredErrorHelper::class); - $fileSpecificErrors = array_values($fileSpecificErrors); + $nextBaselinedErrors = $baselineIgnoredErrorsHelper->removeUnusedIgnoredErrors($currentBaselinedErrors, $currentAnalysisErrors, $baselinePathHelper); return new AnalysisResult( - $fileSpecificErrors, + $nextBaselinedErrors, $analysisResult->getNotFileSpecificErrors(), $analysisResult->getInternalErrorObjects(), $analysisResult->getWarnings(), @@ -736,36 +715,6 @@ private function filterAnalysisResultForExistingErrors(AnalysisResult $analysisR ); } - /** - * @param mixed[][] $ignoreErrorsByFile - * @param int[] $ignoreUseCount map of indexes of ignores and how often they have been "used" to ignore an error - */ - private function checkIgnoreErrorByPath(string $filePath, array $ignoreErrorsByFile, FileHelper $fileHelper, Error $error, array &$ignoreUseCount, RelativePathHelper $baselinePathHelper): bool - { - $relativePath = $baselinePathHelper->getRelativePath($filePath); - if (!isset($ignoreErrorsByFile[$relativePath])) { - return false; - } - - foreach ($ignoreErrorsByFile[$relativePath] as $ignoreError) { - $ignore = $ignoreError['ignoreError']; - $shouldIgnore = IgnoredError::shouldIgnore($fileHelper, $error, $ignore['message'] ?? null, $ignore['identifier'] ?? null, null); - if (!$shouldIgnore) { - continue; - } - - $realCount = $ignoreUseCount[$ignoreError['index']] ?? 0; - $realCount++; - $ignoreUseCount[$ignoreError['index']] = $realCount; - - if ($realCount <= $ignore['count']) { - return true; - } - } - - return false; - } - /** * @param string[] $files */ @@ -808,41 +757,23 @@ private function runDiagnoseExtensions(Container $container, Output $errorOutput } } - private function getCurrentBaselineIgnoreErrors(string $generateBaselineFile, InceptionResult $inceptionResult): mixed + /** + * @return mixed[][] + */ + private function getCurrentBaselinedErrors(string $generateBaselineFile, InceptionResult $inceptionResult): array { $loader = new Loader(); try { $currentBaselineConfig = $loader->load($generateBaselineFile); - $baselineIgnoreErrors = $currentBaselineConfig['parameters']['ignoreErrors'] ?? []; + $baselinedErrors = $currentBaselineConfig['parameters']['ignoreErrors'] ?? []; } catch (FileNotFoundException) { // currently no baseline file -> empty config - $baselineIgnoreErrors = []; + $baselinedErrors = []; } catch (InvalidStateException $invalidStateException) { $inceptionResult->getErrorOutput()->writeLineFormatted($invalidStateException->getMessage()); throw $invalidStateException; } - return $baselineIgnoreErrors; - } - - /** - * @param mixed[][] $baselineIgnoreErrors - * @return mixed[][] - */ - private function mapIgnoredErrors(array $baselineIgnoreErrors, FileHelper $fileHelper): array - { - $ignoreErrorsByFile = []; - - foreach ($baselineIgnoreErrors as $i => $ignoreError) { - $ignoreErrorEntry = [ - 'index' => $i, - 'ignoreError' => $ignoreError, - ]; - - $normalizedPath = $fileHelper->normalizePath($ignoreError['path']); - $ignoreErrorsByFile[$normalizedPath][] = $ignoreErrorEntry; - } - - return $ignoreErrorsByFile; + return $baselinedErrors; } } From 0f27eea7623db9d9f6b67505ea6e92a8eea068f6 Mon Sep 17 00:00:00 2001 From: N-Silbernagel <6422477+N-Silbernagel@users.noreply.github.com> Date: Thu, 20 Mar 2025 21:21:49 +0100 Subject: [PATCH 10/10] Add BaselineIgnoredErrorsHelperTest, remove AnalyseCommandTest cases --- build/collision-detector.json | 7 +- build/phpstan.neon | 1 - phpcs.xml | 1 - .../BaselineIgnoredErrorsHelperTest.php | 128 ++++++++++++++++++ tests/PHPStan/Command/AnalyseCommandTest.php | 101 +------------- .../data-ignore-new-errors-compare/A.php | 16 --- .../data-ignore-new-errors-compare/empty.neon | 0 .../Command/data-ignore-new-errors/A.php | 23 ---- .../Command/data-ignore-new-errors/B.php | 16 --- .../Command/data-ignore-new-errors/C.php | 16 --- .../Command/data-ignore-new-errors/empty.neon | 0 11 files changed, 133 insertions(+), 176 deletions(-) create mode 100644 tests/PHPStan/Analyser/Ignore/BaselineIgnoredErrorsHelperTest.php delete mode 100644 tests/PHPStan/Command/data-ignore-new-errors-compare/A.php delete mode 100644 tests/PHPStan/Command/data-ignore-new-errors-compare/empty.neon delete mode 100644 tests/PHPStan/Command/data-ignore-new-errors/A.php delete mode 100644 tests/PHPStan/Command/data-ignore-new-errors/B.php delete mode 100644 tests/PHPStan/Command/data-ignore-new-errors/C.php delete mode 100644 tests/PHPStan/Command/data-ignore-new-errors/empty.neon diff --git a/build/collision-detector.json b/build/collision-detector.json index ad8e022e1b..a687cd3ea4 100644 --- a/build/collision-detector.json +++ b/build/collision-detector.json @@ -15,9 +15,6 @@ "../tests/PHPStan/Rules/Functions/data/define-bug-3349.php", "../tests/PHPStan/Levels/data/stubs/function.php", "../tests/PHPStan/Rules/Properties/data/abstract-final-property-hook-parse-error.php", - "../tests/PHPStan/Rules/Properties/data/final-property-hooks.php", - "../tests/PHPStan/Levels/data/stubs/function.php", - "../tests/PHPStan/Command/data-ignore-new-errors/A.php", - "../tests/PHPStan/Command/data-ignore-new-errors-compare/A.php" - ] + "../tests/PHPStan/Rules/Properties/data/final-property-hooks.php" + ] } diff --git a/build/phpstan.neon b/build/phpstan.neon index 3fa881294d..1b1bf800f9 100644 --- a/build/phpstan.neon +++ b/build/phpstan.neon @@ -24,7 +24,6 @@ parameters: checkMissingCallableSignature: true excludePaths: - ../tests/*/data/* - - ../tests/*/data-*/* - ../tests/tmp/* - ../tests/PHPStan/Analyser/nsrt/* - ../tests/PHPStan/Analyser/traits/* diff --git a/phpcs.xml b/phpcs.xml index c9fc515426..fa9198745f 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -209,7 +209,6 @@ tests/*/Fixture/ tests/*/cache/ tests/*/data/ - tests/*/data-*/ tests/*/traits/ tests/PHPStan/Analyser/nsrt/ tests/e2e/anon-class/ diff --git a/tests/PHPStan/Analyser/Ignore/BaselineIgnoredErrorsHelperTest.php b/tests/PHPStan/Analyser/Ignore/BaselineIgnoredErrorsHelperTest.php new file mode 100644 index 0000000000..18ea62e9fe --- /dev/null +++ b/tests/PHPStan/Analyser/Ignore/BaselineIgnoredErrorsHelperTest.php @@ -0,0 +1,128 @@ +runRemoveUnusedIgnoredErrors( + [], + [ + new Error( + 'Foo', + __DIR__ . '/foo.php', + ), + ], + ); + + $this->assertCount(0, $result); + } + + public function testRemoveUnusedIgnoreError(): void + { + $result = $this->runRemoveUnusedIgnoredErrors( + [ + [ + 'message' => '#^Foo#', + 'count' => 1, + 'path' => 'foo.php', + ], + ], + [], + ); + + $this->assertCount(0, $result); + } + + public function testeReduceErrorCount(): void + { + $result = $this->runRemoveUnusedIgnoredErrors( + [ + [ + 'message' => '#^Foo#', + 'count' => 2, + 'path' => 'foo.php', + ], + ], + [ + new Error( + 'Foo', + __DIR__ . '/foo.php', + ), + ], + ); + + $this->assertCount(1, $result); + $this->assertSame('Foo', $result[0]->getMessage()); + $this->assertSame(__DIR__ . '/foo.php', $result[0]->getFilePath()); + } + + public function testNewError(): void + { + $result = $this->runRemoveUnusedIgnoredErrors( + [ + [ + 'message' => '#^Foo#', + 'count' => 1, + 'path' => 'foo.php', + ], + ], + [ + new Error( + 'Bar', + __DIR__ . '/bar.php', + ), + ], + ); + + $this->assertCount(0, $result); + } + + public function testIncreaseErrorCount(): void + { + $result = $this->runRemoveUnusedIgnoredErrors( + [ + [ + 'message' => '#^Foo#', + 'count' => 1, + 'path' => 'foo.php', + ], + ], + [ + new Error( + 'Foo', + __DIR__ . '/foo.php', + ), + new Error( + 'Foo', + __DIR__ . '/foo.php', + ), + ], + ); + + $this->assertCount(1, $result); + $this->assertSame('Foo', $result[0]->getMessage()); + $this->assertSame(__DIR__ . '/foo.php', $result[0]->getFilePath()); + } + + /** + * @param mixed[][] $baselinedErrors + * @param list $currentAnalysisErrors + * @return list errors + */ + private function runRemoveUnusedIgnoredErrors(array $baselinedErrors, array $currentAnalysisErrors): array + { + $baselineIgnoredErrorHelper = new BaselineIgnoredErrorHelper(self::getFileHelper()); + + $parentDirHelper = new ParentDirectoryRelativePathHelper(__DIR__); + + return $baselineIgnoredErrorHelper->removeUnusedIgnoredErrors($baselinedErrors, $currentAnalysisErrors, $parentDirHelper); + } + +} diff --git a/tests/PHPStan/Command/AnalyseCommandTest.php b/tests/PHPStan/Command/AnalyseCommandTest.php index f7299f6af3..f47edea747 100644 --- a/tests/PHPStan/Command/AnalyseCommandTest.php +++ b/tests/PHPStan/Command/AnalyseCommandTest.php @@ -6,14 +6,11 @@ use PHPStan\Testing\PHPStanTestCase; use Symfony\Component\Console\Tester\CommandTester; use Throwable; -use function array_merge; use function chdir; use function getcwd; use function microtime; use function realpath; -use function rename; use function sprintf; -use function unlink; use const DIRECTORY_SEPARATOR; use const PHP_EOL; @@ -74,98 +71,6 @@ public function testValidAutoloadFile(): void } } - public function testGenerateBaselineIgnoreNewErrorsRemoveFile(): void - { - $baselineFile = __DIR__ . '/data-ignore-new-errors/baseline.neon'; - $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php', __DIR__ . '/data-ignore-new-errors/B.php'], - '--configuration' => __DIR__ . '/data-ignore-new-errors/empty.neon', - '--level' => '9', - '--generate-baseline' => $baselineFile, - ]); - - $output = $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors/B.php', __DIR__ . '/data-ignore-new-errors/C.php'], - '--configuration' => $baselineFile, - '--level' => '9', - '--generate-baseline' => $baselineFile, - '--only-remove-errors' => true, - ]); - @unlink($baselineFile); - - $this->assertStringContainsString('[OK] Baseline generated with 1 error', $output); - } - - public function testGenerateBaselineIgnoreNewErrorsReducedErrorCount(): void - { - $baselineFile = __DIR__ . '/data-ignore-new-errors-compare/baseline.neon'; - $baselineFileSecondRun = __DIR__ . '/data-ignore-new-errors/baseline.neon'; - $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors-compare/A.php'], - '--configuration' => __DIR__ . '/data-ignore-new-errors-compare/empty.neon', - '--level' => '9', - '--generate-baseline' => $baselineFile, - ]); - - rename($baselineFile, $baselineFileSecondRun); - $output = $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php'], - '--configuration' => $baselineFileSecondRun, - '--level' => '9', - '--generate-baseline' => $baselineFileSecondRun, - '--only-remove-errors' => true, - ]); - @unlink($baselineFileSecondRun); - - $this->assertStringContainsString('[OK] Baseline generated with 2 errors', $output); - } - - public function testGenerateBaselineIgnoreNewErrorsIncreasedErrorCount(): void - { - $baselineFile = __DIR__ . '/data-ignore-new-errors/baseline.neon'; - $baselineFileSecondRun = __DIR__ . '/data-ignore-new-errors-compare/baseline.neon'; - $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php'], - '--configuration' => __DIR__ . '/data-ignore-new-errors/empty.neon', - '--level' => '9', - '--generate-baseline' => $baselineFile, - ]); - - rename($baselineFile, $baselineFileSecondRun); - $output = $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors-compare/A.php'], - '--configuration' => $baselineFileSecondRun, - '--level' => '9', - '--generate-baseline' => $baselineFileSecondRun, - '--only-remove-errors' => true, - ]); - @unlink($baselineFileSecondRun); - - $this->assertStringContainsString('[OK] Baseline generated with 2 errors', $output); - } - - public function testGenerateBaselineIgnoreNewErrorsEmptyBaseline(): void - { - $baselineFile = __DIR__ . '/data-ignore-new-errors/baseline.neon'; - $this->runCommand(0, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors/A.php', __DIR__ . '/data-ignore-new-errors/B.php'], - '--configuration' => __DIR__ . '/data-ignore-new-errors/empty.neon', - '--level' => '9', - '--generate-baseline' => $baselineFile, - ]); - - $output = $this->runCommand(1, [ - 'paths' => [__DIR__ . '/data-ignore-new-errors/C.php'], - '--configuration' => $baselineFile, - '--level' => '9', - '--generate-baseline' => $baselineFile, - '--only-remove-errors' => true, - ]); - @unlink($baselineFile); - - $this->assertStringContainsString('[ERROR] No errors were found during the analysis.', $output); - } - /** * @return string[][] */ @@ -212,16 +117,16 @@ public static function autoDiscoveryPathsProvider(): array } /** - * @param array $parameters + * @param array $parameters */ private function runCommand(int $expectedStatusCode, array $parameters = []): string { $commandTester = new CommandTester(new AnalyseCommand([], microtime(true))); - $commandTester->execute(array_merge([ + $commandTester->execute([ 'paths' => [__DIR__ . DIRECTORY_SEPARATOR . 'test'], '--debug' => true, - ], $parameters), ['debug' => true]); + ] + $parameters, ['debug' => true]); $this->assertSame($expectedStatusCode, $commandTester->getStatusCode(), $commandTester->getDisplay()); diff --git a/tests/PHPStan/Command/data-ignore-new-errors-compare/A.php b/tests/PHPStan/Command/data-ignore-new-errors-compare/A.php deleted file mode 100644 index 59ae2c9e8e..0000000000 --- a/tests/PHPStan/Command/data-ignore-new-errors-compare/A.php +++ /dev/null @@ -1,16 +0,0 @@ -> - */ - public function doFoo(): array - { - return [['foo']]; - } - - public function doBar(): void - { - array_key_first(); - } - -} diff --git a/tests/PHPStan/Command/data-ignore-new-errors/B.php b/tests/PHPStan/Command/data-ignore-new-errors/B.php deleted file mode 100644 index 684afd5a8e..0000000000 --- a/tests/PHPStan/Command/data-ignore-new-errors/B.php +++ /dev/null @@ -1,16 +0,0 @@ -> - */ - public function doFoo(): array - { - return [['foo']]; - } - -} diff --git a/tests/PHPStan/Command/data-ignore-new-errors/C.php b/tests/PHPStan/Command/data-ignore-new-errors/C.php deleted file mode 100644 index 7dfac0fb01..0000000000 --- a/tests/PHPStan/Command/data-ignore-new-errors/C.php +++ /dev/null @@ -1,16 +0,0 @@ -> - */ - public function doFoo(): array - { - return [['foo']]; - } - -} diff --git a/tests/PHPStan/Command/data-ignore-new-errors/empty.neon b/tests/PHPStan/Command/data-ignore-new-errors/empty.neon deleted file mode 100644 index e69de29bb2..0000000000