Skip to content

Commit

Permalink
RuleTestCase - fail on PHP warnings, notices etc.
Browse files Browse the repository at this point in the history
Co-authored-by: Ondrej Mirtes <ondrej@mirtes.cz>
  • Loading branch information
janedbal and ondrejmirtes committed Apr 24, 2024
1 parent 6277fb4 commit 204ab27
Show file tree
Hide file tree
Showing 17 changed files with 248 additions and 25 deletions.
2 changes: 1 addition & 1 deletion phpstan-baseline.neon
Expand Up @@ -47,7 +47,7 @@ parameters:
path: src/Analyser/MutatingScope.php

-
message: "#^Only numeric types are allowed in pre\\-increment, bool\\|float\\|int\\|string\\|null given\\.$#"
message: "#^Only numeric types are allowed in pre\\-increment, float\\|int\\|string\\|null given\\.$#"
count: 1
path: src/Analyser/MutatingScope.php

Expand Down
9 changes: 9 additions & 0 deletions src/Analyser/Analyser.php
Expand Up @@ -49,6 +49,10 @@ public function analyse(

/** @var list<Error> $errors */
$errors = [];
/** @var list<Error> $filteredPhpErrors */
$filteredPhpErrors = [];
/** @var list<Error> $allPhpErrors */
$allPhpErrors = [];

/** @var list<Error> $locallyIgnoredErrors */
$locallyIgnoredErrors = [];
Expand Down Expand Up @@ -77,6 +81,9 @@ public function analyse(
null,
);
$errors = array_merge($errors, $fileAnalyserResult->getErrors());
$filteredPhpErrors = array_merge($filteredPhpErrors, $fileAnalyserResult->getFilteredPhpErrors());
$allPhpErrors = array_merge($allPhpErrors, $fileAnalyserResult->getAllPhpErrors());

$locallyIgnoredErrors = array_merge($locallyIgnoredErrors, $fileAnalyserResult->getLocallyIgnoredErrors());
$linesToIgnore[$file] = $fileAnalyserResult->getLinesToIgnore();
$unmatchedLineIgnores[$file] = $fileAnalyserResult->getUnmatchedLineIgnores();
Expand Down Expand Up @@ -115,6 +122,8 @@ public function analyse(

return new AnalyserResult(
$errors,
$filteredPhpErrors,
$allPhpErrors,
$locallyIgnoredErrors,
$linesToIgnore,
$unmatchedLineIgnores,
Expand Down
20 changes: 20 additions & 0 deletions src/Analyser/AnalyserResult.php
Expand Up @@ -17,6 +17,8 @@ class AnalyserResult

/**
* @param list<Error> $unorderedErrors
* @param list<Error> $filteredPhpErrors
* @param list<Error> $allPhpErrors
* @param list<Error> $locallyIgnoredErrors
* @param array<string, LinesToIgnore> $linesToIgnore
* @param array<string, LinesToIgnore> $unmatchedLineIgnores
Expand All @@ -27,6 +29,8 @@ class AnalyserResult
*/
public function __construct(
private array $unorderedErrors,
private array $filteredPhpErrors,
private array $allPhpErrors,
private array $locallyIgnoredErrors,
private array $linesToIgnore,
private array $unmatchedLineIgnores,
Expand Down Expand Up @@ -72,6 +76,22 @@ public function getErrors(): array
return $this->errors;
}

/**
* @return list<Error>
*/
public function getFilteredPhpErrors(): array
{
return $this->filteredPhpErrors;
}

/**
* @return list<Error>
*/
public function getAllPhpErrors(): array
{
return $this->allPhpErrors;
}

/**
* @return list<Error>
*/
Expand Down
29 changes: 26 additions & 3 deletions src/Analyser/AnalyserResultFinalizer.php
Expand Up @@ -8,6 +8,7 @@
use PHPStan\BetterReflection\Reflector\Exception\IdentifierNotFound;
use PHPStan\Node\CollectedDataNode;
use PHPStan\Rules\Registry as RuleRegistry;
use function array_merge;
use function count;
use function sprintf;

Expand All @@ -27,12 +28,12 @@ public function __construct(
public function finalize(AnalyserResult $analyserResult, bool $onlyFiles): FinalizerResult
{
if (count($analyserResult->getCollectedData()) === 0) {
return $this->addUnmatchedIgnoredErrors($analyserResult, [], []);
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
}

$hasInternalErrors = count($analyserResult->getInternalErrors()) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
if ($hasInternalErrors) {
return $this->addUnmatchedIgnoredErrors($analyserResult, [], []);
return $this->addUnmatchedIgnoredErrors($this->mergeFilteredPhpErrors($analyserResult), [], []);
}

$nodeType = CollectedDataNode::class;
Expand Down Expand Up @@ -88,7 +89,9 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles): Final
}

return $this->addUnmatchedIgnoredErrors(new AnalyserResult(
$errors,
array_merge($errors, $analyserResult->getFilteredPhpErrors()),
[],
$analyserResult->getAllPhpErrors(),
$locallyIgnoredErrors,
$allLinesToIgnore,
$allUnmatchedLineIgnores,
Expand All @@ -101,6 +104,24 @@ public function finalize(AnalyserResult $analyserResult, bool $onlyFiles): Final
), $collectorErrors, $locallyIgnoredCollectorErrors);
}

private function mergeFilteredPhpErrors(AnalyserResult $analyserResult): AnalyserResult
{
return new AnalyserResult(
array_merge($analyserResult->getUnorderedErrors(), $analyserResult->getFilteredPhpErrors()),
[],
$analyserResult->getAllPhpErrors(),
$analyserResult->getLocallyIgnoredErrors(),
$analyserResult->getLinesToIgnore(),
$analyserResult->getUnmatchedLineIgnores(),
$analyserResult->getInternalErrors(),
$analyserResult->getCollectedData(),
$analyserResult->getDependencies(),
$analyserResult->getExportedNodes(),
$analyserResult->hasReachedInternalErrorsCountLimit(),
$analyserResult->getPeakMemoryUsageBytes(),
);
}

/**
* @param list<Error> $collectorErrors
* @param list<Error> $locallyIgnoredCollectorErrors
Expand Down Expand Up @@ -150,6 +171,8 @@ private function addUnmatchedIgnoredErrors(
return new FinalizerResult(
new AnalyserResult(
$errors,
$analyserResult->getFilteredPhpErrors(),
$analyserResult->getAllPhpErrors(),
$analyserResult->getLocallyIgnoredErrors(),
$analyserResult->getLinesToIgnore(),
$analyserResult->getUnmatchedLineIgnores(),
Expand Down
61 changes: 54 additions & 7 deletions src/Analyser/FileAnalyser.php
Expand Up @@ -16,7 +16,6 @@
use PHPStan\Parser\ParserErrorsException;
use PHPStan\Rules\Registry as RuleRegistry;
use function array_keys;
use function array_merge;
use function array_unique;
use function array_values;
use function count;
Expand All @@ -28,12 +27,23 @@
use function set_error_handler;
use function sprintf;
use const E_DEPRECATED;
use const E_ERROR;
use const E_NOTICE;
use const E_PARSE;
use const E_STRICT;
use const E_USER_ERROR;
use const E_USER_NOTICE;
use const E_USER_WARNING;
use const E_WARNING;

class FileAnalyser
{

/** @var list<Error> */
private array $collectedErrors = [];
private array $allPhpErrors = [];

/** @var list<Error> */
private array $filteredPhpErrors = [];

public function __construct(
private ScopeFactory $scopeFactory,
Expand Down Expand Up @@ -209,8 +219,6 @@ public function analyseFile(

$this->restoreCollectErrorsHandler();

$fileErrors = array_merge($fileErrors, $this->collectedErrors);

foreach ($linesToIgnore as $fileKey => $lines) {
if (count($lines) > 0) {
continue;
Expand All @@ -227,7 +235,17 @@ public function analyseFile(
unset($unmatchedLineIgnores[$fileKey]);
}

return new FileAnalyserResult($fileErrors, $locallyIgnoredErrors, $fileCollectedData, array_values(array_unique($fileDependencies)), $exportedNodes, $linesToIgnore, $unmatchedLineIgnores);
return new FileAnalyserResult(
$fileErrors,
$this->filteredPhpErrors,
$this->allPhpErrors,
$locallyIgnoredErrors,
$fileCollectedData,
array_values(array_unique($fileDependencies)),
$exportedNodes,
$linesToIgnore,
$unmatchedLineIgnores,
);
}

/**
Expand All @@ -249,13 +267,18 @@ private function getLinesToIgnoreFromTokens(array $nodes): array
*/
private function collectErrors(array $analysedFiles): void
{
$this->collectedErrors = [];
$this->filteredPhpErrors = [];
$this->allPhpErrors = [];
set_error_handler(function (int $errno, string $errstr, string $errfile, int $errline) use ($analysedFiles): bool {
if ((error_reporting() & $errno) === 0) {
// silence @ operator
return true;
}

$errorMessage = sprintf('%s: %s', $this->getErrorLabel($errno), $errstr);

$this->allPhpErrors[] = (new Error($errorMessage, $errfile, $errline, true))->withIdentifier('phpstan.php');

if ($errno === E_DEPRECATED) {
return true;
}
Expand All @@ -264,7 +287,7 @@ private function collectErrors(array $analysedFiles): void
return true;
}

$this->collectedErrors[] = (new Error($errstr, $errfile, $errline, true))->withIdentifier('phpstan.php');
$this->filteredPhpErrors[] = (new Error($errorMessage, $errfile, $errline, true))->withIdentifier('phpstan.php');

return true;
});
Expand All @@ -275,4 +298,28 @@ private function restoreCollectErrorsHandler(): void
restore_error_handler();
}

private function getErrorLabel(int $errno): string
{
switch ($errno) {
case E_ERROR:
return 'Fatal error';
case E_WARNING:
return 'Warning';
case E_PARSE:
return 'Parse error';
case E_NOTICE:
return 'Notice';
case E_USER_ERROR:
return 'User error (E_USER_ERROR)';
case E_USER_WARNING:
return 'User warning (E_USER_WARNING)';
case E_USER_NOTICE:
return 'User notice (E_USER_NOTICE)';
case E_STRICT:
return 'Strict error (E_STRICT)';
}

return 'Unknown PHP error';
}

}
20 changes: 20 additions & 0 deletions src/Analyser/FileAnalyserResult.php
Expand Up @@ -13,6 +13,8 @@ class FileAnalyserResult

/**
* @param list<Error> $errors
* @param list<Error> $filteredPhpErrors
* @param list<Error> $allPhpErrors
* @param list<Error> $locallyIgnoredErrors
* @param list<CollectedData> $collectedData
* @param list<string> $dependencies
Expand All @@ -22,6 +24,8 @@ class FileAnalyserResult
*/
public function __construct(
private array $errors,
private array $filteredPhpErrors,
private array $allPhpErrors,
private array $locallyIgnoredErrors,
private array $collectedData,
private array $dependencies,
Expand All @@ -40,6 +44,22 @@ public function getErrors(): array
return $this->errors;
}

/**
* @return list<Error>
*/
public function getFilteredPhpErrors(): array
{
return $this->filteredPhpErrors;
}

/**
* @return list<Error>
*/
public function getAllPhpErrors(): array
{
return $this->allPhpErrors;
}

/**
* @return list<Error>
*/
Expand Down
5 changes: 4 additions & 1 deletion src/Analyser/MutatingScope.php
Expand Up @@ -144,6 +144,7 @@
use function get_class;
use function implode;
use function in_array;
use function is_bool;
use function is_numeric;
use function is_string;
use function ltrim;
Expand Down Expand Up @@ -1540,7 +1541,9 @@ static function (): void {

foreach ($varScalars as $varValue) {
if ($node instanceof Expr\PreInc) {
++$varValue;
if (!is_bool($varValue)) {
++$varValue;
}
} elseif (is_numeric($varValue)) {
--$varValue;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Analyser/ResultCache/ResultCacheManager.php
Expand Up @@ -507,6 +507,8 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache

return new ResultCacheProcessResult(new AnalyserResult(
$flatErrors,
$analyserResult->getFilteredPhpErrors(),
$analyserResult->getAllPhpErrors(),
$flatLocallyIgnoredErrors,
$linesToIgnore,
$unmatchedLineIgnores,
Expand Down
9 changes: 7 additions & 2 deletions src/Command/AnalyseApplication.php
Expand Up @@ -89,6 +89,8 @@ public function analyse(
$stubErrors = $this->stubValidator->validate($projectStubFiles, $debug);
$intermediateAnalyserResult = new AnalyserResult(
array_merge($intermediateAnalyserResult->getUnorderedErrors(), $stubErrors),
$intermediateAnalyserResult->getFilteredPhpErrors(),
$intermediateAnalyserResult->getAllPhpErrors(),
$intermediateAnalyserResult->getLocallyIgnoredErrors(),
$intermediateAnalyserResult->getLinesToIgnore(),
$intermediateAnalyserResult->getUnmatchedLineIgnores(),
Expand All @@ -104,7 +106,10 @@ public function analyse(
$resultCacheResult = $resultCacheManager->process($intermediateAnalyserResult, $resultCache, $errorOutput, $onlyFiles, true);
$analyserResult = $this->analyserResultFinalizer->finalize($resultCacheResult->getAnalyserResult(), $onlyFiles)->getAnalyserResult();
$internalErrors = $analyserResult->getInternalErrors();
$errors = $analyserResult->getErrors();
$errors = array_merge(
$analyserResult->getErrors(),
$analyserResult->getFilteredPhpErrors(),
);
$hasInternalErrors = count($internalErrors) > 0 || $analyserResult->hasReachedInternalErrorsCountLimit();
$memoryUsageBytes = $analyserResult->getPeakMemoryUsageBytes();
$isResultCacheUsed = !$resultCache->isFullAnalysis();
Expand Down Expand Up @@ -181,7 +186,7 @@ private function runAnalyser(
$errorOutput->getStyle()->progressStart($allAnalysedFilesCount);
$errorOutput->getStyle()->progressAdvance($allAnalysedFilesCount);
$errorOutput->getStyle()->progressFinish();
return new AnalyserResult([], [], [], [], [], [], [], [], false, memory_get_peak_usage(true));
return new AnalyserResult([], [], [], [], [], [], [], [], [], [], false, memory_get_peak_usage(true));
}

if (!$debug) {
Expand Down
2 changes: 1 addition & 1 deletion src/Command/AnalyserRunner.php
Expand Up @@ -47,7 +47,7 @@ public function runAnalyser(
{
$filesCount = count($files);
if ($filesCount === 0) {
return new AnalyserResult([], [], [], [], [], [], [], [], false, memory_get_peak_usage(true));
return new AnalyserResult([], [], [], [], [], [], [], [], [], [], false, memory_get_peak_usage(true));
}

$schedule = $this->scheduler->scheduleWork($this->cpuCoreCounter->getNumberOfCpuCores(), $files);
Expand Down
2 changes: 1 addition & 1 deletion src/Command/FixerWorkerCommand.php
Expand Up @@ -321,7 +321,7 @@ private function runAnalyser(LoopInterface $loop, Container $container, array $f
$parallelAnalyser = $container->getByType(ParallelAnalyser::class);
$filesCount = count($files);
if ($filesCount === 0) {
return resolve(new AnalyserResult([], [], [], [], [], [], [], [], false, memory_get_peak_usage(true)));
return resolve(new AnalyserResult([], [], [], [], [], [], [], [], [], [], false, memory_get_peak_usage(true)));
}

/** @var Scheduler $scheduler */
Expand Down

0 comments on commit 204ab27

Please sign in to comment.