Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -2288,10 +2288,12 @@ public function processStmtNode(

// emit error
foreach ($matchingCatchTypes as $catchTypeIndex => $matched) {
if ($matched) {
// Matched (non-dead) catches only need to be reported to the rule
// when in a trait, to detect disagreement between the classes using it.
if ($matched && !$scope->isInTrait()) {
continue;
}
$this->callNodeCallback($nodeCallback, new CatchWithUnthrownExceptionNode($catchNode, $catchTypes[$catchTypeIndex], $originalCatchTypes[$catchTypeIndex]), $scope, $storage);
$this->callNodeCallback($nodeCallback, new CatchWithUnthrownExceptionNode($catchNode, $catchTypes[$catchTypeIndex], $originalCatchTypes[$catchTypeIndex], $matched), $scope, $storage);
}

if (count($matchingThrowPoints) === 0) {
Expand Down
10 changes: 9 additions & 1 deletion src/Node/CatchWithUnthrownExceptionNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
final class CatchWithUnthrownExceptionNode extends NodeAbstract implements VirtualNode
{

public function __construct(private Catch_ $originalNode, private Type $caughtType, private Type $originalCaughtType)
public function __construct(private Catch_ $originalNode, private Type $caughtType, private Type $originalCaughtType, private bool $matched)
{
parent::__construct($originalNode->getAttributes());
}
Expand All @@ -33,6 +33,14 @@ public function getOriginalCaughtType(): Type
return $this->originalCaughtType;
}

/**
* Whether this caught type is actually thrown in the try block, i.e. the catch is not dead.
*/
public function isMatched(): bool
{
return $this->matched;
}

#[Override]
public function getType(): string
{
Expand Down
43 changes: 38 additions & 5 deletions src/Rules/Comparison/ConstantConditionInTraitHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,35 @@ public function emitNoError(
Scope&NodeCallbackInvoker&CollectedDataEmitter $scope,
Expr $expr,
): void
{
$this->emitNoErrorForKey($ruleName, $scope, $this->exprString($expr));
}

/**
* @param class-string<Rule<covariant Node>> $ruleName
*/
public function emitError(
string $ruleName,
Scope&NodeCallbackInvoker&CollectedDataEmitter $scope,
Expr $expr,
bool $value,
RuleError $ruleError,
): void
{
$this->emitErrorForKey($ruleName, $scope, $expr, $this->exprString($expr), $value, $ruleError);
}

/**
* Like emitNoError(), but for callers that cannot key their check by a single Expr
* (e.g. one Rule node covering several distinct checks at the same location).
*
* @param class-string<Rule<covariant Node>> $ruleName
*/
public function emitNoErrorForKey(
string $ruleName,
Scope&NodeCallbackInvoker&CollectedDataEmitter $scope,
string $key,
): void
{
if (!$scope->isInTrait()) {
return;
Expand All @@ -48,18 +77,22 @@ public function emitNoError(
$scope->emitCollectedData(ConstantConditionInTraitCollector::class, [
$ruleName,
$scope->getTraitReflection()->getName(),
$this->exprString($expr),
$key,
null,
]);
}

/**
* Like emitError(), but for callers that cannot key their check by a single Expr
* (e.g. one Rule node covering several distinct checks at the same location).
*
* @param class-string<Rule<covariant Node>> $ruleName
*/
public function emitError(
public function emitErrorForKey(
string $ruleName,
Scope&NodeCallbackInvoker&CollectedDataEmitter $scope,
Expr $expr,
Node $node,
string $key,
bool $value,
RuleError $ruleError,
): void
Expand All @@ -75,9 +108,9 @@ public function emitError(
$scope->emitCollectedData(ConstantConditionInTraitCollector::class, [
$ruleName,
$scope->getTraitReflection()->getName(),
$this->exprString($expr),
$key,
$value,
$this->ruleErrorTransformer->transform($ruleError, $scope, [], $expr),
$this->ruleErrorTransformer->transform($ruleError, $scope, [], $node),
]);
}

Expand Down
66 changes: 42 additions & 24 deletions src/Rules/Exceptions/CatchWithUnthrownExceptionRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@
namespace PHPStan\Rules\Exceptions;

use PhpParser\Node;
use PHPStan\Analyser\CollectedDataEmitter;
use PHPStan\Analyser\NodeCallbackInvoker;
use PHPStan\Analyser\Scope;
use PHPStan\DependencyInjection\AutowiredParameter;
use PHPStan\DependencyInjection\RegisteredRule;
use PHPStan\Node\CatchWithUnthrownExceptionNode;
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Type\NeverType;
Expand All @@ -25,6 +28,7 @@ public function __construct(
private ExceptionTypeResolver $exceptionTypeResolver,
#[AutowiredParameter(ref: '%exceptions.reportUncheckedExceptionDeadCatch%')]
private bool $reportUncheckedExceptionDeadCatch,
private ConstantConditionInTraitHelper $constantConditionInTraitHelper,
)
{
}
Expand All @@ -34,41 +38,55 @@ public function getNodeType(): string
return CatchWithUnthrownExceptionNode::class;
}

public function processNode(Node $node, Scope $scope): array
public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
{
if ($node->getCaughtType() instanceof NeverType) {
return [
RuleErrorBuilder::message(
sprintf('Dead catch - %s is already caught above.', $node->getOriginalCaughtType()->describe(VerbosityLevel::typeOnly())),
)
->line($node->getStartLine())
->identifier('catch.alreadyCaught')
->build(),
];
// A trait's catch block can be dead in the context of one class using the trait
// and alive in the context of another, e.g. when it depends on whether an
// abstract method gets overridden. The key below identifies this specific catch
// type occurrence so its verdicts can be compared across all classes using the trait.
$key = sprintf('%s:%d', $node->getOriginalCaughtType()->describe(VerbosityLevel::typeOnly()), $node->getOriginalNode()->getStartLine());

if ($node->isMatched()) {
$this->constantConditionInTraitHelper->emitNoErrorForKey(self::class, $scope, $key);
return [];
}

if (!$this->reportUncheckedExceptionDeadCatch) {
$isCheckedException = false;
foreach ($node->getCaughtType()->getObjectClassNames() as $objectClassName) {
if ($this->exceptionTypeResolver->isCheckedException($objectClassName, $scope)) {
$isCheckedException = true;
break;
if ($node->getCaughtType() instanceof NeverType) {
$error = RuleErrorBuilder::message(
sprintf('Dead catch - %s is already caught above.', $node->getOriginalCaughtType()->describe(VerbosityLevel::typeOnly())),
)
->line($node->getStartLine())
->identifier('catch.alreadyCaught')
->build();
} else {
if (!$this->reportUncheckedExceptionDeadCatch) {
$isCheckedException = false;
foreach ($node->getCaughtType()->getObjectClassNames() as $objectClassName) {
if ($this->exceptionTypeResolver->isCheckedException($objectClassName, $scope)) {
$isCheckedException = true;
break;
}
}
}

if (!$isCheckedException) {
return [];
if (!$isCheckedException) {
return [];
}
}
}

return [
RuleErrorBuilder::message(
$error = RuleErrorBuilder::message(
sprintf('Dead catch - %s is never thrown in the try block.', $node->getCaughtType()->describe(VerbosityLevel::typeOnly())),
)
->line($node->getStartLine())
->identifier('catch.neverThrown')
->build(),
];
->build();
}

if ($scope->isInTrait()) {
$this->constantConditionInTraitHelper->emitErrorForKey(self::class, $scope, $node->getOriginalNode(), $key, true, $error);
return [];
}

return [$error];
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,37 @@

namespace PHPStan\Rules\Exceptions;

use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
use PHPStan\Rules\Comparison\ConstantConditionInTraitRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\CompositeRule;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\RequiresPhp;
use function array_merge;

/**
* @extends RuleTestCase<CatchWithUnthrownExceptionRule>
* @extends RuleTestCase<CompositeRule>
*/
class AbilityToDisableImplicitThrowsTest extends RuleTestCase
{

protected function getRule(): Rule
{
return new CatchWithUnthrownExceptionRule(new DefaultExceptionTypeResolver(
self::createReflectionProvider(),
[],
[],
[],
[],
), true);
// @phpstan-ignore argument.type
return new CompositeRule([
new CatchWithUnthrownExceptionRule(
new DefaultExceptionTypeResolver(
self::createReflectionProvider(),
[],
[],
[],
[],
),
true,
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
),
new ConstantConditionInTraitRule(),
]);
}

public function testRule(): void
Expand Down Expand Up @@ -97,6 +108,11 @@ public function testBug7799(): void
]);
}

public function testBug10315(): void
{
$this->analyse([__DIR__ . '/data/bug-10315.php'], []);
}

public static function getAdditionalConfigFiles(): array
{
return array_merge(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace PHPStan\Rules\Exceptions;

use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;

Expand All @@ -19,7 +20,7 @@ protected function getRule(): Rule
[],
[],
[],
), true);
), true, self::getContainer()->getByType(ConstantConditionInTraitHelper::class));
}

public function testRule(): void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@

use Error;
use InvalidArgumentException;
use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
use PHPStan\Rules\Comparison\ConstantConditionInTraitRule;
use PHPStan\Rules\Rule;
use PHPStan\Testing\CompositeRule;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\RequiresPhp;

/**
* @extends RuleTestCase<CatchWithUnthrownExceptionRule>
* @extends RuleTestCase<CompositeRule>
*/
class CatchWithUnthrownExceptionRuleTest extends RuleTestCase
{
Expand All @@ -21,13 +24,21 @@ class CatchWithUnthrownExceptionRuleTest extends RuleTestCase

protected function getRule(): Rule
{
return new CatchWithUnthrownExceptionRule(new DefaultExceptionTypeResolver(
self::createReflectionProvider(),
[],
$this->uncheckedExceptionClasses,
[],
[],
), $this->reportUncheckedExceptionDeadCatch);
// @phpstan-ignore argument.type
return new CompositeRule([
new CatchWithUnthrownExceptionRule(
new DefaultExceptionTypeResolver(
self::createReflectionProvider(),
[],
$this->uncheckedExceptionClasses,
[],
[],
),
$this->reportUncheckedExceptionDeadCatch,
self::getContainer()->getByType(ConstantConditionInTraitHelper::class),
),
new ConstantConditionInTraitRule(),
]);
}

public function testRule(): void
Expand Down Expand Up @@ -842,4 +853,9 @@ public function testBug9826(): void
$this->analyse([__DIR__ . '/data/bug-9826.php'], []);
}

public function testBug10315(): void
{
$this->analyse([__DIR__ . '/data/bug-10315.php'], []);
}

}
Loading
Loading