Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --ignore-new-errors analyse command option #3847

Open
wants to merge 7 commits into
base: 2.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions build/collision-detector.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
"../tests/PHPStan/Rules/Names/data/no-namespace.php",
"../tests/notAutoloaded",
"../tests/PHPStan/Rules/Functions/data/define-bug-3349.php",
"../tests/PHPStan/Levels/data/stubs/function.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"
]
}
1 change: 1 addition & 0 deletions build/phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ parameters:
checkMissingCallableSignature: true
excludePaths:
- ../tests/*/data/*
- ../tests/*/data-*/*
- ../tests/tmp/*
- ../tests/PHPStan/Analyser/nsrt/*
- ../tests/PHPStan/Analyser/traits/*
Expand Down
1 change: 1 addition & 0 deletions phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@
<exclude-pattern>tests/*/Fixture/</exclude-pattern>
<exclude-pattern>tests/*/cache/</exclude-pattern>
<exclude-pattern>tests/*/data/</exclude-pattern>
<exclude-pattern>tests/*/data-*/</exclude-pattern>
<exclude-pattern>tests/*/traits/</exclude-pattern>
<exclude-pattern>tests/PHPStan/Analyser/nsrt/</exclude-pattern>
<exclude-pattern>tests/e2e/anon-class/</exclude-pattern>
Expand Down
135 changes: 132 additions & 3 deletions src/Command/AnalyseCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@

namespace PHPStan\Command;

use Nette\DI\Config\Loader;
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;
use PHPStan\Command\ErrorFormatter\BaselinePhpErrorFormatter;
Expand Down Expand Up @@ -102,6 +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.'),
]);
}

Expand Down Expand Up @@ -136,6 +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');

/** @var string|false|null $generateBaselineFile */
$generateBaselineFile = $input->getOption('generate-baseline');
Expand Down Expand Up @@ -182,6 +189,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');

Expand Down Expand Up @@ -411,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);
return $this->generateBaseline($generateBaselineFile, $inceptionResult, $analysisResult, $output, $allowEmptyBaseline, $baselineExtension, $failWithoutResultCache, $ignoreNewErrors, $container);
}

/** @var ErrorFormatter $errorFormatter */
Expand Down Expand Up @@ -587,8 +599,15 @@ 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);
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.');
$inceptionResult->getStdOutput()->writeLineFormatted('To allow generating empty baselines, pass <fg=cyan>--allow-empty-baseline</> option.');
Expand All @@ -599,7 +618,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') {
Expand Down Expand Up @@ -674,6 +692,80 @@ 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
{
$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;
}

$traitFilePath = $error->getTraitFilePath();
if ($traitFilePath !== null) {
$hasMatchingIgnore = $this->checkIgnoreErrorByPath($traitFilePath, $ignoreErrorsByFile, $fileHelper, $error, $ignoreUseCount, $baselinePathHelper);
if ($hasMatchingIgnore) {
continue;
}
}

// 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 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
*/
Expand Down Expand Up @@ -716,4 +808,41 @@ 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 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;
}

}
101 changes: 98 additions & 3 deletions tests/PHPStan/Command/AnalyseCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -71,6 +74,98 @@ 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 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,
'--ignore-new-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,
'--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.', $output);
}

/**
* @return string[][]
*/
Expand Down Expand Up @@ -117,16 +212,16 @@ public static function autoDiscoveryPathsProvider(): array
}

/**
* @param array<string, string> $parameters
* @param array<string, string|string[]|bool> $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());

Expand Down
16 changes: 16 additions & 0 deletions tests/PHPStan/Command/data-ignore-new-errors-compare/A.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php declare(strict_types = 1);

namespace BaselineIntegration;

use function array_key_first;

class A
{

public function doBar(): void
{
array_key_first();
array_key_first();
}

}
Empty file.
23 changes: 23 additions & 0 deletions tests/PHPStan/Command/data-ignore-new-errors/A.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php declare(strict_types = 1);

namespace BaselineIntegration;

use function array_key_first;

class A
{

/**
* @return array<array<int>>
*/
public function doFoo(): array
{
return [['foo']];
}

public function doBar(): void
{
array_key_first();
}

}
Loading
Loading