Skip to content

Commit

Permalink
Report unintended ignoreErrors regexes as warnings
Browse files Browse the repository at this point in the history
  • Loading branch information
ondrejmirtes committed Jan 24, 2020
1 parent ded4467 commit bc8d39d
Show file tree
Hide file tree
Showing 14 changed files with 340 additions and 7 deletions.
5 changes: 4 additions & 1 deletion composer.json
Expand Up @@ -7,13 +7,16 @@
"require": {
"php": "^7.1",
"composer/xdebug-handler": "^1.3.0",
"hoa/compiler": "3.17.08.08",
"hoa/exception": "^1.0",
"hoa/regex": "1.17.01.13",
"jean85/pretty-package-versions": "^1.0.3",
"nette/bootstrap": "^3.0",
"nette/di": "^3.0",
"nette/neon": "^3.0",
"nette/robot-loader": "^3.0.1",
"nette/schema": "^1.0",
"nette/utils": "^3.0",
"nette/utils": "^3.1.1",
"nikic/php-parser": "^4.3.0",
"ondram/ci-detector": "^3.1",
"ondrejmirtes/better-reflection": "^3.5.6",
Expand Down
15 changes: 15 additions & 0 deletions conf/config.neon
Expand Up @@ -290,6 +290,11 @@ services:
arguments:
memoryLimitFile: %memoryLimitFile%

-
class: PHPStan\Command\IgnoredRegexValidator
arguments:
parser: @regexParser

-
class: PHPStan\Dependency\DependencyDumper

Expand Down Expand Up @@ -977,6 +982,16 @@ services:
constantReflector: @betterReflectionConstantReflector
autowired: false

regexParser:
class: Hoa\Compiler\Llk\Parser
factory: Hoa\Compiler\Llk\Llk::load(@regexGrammarStream)

regexGrammarStream:
class: Hoa\File\Read
arguments:
streamName: 'hoa://Library/Regex/Grammar.pp'
mode: rb

runtimeReflectionProvider:
class: PHPStan\Reflection\Runtime\RuntimeReflectionProvider
autowired: false
Expand Down
43 changes: 41 additions & 2 deletions src/Analyser/Analyser.php
Expand Up @@ -3,6 +3,7 @@
namespace PHPStan\Analyser;

use Nette\Utils\Json;
use PHPStan\Command\IgnoredRegexValidator;
use PHPStan\File\FileHelper;
use PHPStan\PhpDoc\StubValidator;
use PHPStan\Rules\Registry;
Expand All @@ -25,6 +26,9 @@ class Analyser
/** @var \PHPStan\PhpDoc\StubValidator */
private $stubValidator;

/** @var \PHPStan\Command\IgnoredRegexValidator */
private $ignoredRegexValidator;

/** @var (string|mixed[])[] */
private $ignoreErrors;

Expand All @@ -43,6 +47,7 @@ class Analyser
* @param \PHPStan\Analyser\NodeScopeResolver $nodeScopeResolver
* @param \PHPStan\File\FileHelper $fileHelper
* @param \PHPStan\PhpDoc\StubValidator $stubValidator
* @param \PHPStan\Command\IgnoredRegexValidator $ignoredRegexValidator
* @param (string|array<string, string>)[] $ignoreErrors
* @param bool $reportUnmatchedIgnoredErrors
* @param int $internalErrorsCountLimit
Expand All @@ -53,6 +58,7 @@ public function __construct(
NodeScopeResolver $nodeScopeResolver,
FileHelper $fileHelper,
StubValidator $stubValidator,
IgnoredRegexValidator $ignoredRegexValidator,
array $ignoreErrors,
bool $reportUnmatchedIgnoredErrors,
int $internalErrorsCountLimit
Expand All @@ -63,6 +69,7 @@ public function __construct(
$this->nodeScopeResolver = $nodeScopeResolver;
$this->fileHelper = $fileHelper;
$this->stubValidator = $stubValidator;
$this->ignoredRegexValidator = $ignoredRegexValidator;
$this->ignoreErrors = $ignoreErrors;
$this->reportUnmatchedIgnoredErrors = $reportUnmatchedIgnoredErrors;
$this->internalErrorsCountLimit = $internalErrorsCountLimit;
Expand All @@ -87,9 +94,9 @@ public function analyse(
): array
{
$errors = [];

$otherIgnoreErrors = [];
$ignoreErrorsByFile = [];
$warnings = [];
foreach ($this->ignoreErrors as $i => $ignoreError) {
try {
if (is_array($ignoreError)) {
Expand Down Expand Up @@ -126,12 +133,23 @@ public function analyse(
}

$ignoreMessage = $ignoreError['message'];
if (isset($ignoreError['count'])) {
continue; // ignoreError coming from baseline will be correct
}
$ignoredTypes = $this->ignoredRegexValidator->getIgnoredTypes($ignoreMessage);
if (count($ignoredTypes) > 0) {
$warnings[] = $this->createIgnoredTypesWarning($ignoreMessage, $ignoredTypes);
}
} else {
$otherIgnoreErrors[] = [
'index' => $i,
'ignoreError' => $ignoreError,
];
$ignoreMessage = $ignoreError;
$ignoredTypes = $this->ignoredRegexValidator->getIgnoredTypes($ignoreMessage);
if (count($ignoredTypes) > 0) {
$warnings[] = $this->createIgnoredTypesWarning($ignoreMessage, $ignoredTypes);
}
}

\Nette\Utils\Strings::match('', $ignoreMessage);
Expand Down Expand Up @@ -349,7 +367,28 @@ public function analyse(
$errors[] = sprintf('Reached internal errors count limit of %d, exiting...', $this->internalErrorsCountLimit);
}

return $errors;
return array_merge($errors, $warnings);
}

/**
* @param string $regex
* @param array<string, string> $ignoredTypes
* @return Error
*/
private function createIgnoredTypesWarning(string $regex, array $ignoredTypes): Error
{
return new Error(
sprintf("Ignored error %s has an unescaped '|' which leads to ignoring more errors than intended. Use '\\|' instead.\n%s", $regex, sprintf("It ignores all errors containing the following types:\n%s", implode("\n", array_map(static function (string $typeDescription): string {
return sprintf('* %s', $typeDescription);
}, array_keys($ignoredTypes))))),
'placeholder', // this value will never get used
null,
false,
null,
null,
null,
true
);
}

/**
Expand Down
12 changes: 11 additions & 1 deletion src/Analyser/Error.php
Expand Up @@ -26,14 +26,18 @@ class Error
/** @var string|null */
private $tip;

/** @var bool */
private $warning;

public function __construct(
string $message,
string $file,
?int $line = null,
bool $canBeIgnored = true,
?string $filePath = null,
?string $traitFilePath = null,
?string $tip = null
?string $tip = null,
bool $warning = false
)
{
$this->message = $message;
Expand All @@ -43,6 +47,7 @@ public function __construct(
$this->filePath = $filePath;
$this->traitFilePath = $traitFilePath;
$this->tip = $tip;
$this->warning = $warning;
}

public function getMessage(): string
Expand Down Expand Up @@ -101,4 +106,9 @@ public function withoutTip(): self
);
}

public function isWarning(): bool
{
return $this->warning;
}

}
6 changes: 6 additions & 0 deletions src/Command/AnalyseApplication.php
Expand Up @@ -136,10 +136,15 @@ static function (Node $node, Scope $scope) use (&$hasInferrablePropertyTypesFrom

$fileSpecificErrors = [];
$notFileSpecificErrors = [];
$warnings = [];
foreach ($errors as $error) {
if (is_string($error)) {
$notFileSpecificErrors[] = $error;
} else {
if ($error->isWarning()) {
$warnings[] = $error->getMessage();
continue;
}
$fileSpecificErrors[] = $error;
}
}
Expand All @@ -148,6 +153,7 @@ static function (Node $node, Scope $scope) use (&$hasInferrablePropertyTypesFrom
new AnalysisResult(
$fileSpecificErrors,
$notFileSpecificErrors,
$warnings,
$defaultLevelUsed,
$hasInferrablePropertyTypesFromConstructor,
$projectConfigFile
Expand Down
19 changes: 19 additions & 0 deletions src/Command/AnalysisResult.php
Expand Up @@ -13,6 +13,9 @@ class AnalysisResult
/** @var string[] */
private $notFileSpecificErrors;

/** @var string[] */
private $warnings;

/** @var bool */
private $defaultLevelUsed;

Expand All @@ -25,13 +28,15 @@ class AnalysisResult
/**
* @param \PHPStan\Analyser\Error[] $fileSpecificErrors
* @param string[] $notFileSpecificErrors
* @param string[] $warnings
* @param bool $defaultLevelUsed
* @param bool $hasInferrablePropertyTypesFromConstructor
* @param string|null $projectConfigFile
*/
public function __construct(
array $fileSpecificErrors,
array $notFileSpecificErrors,
array $warnings,
bool $defaultLevelUsed,
bool $hasInferrablePropertyTypesFromConstructor,
?string $projectConfigFile
Expand All @@ -54,6 +59,7 @@ static function (Error $a, Error $b): int {

$this->fileSpecificErrors = $fileSpecificErrors;
$this->notFileSpecificErrors = $notFileSpecificErrors;
$this->warnings = $warnings;
$this->defaultLevelUsed = $defaultLevelUsed;
$this->hasInferrablePropertyTypesFromConstructor = $hasInferrablePropertyTypesFromConstructor;
$this->projectConfigFile = $projectConfigFile;
Expand Down Expand Up @@ -85,6 +91,19 @@ public function getNotFileSpecificErrors(): array
return $this->notFileSpecificErrors;
}

/**
* @return string[]
*/
public function getWarnings(): array
{
return $this->warnings;
}

public function hasWarnings(): bool
{
return count($this->warnings) > 0;
}

public function isDefaultLevelUsed(): bool
{
return $this->defaultLevelUsed;
Expand Down
22 changes: 19 additions & 3 deletions src/Command/ErrorFormatter/TableErrorFormatter.php
Expand Up @@ -67,7 +67,7 @@ public function formatErrors(
$output->writeLineFormatted('');
};

if (!$analysisResult->hasErrors()) {
if (!$analysisResult->hasErrors() && !$analysisResult->hasWarnings()) {
$style->success('No errors');
if ($this->showTipsOfTheDay) {
if ($analysisResult->isDefaultLevelUsed()) {
Expand Down Expand Up @@ -122,13 +122,29 @@ public function formatErrors(
}, $analysisResult->getNotFileSpecificErrors()));
}

$style->error(sprintf($analysisResult->getTotalErrorsCount() === 1 ? 'Found %d error' : 'Found %d errors', $analysisResult->getTotalErrorsCount()));
$warningsCount = count($analysisResult->getWarnings());
if ($warningsCount > 0) {
$style->table(['', 'Warning'], array_map(static function (string $warning): array {
return ['', $warning];
}, $analysisResult->getWarnings()));
}

$finalMessage = sprintf($analysisResult->getTotalErrorsCount() === 1 ? 'Found %d error' : 'Found %d errors', $analysisResult->getTotalErrorsCount());
if ($warningsCount > 0) {
$finalMessage .= sprintf($warningsCount === 1 ? ' and %d warning' : ' and %d warnings', $warningsCount);
}

if ($analysisResult->getTotalErrorsCount() > 0) {
$style->error($finalMessage);
} else {
$style->warning($finalMessage);
}

if ($this->checkMissingTypehints && $this->showTipsOfTheDay) {
$showInferPropertiesTip();
}

return 1;
return $analysisResult->getTotalErrorsCount() > 0 ? 1 : 0;
}

}

0 comments on commit bc8d39d

Please sign in to comment.