Avoid dead catch false positive for inconsistently overridden trait methods

src/Analyser/NodeScopeResolver.php

@@ -65,6 +65,7 @@
 use PHPStan\File\FileHelper;
 use PHPStan\File\FileReader;
 use PHPStan\Node\BreaklessWhileLoopNode;
+use PHPStan\Node\CatchWithThrownExceptionInTraitNode;
 use PHPStan\Node\CatchWithUnthrownExceptionNode;
 use PHPStan\Node\ClassConstantsNode;
 use PHPStan\Node\ClassMethodsNode;
@@ -2372,6 +2373,13 @@ public function processStmtNode(
 				// emit error
 				foreach ($matchingCatchTypes as $catchTypeIndex => $matched) {
 					if ($matched) {
+						// A trait's catch can be dead in the context of one class using the
+						// trait and alive in the context of another, so the alive ones have
+						// to be reported there as well for the disagreement to be noticed.
+						if ($scope->isInTrait()) {
+							$this->callNodeCallback($nodeCallback, new CatchWithThrownExceptionInTraitNode($catchNode, $originalCatchTypes[$catchTypeIndex]), $scope, $storage);
+						}
+
 						continue;
 					}
 					$this->callNodeCallback($nodeCallback, new CatchWithUnthrownExceptionNode($catchNode, $catchTypes[$catchTypeIndex], $originalCatchTypes[$catchTypeIndex]), $scope, $storage);

src/Node/CatchWithThrownExceptionInTraitNode.php

@@ -0,0 +1,51 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Node;
+
+use Override;
+use PhpParser\Node\Stmt\Catch_;
+use PhpParser\NodeAbstract;
+use PHPStan\Type\Type;
+
+/**
+ * A catch clause inside a trait whose caught type _is_ thrown in the try block.
+ *
+ * Emitted only in traits. A trait's catch can be dead in the context of one class using
+ * the trait and alive in the context of another, so CatchWithUnthrownExceptionRule has to
+ * learn about the alive ones as well to notice the disagreement. Dead catches keep being
+ * reported through CatchWithUnthrownExceptionNode.
+ */
+final class CatchWithThrownExceptionInTraitNode extends NodeAbstract implements VirtualNode
+{
+
+	public function __construct(private Catch_ $originalNode, private Type $originalCaughtType)
+	{
+		parent::__construct($originalNode->getAttributes());
+	}
+
+	public function getOriginalNode(): Catch_
+	{
+		return $this->originalNode;
+	}
+
+	public function getOriginalCaughtType(): Type
+	{
+		return $this->originalCaughtType;
+	}
+
+	#[Override]
+	public function getType(): string
+	{
+		return 'PHPStan_Node_CatchWithThrownExceptionInTraitNode';
+	}
+
+	/**
+	 * @return string[]
+	 */
+	#[Override]
+	public function getSubNodeNames(): array
+	{
+		return [];
+	}
+
+}

src/Rules/Comparison/ConstantConditionInTraitHelper.php

@@ -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;
@@ -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
@@ -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),
 		]);
 	}
 

src/Rules/Exceptions/CatchWithThrownExceptionInTraitRule.php

@@ -0,0 +1,45 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Exceptions;
+
+use PhpParser\Node;
+use PHPStan\Analyser\CollectedDataEmitter;
+use PHPStan\Analyser\NodeCallbackInvoker;
+use PHPStan\Analyser\Scope;
+use PHPStan\DependencyInjection\RegisteredRule;
+use PHPStan\Node\CatchWithThrownExceptionInTraitNode;
+use PHPStan\Rules\Comparison\ConstantConditionInTraitHelper;
+use PHPStan\Rules\Rule;
+
+/**
+ * Records that a catch clause in a trait is alive in the context of the current class,
+ * so that CatchWithUnthrownExceptionRule does not report it as dead based only on the
+ * classes using the trait where it happens to be unreachable.
+ *
+ * @implements Rule<CatchWithThrownExceptionInTraitNode>
+ */
+#[RegisteredRule(level: 4)]
+final class CatchWithThrownExceptionInTraitRule implements Rule
+{
+
+	public function __construct(private ConstantConditionInTraitHelper $constantConditionInTraitHelper)
+	{
+	}
+
+	public function getNodeType(): string
+	{
+		return CatchWithThrownExceptionInTraitNode::class;
+	}
+
+	public function processNode(Node $node, Scope&NodeCallbackInvoker&CollectedDataEmitter $scope): array
+	{
+		$this->constantConditionInTraitHelper->emitNoErrorForKey(
+			CatchWithUnthrownExceptionRule::class,
+			$scope,
+			DeadCatchInTraitKey::create($node->getOriginalNode(), $node->getOriginalCaughtType()),
+		);
+
+		return [];
+	}
+
+}

src/Rules/Exceptions/CatchWithUnthrownExceptionRule.php

@@ -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;
@@ -25,6 +28,7 @@ public function __construct(
 		private ExceptionTypeResolver $exceptionTypeResolver,
 		#[AutowiredParameter(ref: '%exceptions.reportUncheckedExceptionDeadCatch%')]
 		private bool $reportUncheckedExceptionDeadCatch,
+		private ConstantConditionInTraitHelper $constantConditionInTraitHelper,
 	)
 	{
 	}
@@ -34,41 +38,56 @@ 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(),
-			];
-		}
+			$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 (!$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()) {
+			// A trait's catch 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. Let the collector compare the verdicts of all the
+			// classes using the trait instead of reporting right away; the alive ones are
+			// recorded by CatchWithThrownExceptionInTraitRule under the same key.
+			$this->constantConditionInTraitHelper->emitErrorForKey(
+				self::class,
+				$scope,
+				$node->getOriginalNode(),
+				DeadCatchInTraitKey::create($node->getOriginalNode(), $node->getOriginalCaughtType()),
+				true,
+				$error,
+			);
+			return [];
+		}
+
+		return [$error];
 	}
 
 }

src/Rules/Exceptions/DeadCatchInTraitKey.php

@@ -0,0 +1,26 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Rules\Exceptions;
+
+use PhpParser\Node\Stmt\Catch_;
+use PHPStan\Type\Type;
+use function implode;
+use function sprintf;
+
+/**
+ * Identifies one caught type of one catch clause in a trait, so that the dead-catch
+ * verdicts collected from every class using the trait can be compared.
+ *
+ * The trait is parsed from the same file for every using class, so the caught type
+ * together with the line pins down the occurrence: `catch (A|B)` yields two keys,
+ * one per caught type.
+ */
+final class DeadCatchInTraitKey
+{
+
+	public static function create(Catch_ $catchNode, Type $originalCaughtType): string
+	{
+		return sprintf('%s:%d', implode('|', $originalCaughtType->getObjectClassNames()), $catchNode->getStartLine());
+	}
+
+}