diff --git a/composer.json b/composer.json index 24821c3c0e4..3bf0a5d323e 100644 --- a/composer.json +++ b/composer.json @@ -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", diff --git a/conf/config.neon b/conf/config.neon index f72ca4c3fd7..89fb66e7f8d 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -290,6 +290,11 @@ services: arguments: memoryLimitFile: %memoryLimitFile% + - + class: PHPStan\Command\IgnoredRegexValidator + arguments: + parser: @regexParser + - class: PHPStan\Dependency\DependencyDumper @@ -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 diff --git a/src/Analyser/Analyser.php b/src/Analyser/Analyser.php index 62b5aa3e2aa..4dfe22d54f9 100644 --- a/src/Analyser/Analyser.php +++ b/src/Analyser/Analyser.php @@ -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; @@ -25,6 +26,9 @@ class Analyser /** @var \PHPStan\PhpDoc\StubValidator */ private $stubValidator; + /** @var \PHPStan\Command\IgnoredRegexValidator */ + private $ignoredRegexValidator; + /** @var (string|mixed[])[] */ private $ignoreErrors; @@ -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)[] $ignoreErrors * @param bool $reportUnmatchedIgnoredErrors * @param int $internalErrorsCountLimit @@ -53,6 +58,7 @@ public function __construct( NodeScopeResolver $nodeScopeResolver, FileHelper $fileHelper, StubValidator $stubValidator, + IgnoredRegexValidator $ignoredRegexValidator, array $ignoreErrors, bool $reportUnmatchedIgnoredErrors, int $internalErrorsCountLimit @@ -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; @@ -87,9 +94,9 @@ public function analyse( ): array { $errors = []; - $otherIgnoreErrors = []; $ignoreErrorsByFile = []; + $warnings = []; foreach ($this->ignoreErrors as $i => $ignoreError) { try { if (is_array($ignoreError)) { @@ -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); @@ -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 $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 + ); } /** diff --git a/src/Analyser/Error.php b/src/Analyser/Error.php index 12cbda7229e..65b5382cbaf 100644 --- a/src/Analyser/Error.php +++ b/src/Analyser/Error.php @@ -26,6 +26,9 @@ class Error /** @var string|null */ private $tip; + /** @var bool */ + private $warning; + public function __construct( string $message, string $file, @@ -33,7 +36,8 @@ public function __construct( bool $canBeIgnored = true, ?string $filePath = null, ?string $traitFilePath = null, - ?string $tip = null + ?string $tip = null, + bool $warning = false ) { $this->message = $message; @@ -43,6 +47,7 @@ public function __construct( $this->filePath = $filePath; $this->traitFilePath = $traitFilePath; $this->tip = $tip; + $this->warning = $warning; } public function getMessage(): string @@ -101,4 +106,9 @@ public function withoutTip(): self ); } + public function isWarning(): bool + { + return $this->warning; + } + } diff --git a/src/Command/AnalyseApplication.php b/src/Command/AnalyseApplication.php index 70d04987416..8845ced3e87 100644 --- a/src/Command/AnalyseApplication.php +++ b/src/Command/AnalyseApplication.php @@ -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; } } @@ -148,6 +153,7 @@ static function (Node $node, Scope $scope) use (&$hasInferrablePropertyTypesFrom new AnalysisResult( $fileSpecificErrors, $notFileSpecificErrors, + $warnings, $defaultLevelUsed, $hasInferrablePropertyTypesFromConstructor, $projectConfigFile diff --git a/src/Command/AnalysisResult.php b/src/Command/AnalysisResult.php index 76c0742e8ea..e6e373ad010 100644 --- a/src/Command/AnalysisResult.php +++ b/src/Command/AnalysisResult.php @@ -13,6 +13,9 @@ class AnalysisResult /** @var string[] */ private $notFileSpecificErrors; + /** @var string[] */ + private $warnings; + /** @var bool */ private $defaultLevelUsed; @@ -25,6 +28,7 @@ class AnalysisResult /** * @param \PHPStan\Analyser\Error[] $fileSpecificErrors * @param string[] $notFileSpecificErrors + * @param string[] $warnings * @param bool $defaultLevelUsed * @param bool $hasInferrablePropertyTypesFromConstructor * @param string|null $projectConfigFile @@ -32,6 +36,7 @@ class AnalysisResult public function __construct( array $fileSpecificErrors, array $notFileSpecificErrors, + array $warnings, bool $defaultLevelUsed, bool $hasInferrablePropertyTypesFromConstructor, ?string $projectConfigFile @@ -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; @@ -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; diff --git a/src/Command/ErrorFormatter/TableErrorFormatter.php b/src/Command/ErrorFormatter/TableErrorFormatter.php index acb94e96d3e..15cdeb51ccd 100644 --- a/src/Command/ErrorFormatter/TableErrorFormatter.php +++ b/src/Command/ErrorFormatter/TableErrorFormatter.php @@ -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()) { @@ -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; } } diff --git a/src/Command/IgnoredRegexValidator.php b/src/Command/IgnoredRegexValidator.php new file mode 100644 index 00000000000..e86cd6fc650 --- /dev/null +++ b/src/Command/IgnoredRegexValidator.php @@ -0,0 +1,126 @@ +parser = $parser; + $this->typeStringResolver = $typeStringResolver; + } + + /** + * @param string $regex + * @return string[] + */ + public function getIgnoredTypes(string $regex): array + { + $regex = $this->removeDelimiters($regex); + + try { + /** @var TreeNode $ast */ + $ast = $this->parser->parse($regex); + } catch (\Hoa\Exception $e) { + return []; + } + + /** @var TreeNode|null $alternation */ + $alternation = $ast->getChild(0); + if ($alternation === null) { + return []; + } + + if ($alternation->getId() !== '#alternation') { + return []; + } + + $types = []; + foreach ($alternation->getChildren() as $child) { + $text = $this->getText($child); + if ($text === null) { + continue; + } + + $matches = Strings::match($text, '#^([a-zA-Z0-9]+)[,]?\s*#'); + if ($matches === null) { + continue; + } + + try { + $type = $this->typeStringResolver->resolve($matches[1], null); + } catch (\PHPStan\PhpDocParser\Parser\ParserException $e) { + continue; + } + + if ($type->describe(VerbosityLevel::typeOnly()) !== $matches[1]) { + continue; + } + + if ($type instanceof ObjectType) { + continue; + } + + $types[$type->describe(VerbosityLevel::typeOnly())] = $text; + } + + return $types; + } + + private function removeDelimiters(string $regex): string + { + $delimiter = substr($regex, 0, 1); + $endDelimiterPosition = strrpos($regex, $delimiter); + if ($endDelimiterPosition === false) { + throw new \PHPStan\ShouldNotHappenException(); + } + + return substr($regex, 1, $endDelimiterPosition - 1); + } + + private function getText(TreeNode $treeNode): ?string + { + if ($treeNode->getId() === 'token') { + return $treeNode->getValueValue(); + } + + if ($treeNode->getId() === '#concatenation') { + $fullText = ''; + foreach ($treeNode->getChildren() as $child) { + $text = $this->getText($child); + if ($text === null) { + continue; + } + + $fullText .= $text; + } + + if ($fullText === '') { + return null; + } + + return $fullText; + } + + return null; + } + +} diff --git a/src/Testing/ErrorFormatterTestCase.php b/src/Testing/ErrorFormatterTestCase.php index d0076e7932c..d89cdd92f47 100644 --- a/src/Testing/ErrorFormatterTestCase.php +++ b/src/Testing/ErrorFormatterTestCase.php @@ -78,6 +78,7 @@ protected function getAnalysisResult(int $numFileErrors, int $numGenericErrors): return new AnalysisResult( $fileErrors, $genericErrors, + [], false, false, null diff --git a/src/Testing/RuleTestCase.php b/src/Testing/RuleTestCase.php index 9c9310d2b5e..babe172debe 100644 --- a/src/Testing/RuleTestCase.php +++ b/src/Testing/RuleTestCase.php @@ -9,6 +9,7 @@ use PHPStan\Analyser\TypeSpecifier; use PHPStan\Broker\AnonymousClassNameHelper; use PHPStan\Cache\Cache; +use PHPStan\Command\IgnoredRegexValidator; use PHPStan\File\FileHelper; use PHPStan\File\FuzzyRelativePathHelper; use PHPStan\PhpDoc\PhpDocNodeResolver; @@ -80,12 +81,16 @@ private function getAnalyser(): Analyser $stubValidator = $this->createMock(StubValidator::class); $stubValidator->method('validate') ->willReturn([]); + $ignoredRegexValidator = $this->createMock(IgnoredRegexValidator::class); + $ignoredRegexValidator->method('getIgnoredTypes') + ->willReturn([]); $this->analyser = new Analyser( $fileAnalyser, $registry, $nodeScopeResolver, $fileHelper, $stubValidator, + $ignoredRegexValidator, [], true, 50 diff --git a/tests/PHPStan/Analyser/AnalyserTest.php b/tests/PHPStan/Analyser/AnalyserTest.php index 55e8556daa9..e6d36c61204 100644 --- a/tests/PHPStan/Analyser/AnalyserTest.php +++ b/tests/PHPStan/Analyser/AnalyserTest.php @@ -4,6 +4,7 @@ use PHPStan\Broker\AnonymousClassNameHelper; use PHPStan\Cache\Cache; +use PHPStan\Command\IgnoredRegexValidator; use PHPStan\File\FileHelper; use PHPStan\File\RelativePathHelper; use PHPStan\Parser\DirectParser; @@ -386,6 +387,7 @@ private function createAnalyser( $nodeScopeResolver, $fileHelper, $stubValidator, + self::getContainer()->getByType(IgnoredRegexValidator::class), $ignoreErrors, $reportUnmatchedIgnoredErrors, 50 diff --git a/tests/PHPStan/Command/AnalysisResultTest.php b/tests/PHPStan/Command/AnalysisResultTest.php index 8c2ae765f1b..8feb4269a6f 100644 --- a/tests/PHPStan/Command/AnalysisResultTest.php +++ b/tests/PHPStan/Command/AnalysisResultTest.php @@ -37,6 +37,7 @@ public function testErrorsAreSortedByFileNameAndLine(): void new Error('aa4', 'aaa', 16), ], [], + [], false, false, null diff --git a/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php b/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php index d028c34016c..5ca07c70752 100644 --- a/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php +++ b/tests/PHPStan/Command/ErrorFormatter/BaselineNeonErrorFormatterTest.php @@ -129,6 +129,7 @@ public function testFormatErrorMessagesRegexEscape(): void $result = new AnalysisResult( [new Error('Escape Regex with file # ~ \' ()', 'Testfile')], ['Escape Regex without file # ~ <> \' ()'], + [], false, false, null diff --git a/tests/PHPStan/Command/IgnoredRegexValidatorTest.php b/tests/PHPStan/Command/IgnoredRegexValidatorTest.php new file mode 100644 index 00000000000..a883a2a9e6d --- /dev/null +++ b/tests/PHPStan/Command/IgnoredRegexValidatorTest.php @@ -0,0 +1,89 @@ + 'null, array', + 'string' => 'string', + 'int' => 'int given', + ], + ], + [ + '#Parameter \#2 $destination of method Nette\\\\Application\\\\UI\\\\Component::redirect\(\) expects string|null, array|Foo|Bar given#', + [ + 'null' => 'null, array', + ], + ], + [ + '#Parameter \#2 $destination of method Nette\\\\Application\\\\UI\\\\Component::redirect\(\) expects string\|null, array\|string\|int given#', + [], + ], + [ + '#Invalid array key type array|string\.#', + [ + 'string' => 'string\\.', + ], + ], + [ + '#Invalid array key type array\|string\.#', + [], + ], + [ + '#Array (array) does not accept key resource|iterable\.#', + [ + 'iterable' => 'iterable\.', + ], + ], + [ + '#Parameter \#1 $i of method Levels\\\\AcceptTypes\\\\Foo::doBarArray\(\) expects array, array given.#', + [ + 'int' => 'int> given.', + ], + ], + [ + '#Parameter \#1 $i of method Levels\\\\AcceptTypes\\\\Foo::doBarArray\(\) expects array|callable, array given.#', + [ + 'callable' => 'callable, array 'int> given.', + ], + ], + ]; + } + + /** + * @dataProvider dataGetIgnoredTypes + * @param string $regex + * @param string[] $expectedTypes + */ + public function testGetIgnoredTypes(string $regex, array $expectedTypes): void + { + $grammar = new \Hoa\File\Read('hoa://Library/Regex/Grammar.pp'); + $parser = \Hoa\Compiler\Llk\Llk::load($grammar); + $validator = new IgnoredRegexValidator($parser, self::getContainer()->getByType(TypeStringResolver::class)); + $this->assertSame($expectedTypes, $validator->getIgnoredTypes($regex)); + } + +}