Skip to content

Commit

Permalink
Bleeding edge - consider implicit throw points when the only explicit…
Browse files Browse the repository at this point in the history
… one is Throw_
  • Loading branch information
ondrejmirtes committed Aug 21, 2024
1 parent 1fcae1c commit 22eef6d
Show file tree
Hide file tree
Showing 10 changed files with 80 additions and 7 deletions.
1 change: 1 addition & 0 deletions conf/bleedingEdge.neon
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,6 @@ parameters:
validatePregQuote: true
noImplicitWildcard: true
tooWidePropertyType: true
explicitThrow: true
stubFiles:
- ../stubs/bleedingEdge/Rule.stub
2 changes: 2 additions & 0 deletions conf/config.neon
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ parameters:
noImplicitWildcard: false
narrowPregMatches: true
tooWidePropertyType: false
explicitThrow: false
fileExtensions:
- php
checkAdvancedIsset: false
Expand Down Expand Up @@ -538,6 +539,7 @@ services:
universalObjectCratesClasses: %universalObjectCratesClasses%
paramOutType: %featureToggles.paramOutType%
preciseMissingReturn: %featureToggles.preciseMissingReturn%
explicitThrow: %featureToggles.explicitThrow%

-
class: PHPStan\Analyser\ConstantResolver
Expand Down
1 change: 1 addition & 0 deletions conf/parametersSchema.neon
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ parametersSchema:
noImplicitWildcard: bool()
narrowPregMatches: bool()
tooWidePropertyType: bool()
explicitThrow: bool()
])
fileExtensions: listOf(string())
checkAdvancedIsset: bool()
Expand Down
15 changes: 14 additions & 1 deletion src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ public function __construct(
private readonly bool $detectDeadTypeInMultiCatch,
private readonly bool $paramOutType,
private readonly bool $preciseMissingReturn,
private readonly bool $explicitThrow,
)
{
$earlyTerminatingMethodNames = [];
Expand Down Expand Up @@ -1545,6 +1546,7 @@ private function processStmtNode(
}

// explicit only
$onlyExplicitIsThrow = true;
if (count($matchingThrowPoints) === 0) {
foreach ($throwPoints as $throwPointIndex => $throwPoint) {
foreach ($catchTypes as $catchTypeIndex => $catchTypeItem) {
Expand All @@ -1556,13 +1558,24 @@ private function processStmtNode(
if (!$throwPoint->isExplicit()) {
continue;
}
$throwNode = $throwPoint->getNode();
if (
!$throwNode instanceof Throw_
&& !$throwNode instanceof Expr\Throw_
&& !($throwNode instanceof Node\Stmt\Expression && $throwNode->expr instanceof Expr\Throw_)
) {
$onlyExplicitIsThrow = false;
}
$matchingThrowPoints[$throwPointIndex] = $throwPoint;
}
}
}

// implicit only
if (count($matchingThrowPoints) === 0) {
if (
count($matchingThrowPoints) === 0
|| ($this->explicitThrow && $onlyExplicitIsThrow)
) {
foreach ($throwPoints as $throwPointIndex => $throwPoint) {
if ($throwPoint->isExplicit()) {
continue;
Expand Down
1 change: 1 addition & 0 deletions src/Testing/RuleTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ private function getAnalyser(DirectRuleRegistry $ruleRegistry): Analyser
self::getContainer()->getParameter('featureToggles')['detectDeadTypeInMultiCatch'],
self::getContainer()->getParameter('featureToggles')['paramOutType'],
self::getContainer()->getParameter('featureToggles')['preciseMissingReturn'],
self::getContainer()->getParameter('featureToggles')['explicitThrow'],
);
$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 @@ -87,6 +87,7 @@ public static function processFile(
self::getContainer()->getParameter('featureToggles')['detectDeadTypeInMultiCatch'],
self::getContainer()->getParameter('featureToggles')['paramOutType'],
self::getContainer()->getParameter('featureToggles')['preciseMissingReturn'],
self::getContainer()->getParameter('featureToggles')['explicitThrow'],
);
$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 @@ -743,6 +743,7 @@ private function createAnalyser(bool $enableIgnoreErrorsWithinPhpDocs): Analyser
self::getContainer()->getParameter('featureToggles')['detectDeadTypeInMultiCatch'],
self::getContainer()->getParameter('featureToggles')['paramOutType'],
self::getContainer()->getParameter('featureToggles')['preciseMissingReturn'],
self::getContainer()->getParameter('featureToggles')['explicitThrow'],
);
$lexer = new Lexer(['usedAttributes' => ['comments', 'startLine', 'endLine', 'startTokenPos', 'endTokenPos']]);
$fileAnalyser = new FileAnalyser(
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug-4879.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public function sayHello2(bool $bool1): void

$this->test();
} catch (\Exception $ex) {
assertVariableCertainty(TrinaryLogic::createNo(), $var);
assertVariableCertainty(TrinaryLogic::createMaybe(), $var);
}
}

Expand Down
53 changes: 53 additions & 0 deletions tests/PHPStan/Analyser/nsrt/explicit-throws.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

namespace ExplicitThrows;

use PHPStan\TrinaryLogic;
use function PHPStan\Testing\assertVariableCertainty;

class Foo
{

public function doFoo(): void
{
try {
doFoo();
$a = 1;
throw new \InvalidArgumentException();
} catch (\InvalidArgumentException $e) {
assertVariableCertainty(TrinaryLogic::createMaybe(), $a);
}
}

public function doBar(): void
{
try {
doFoo();
$a = 1;
$this->throwInvalidArgument();
} catch (\InvalidArgumentException $e) {
assertVariableCertainty(TrinaryLogic::createYes(), $a);
}
}

public function doBaz(): void
{
try {
doFoo();
$a = 1;
$this->throwInvalidArgument();
throw new \InvalidArgumentException();
} catch (\InvalidArgumentException $e) {
assertVariableCertainty(TrinaryLogic::createYes(), $a);
}
}

/**
* @throws \InvalidArgumentException
*/
private function throwInvalidArgument(): void
{

}

}
10 changes: 5 additions & 5 deletions tests/PHPStan/Analyser/nsrt/throw-points/try-catch.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ function (): void {
$bar = 1;
maybeThrows();
} catch (\InvalidArgumentException $e) {
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
assertType('1|2', $foo);

assertVariableCertainty(TrinaryLogic::createNo(), $bar);
assertVariableCertainty(TrinaryLogic::createMaybe(), $bar);
assertVariableCertainty(TrinaryLogic::createNo(), $baz);
} catch (\RuntimeException $e) {
assertVariableCertainty(TrinaryLogic::createNo(), $foo);
assertVariableCertainty(TrinaryLogic::createNo(), $bar);
assertVariableCertainty(TrinaryLogic::createYes(), $baz);
assertVariableCertainty(TrinaryLogic::createMaybe(), $bar);
assertVariableCertainty(TrinaryLogic::createMaybe(), $baz);
assertType('1|2', $baz);
} catch (\Throwable $e) {
assertType('Throwable~InvalidArgumentException|RuntimeException', $e);
Expand All @@ -99,7 +99,7 @@ function (): void {
throw new \InvalidArgumentException();
} catch (\InvalidArgumentException $e) {
assertType('1', $foo);
assertVariableCertainty(TrinaryLogic::createYes(), $foo);
assertVariableCertainty(TrinaryLogic::createMaybe(), $foo);
}
};

Expand Down

0 comments on commit 22eef6d

Please sign in to comment.