Skip to content

Commit

Permalink
Report dead types even in multi-exception catch
Browse files Browse the repository at this point in the history
[closes phpstan/phpstan#7312]


Co-authored-by: Jan Nedbal <janedbal@gmail.com>
  • Loading branch information
2 people authored and ondrejmirtes committed May 12, 2023
1 parent b206ea8 commit 7bb15fa
Show file tree
Hide file tree
Showing 12 changed files with 357 additions and 19 deletions.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,4 @@ parameters:
genericPrototypeMessage: true
stricterFunctionMap: true
invalidPhpDocTagLine: true
detectDeadTypeInMultiCatch: true
3 changes: 3 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ parameters:
genericPrototypeMessage: false
stricterFunctionMap: false
invalidPhpDocTagLine: false
detectDeadTypeInMultiCatch: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down Expand Up @@ -306,6 +307,7 @@ parametersSchema:
genericPrototypeMessage: bool()
stricterFunctionMap: bool()
invalidPhpDocTagLine: bool()
detectDeadTypeInMultiCatch: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down Expand Up @@ -664,6 +666,7 @@ services:
earlyTerminatingFunctionCalls: %earlyTerminatingFunctionCalls%
implicitThrows: %exceptions.implicitThrows%
treatPhpDocTypesAsCertain: %treatPhpDocTypesAsCertain%
detectDeadTypeInMultiCatch: %featureToggles.detectDeadTypeInMultiCatch%

-
class: PHPStan\Analyser\ConstantResolver
Expand Down
24 changes: 22 additions & 2 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,21 @@ parameters:

-
message: "#^Dead catch \\- PHPStan\\\\BetterReflection\\\\Identifier\\\\Exception\\\\InvalidIdentifierName is never thrown in the try block\\.$#"
count: 2
count: 3
path: src/Reflection/BetterReflection/BetterReflectionProvider.php

-
message: "#^Dead catch \\- PHPStan\\\\BetterReflection\\\\NodeCompiler\\\\Exception\\\\UnableToCompileNode\\|PHPStan\\\\BetterReflection\\\\Reflection\\\\Exception\\\\NotAClassReflection\\|PHPStan\\\\BetterReflection\\\\Reflection\\\\Exception\\\\NotAnInterfaceReflection is never thrown in the try block\\.$#"
message: "#^Dead catch \\- PHPStan\\\\BetterReflection\\\\NodeCompiler\\\\Exception\\\\UnableToCompileNode is never thrown in the try block\\.$#"
count: 1
path: src/Reflection/BetterReflection/BetterReflectionProvider.php

-
message: "#^Dead catch \\- PHPStan\\\\BetterReflection\\\\Reflection\\\\Exception\\\\NotAClassReflection is never thrown in the try block\\.$#"
count: 1
path: src/Reflection/BetterReflection/BetterReflectionProvider.php

-
message: "#^Dead catch \\- PHPStan\\\\BetterReflection\\\\Reflection\\\\Exception\\\\NotAnInterfaceReflection is never thrown in the try block\\.$#"
count: 1
path: src/Reflection/BetterReflection/BetterReflectionProvider.php

Expand Down Expand Up @@ -415,6 +425,11 @@ parameters:
count: 6
path: src/Reflection/InitializerExprTypeResolver.php

-
message: "#^Dead catch \\- PHPStan\\\\BetterReflection\\\\Identifier\\\\Exception\\\\InvalidIdentifierName is never thrown in the try block\\.$#"
count: 1
path: src/Reflection/SignatureMap/NativeFunctionReflectionProvider.php

-
message: "#^Creating new PHPStan\\\\Php8StubsMap is not covered by backward compatibility promise\\. The class might change in a minor PHPStan version\\.$#"
count: 1
Expand Down Expand Up @@ -1749,6 +1764,11 @@ parameters:
count: 1
path: tests/PHPStan/Reflection/SignatureMap/Php8SignatureMapProviderTest.php

-
message: "#^Dead catch \\- OutOfBoundsException is never thrown in the try block\\.$#"
count: 1
path: tests/PHPStan/Reflection/SignatureMap/SignatureMapParserTest.php

-
message: """
#^Instantiation of deprecated class PHPStan\\\\Rules\\\\Arrays\\\\AppendedArrayItemTypeRule\\:
Expand Down
80 changes: 63 additions & 17 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@
use function array_fill_keys;
use function array_filter;
use function array_key_exists;
use function array_keys;
use function array_map;
use function array_merge;
use function array_pop;
Expand Down Expand Up @@ -209,6 +210,7 @@ public function __construct(
private readonly array $earlyTerminatingFunctionCalls,
private readonly bool $implicitThrows,
private readonly bool $treatPhpDocTypesAsCertain,
private readonly bool $detectDeadTypeInMultiCatch,
)
{
$earlyTerminatingMethodNames = [];
Expand Down Expand Up @@ -1267,48 +1269,92 @@ private function processStmtNode(
foreach ($stmt->catches as $catchNode) {
$nodeCallback($catchNode, $scope);

$originalCatchType = TypeCombinator::union(...array_map(static fn (Name $name): Type => new ObjectType($name->toString()), $catchNode->types));
$isThrowable = $originalCatchType->isSuperTypeOf(new ObjectType(Throwable::class))->yes();
$catchType = TypeCombinator::remove($originalCatchType, $pastCatchTypes);
$originalCatchTypes = array_map(static fn (Name $name): Type => new ObjectType($name->toString()), $catchNode->types);
$catchTypes = array_map(static fn (Type $type): Type => TypeCombinator::remove($type, $pastCatchTypes), $originalCatchTypes);

$originalCatchType = TypeCombinator::union(...$originalCatchTypes);
$catchType = TypeCombinator::union(...$catchTypes);
$pastCatchTypes = TypeCombinator::union($pastCatchTypes, $originalCatchType);
$matchingThrowPoints = $isThrowable ? $throwPoints : [];

$matchingThrowPoints = [];
$matchingCatchTypes = array_fill_keys(array_keys($originalCatchTypes), false);

// throwable matches all
foreach ($originalCatchTypes as $catchTypeIndex => $catchTypeItem) {
if (!$catchTypeItem->isSuperTypeOf(new ObjectType(Throwable::class))->yes()) {
continue;
}

foreach ($throwPoints as $throwPointIndex => $throwPoint) {
$matchingThrowPoints[$throwPointIndex] = $throwPoint;
$matchingCatchTypes[$catchTypeIndex] = true;
}
}

// explicit only
if (count($matchingThrowPoints) === 0) {
foreach ($throwPoints as $throwPoint) {
if (!$throwPoint->isExplicit() || $catchType->isSuperTypeOf($throwPoint->getType())->no()) {
foreach ($throwPoints as $throwPointIndex => $throwPoint) {
if (!$throwPoint->isExplicit()) {
continue;
}

$matchingThrowPoints[] = $throwPoint;
foreach ($catchTypes as $catchTypeIndex => $catchTypeItem) {
if ($catchTypeItem->isSuperTypeOf($throwPoint->getType())->no()) {
continue;
}

$matchingThrowPoints[$throwPointIndex] = $throwPoint;
$matchingCatchTypes[$catchTypeIndex] = true;
}
}
}

// implicit only
if (count($matchingThrowPoints) === 0) {
foreach ($throwPoints as $throwPoint) {
if ($throwPoint->isExplicit() || $catchType->isSuperTypeOf($throwPoint->getType())->no()) {
foreach ($throwPoints as $throwPointIndex => $throwPoint) {
if ($throwPoint->isExplicit()) {
continue;
}

$matchingThrowPoints[] = $throwPoint;
foreach ($catchTypes as $catchTypeIndex => $catchTypeItem) {
if ($catchTypeItem->isSuperTypeOf($throwPoint->getType())->no()) {
continue;
}

$matchingThrowPoints[$throwPointIndex] = $throwPoint;
$matchingCatchTypes[$catchTypeIndex] = true;
}
}
}

// include previously removed throw points
if (count($matchingThrowPoints) === 0 && $isThrowable) {
foreach ($branchScopeResult->getThrowPoints() as $originalThrowPoint) {
if (!$originalThrowPoint->canContainAnyThrowable()) {
continue;
}
if (count($matchingThrowPoints) === 0) {
if ($originalCatchType->isSuperTypeOf(new ObjectType(Throwable::class))->yes()) {
foreach ($branchScopeResult->getThrowPoints() as $originalThrowPoint) {
if (!$originalThrowPoint->canContainAnyThrowable()) {
continue;
}

$matchingThrowPoints[] = $originalThrowPoint;
$matchingThrowPoints[] = $originalThrowPoint;
$matchingCatchTypes = array_fill_keys(array_keys($originalCatchTypes), true);
}
}
}

// emit error
if ($this->detectDeadTypeInMultiCatch) {
foreach ($matchingCatchTypes as $catchTypeIndex => $matched) {
if ($matched) {
continue;
}
$nodeCallback(new CatchWithUnthrownExceptionNode($catchNode, $catchTypes[$catchTypeIndex], $originalCatchTypes[$catchTypeIndex]), $scope);
}
}

if (count($matchingThrowPoints) === 0) {
$nodeCallback(new CatchWithUnthrownExceptionNode($catchNode, $catchType, $originalCatchType), $scope);
if (!$this->detectDeadTypeInMultiCatch) {
$nodeCallback(new CatchWithUnthrownExceptionNode($catchNode, $catchType, $originalCatchType), $scope);
}
continue;
}

Expand Down
3 changes: 3 additions & 0 deletions src/File/FileReader.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@
class FileReader
{

/**
* @throws CouldNotReadFileException
*/
public static function read(string $fileName): string
{
$path = $fileName;
Expand Down
1 change: 1 addition & 0 deletions src/Testing/RuleTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ private function getAnalyser(): Analyser
[],
true,
$this->shouldTreatPhpDocTypesAsCertain(),
self::getContainer()->getParameter('featureToggles')['detectDeadTypeInMultiCatch'],
);
$fileAnalyser = new FileAnalyser(
$this->createScopeFactory($reflectionProvider, $typeSpecifier),
Expand Down
1 change: 1 addition & 0 deletions src/Testing/TypeInferenceTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public static function processFile(
static::getEarlyTerminatingFunctionCalls(),
true,
self::getContainer()->getParameter('treatPhpDocTypesAsCertain'),
self::getContainer()->getParameter('featureToggles')['detectDeadTypeInMultiCatch'],
);
$resolver->setAnalysedFiles(array_map(static fn (string $file): string => $fileHelper->normalizePath($file), array_merge([$file], static::getAdditionalAnalysedFiles())));

Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/AnalyserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@ private function createAnalyser(bool $reportUnmatchedIgnoredErrors): Analyser
[],
true,
$this->shouldTreatPhpDocTypesAsCertain(),
self::getContainer()->getParameter('featureToggles')['detectDeadTypeInMultiCatch'],
);
$lexer = new Lexer(['usedAttributes' => ['comments', 'startLine', 'endLine', 'startTokenPos', 'endTokenPos']]);
$fileAnalyser = new FileAnalyser(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,36 @@ public function testRule(): void
]);
}

public function testMultiCatch(): void
{
$this->analyse([__DIR__ . '/data/unthrown-exception-multi.php'], [
[
'Dead catch - LogicException is never thrown in the try block.',
12,
],
[
'Dead catch - OverflowException is never thrown in the try block.',
36,
],
[
'Dead catch - JsonException is never thrown in the try block.',
58,
],
[
'Dead catch - RuntimeException is never thrown in the try block.',
120,
],
[
'Dead catch - InvalidArgumentException is already caught above.',
145,
],
[
'Dead catch - InvalidArgumentException is already caught above.',
156,
],
]);
}

public function testBug4806(): void
{
$this->analyse([__DIR__ . '/data/bug-4806.php'], [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php declare(strict_types = 1);

namespace PHPStan\Rules\Exceptions;

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use function array_merge;

/**
* @extends RuleTestCase<CatchWithUnthrownExceptionRule>
*/
class CatchWithUnthrownExceptionRuleWithDisabledMultiCatchTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new CatchWithUnthrownExceptionRule();
}

public static function getAdditionalConfigFiles(): array
{
return array_merge(
parent::getAdditionalConfigFiles(),
[__DIR__ . '/disable-detect-multi-catch.neon'],
);
}

public function testMultiCatchBackwardCompatible(): void
{
$this->analyse([__DIR__ . '/data/unthrown-exception-multi.php'], [
[
'Dead catch - InvalidArgumentException is already caught above.',
145,
],
[
'Dead catch - InvalidArgumentException is already caught above.',
156,
],
]);
}

}
Loading

0 comments on commit 7bb15fa

Please sign in to comment.