From bfdbb891a0ea0c451e7c09b153eeeef04687c78c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Mor=C3=A1vek?= Date: Fri, 15 Sep 2023 13:13:26 +0200 Subject: [PATCH] Arrow function support for immediately called callable arguments (#160) --- ...iatelyCalledCallableThrowTypeExtension.php | 15 ++++ .../ForbidCheckedExceptionInCallableRule.php | 55 +++++++++++++++ .../ImmediatelyCalledCallableVisitor.php | 10 +-- .../code.php | 36 ++++++++++ ...rbidCheckedExceptionInCallableRuleTest.php | 2 + .../code.php | 70 +++++++++++++++++++ .../visitor.neon | 2 + 7 files changed, 186 insertions(+), 4 deletions(-) diff --git a/src/Extension/ImmediatelyCalledCallableThrowTypeExtension.php b/src/Extension/ImmediatelyCalledCallableThrowTypeExtension.php index 7b07bd3..996166e 100644 --- a/src/Extension/ImmediatelyCalledCallableThrowTypeExtension.php +++ b/src/Extension/ImmediatelyCalledCallableThrowTypeExtension.php @@ -3,6 +3,7 @@ namespace ShipMonk\PHPStan\Extension; use LogicException; +use PhpParser\Node\Expr\ArrowFunction; use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\FuncCall; @@ -150,6 +151,20 @@ static function (): void { } } + if ($argumentValue instanceof ArrowFunction) { + $result = $this->nodeScopeResolver->processStmtNodes( + $call, + $argumentValue->getStmts(), + $scope->enterArrowFunction($argumentValue), + static function (): void { + }, + ); + + foreach ($result->getThrowPoints() as $throwPoint) { + $throwTypes[] = $throwPoint->getType(); + } + } + if ($argumentValue instanceof StaticCall && $argumentValue->isFirstClassCallable() && $argumentValue->name instanceof Identifier diff --git a/src/Rule/ForbidCheckedExceptionInCallableRule.php b/src/Rule/ForbidCheckedExceptionInCallableRule.php index d25a06f..5e6f37c 100644 --- a/src/Rule/ForbidCheckedExceptionInCallableRule.php +++ b/src/Rule/ForbidCheckedExceptionInCallableRule.php @@ -5,12 +5,16 @@ use LogicException; use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\ArrowFunction; use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Identifier; use PhpParser\Node\Name; +use PHPStan\Analyser\ExpressionContext; +use PHPStan\Analyser\MutatingScope; +use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\Scope; use PHPStan\Node\ClosureReturnStatementsNode; use PHPStan\Node\FunctionCallableNode; @@ -36,6 +40,8 @@ class ForbidCheckedExceptionInCallableRule implements Rule { + private NodeScopeResolver $nodeScopeResolver; + private ReflectionProvider $reflectionProvider; private DefaultExceptionTypeResolver $exceptionTypeResolver; @@ -54,6 +60,7 @@ class ForbidCheckedExceptionInCallableRule implements Rule * @param array> $allowedCheckedExceptionCallables */ public function __construct( + NodeScopeResolver $nodeScopeResolver, ReflectionProvider $reflectionProvider, DefaultExceptionTypeResolver $exceptionTypeResolver, array $immediatelyCalledCallables, @@ -71,6 +78,7 @@ function ($argumentIndexes): array { ); $this->exceptionTypeResolver = $exceptionTypeResolver; $this->reflectionProvider = $reflectionProvider; + $this->nodeScopeResolver = $nodeScopeResolver; } public function getNodeType(): string @@ -98,6 +106,10 @@ public function processNode( return $this->processClosure($node, $scope); } + if ($node instanceof ArrowFunction) { + return $this->processArrowFunction($node, $scope); + } + return []; } @@ -180,6 +192,49 @@ public function processClosure( return $errors; } + /** + * @return list + */ + public function processArrowFunction( + ArrowFunction $node, + Scope $scope + ): array + { + if (!$scope instanceof MutatingScope) { // @phpstan-ignore-line ignore BC promise + throw new LogicException('Unexpected scope implementation'); + } + + if ($this->isAllowedToThrowCheckedException($node, $scope)) { + return []; + } + + $result = $this->nodeScopeResolver->processExprNode( // @phpstan-ignore-line ignore BC promise + $node->expr, + $scope->enterArrowFunction($node), + static function (): void { + }, + ExpressionContext::createDeep(), // @phpstan-ignore-line ignore BC promise + ); + + $errors = []; + + foreach ($result->getThrowPoints() as $throwPoint) { // @phpstan-ignore-line ignore BC promise + if (!$throwPoint->isExplicit()) { + continue; + } + + foreach ($throwPoint->getType()->getObjectClassNames() as $exceptionClass) { + if ($this->exceptionTypeResolver->isCheckedException($exceptionClass, $throwPoint->getScope())) { + $errors[] = RuleErrorBuilder::message("Throwing checked exception $exceptionClass in arrow function!") + ->line($throwPoint->getNode()->getLine()) + ->build(); + } + } + } + + return $errors; + } + /** * @return list */ diff --git a/src/Visitor/ImmediatelyCalledCallableVisitor.php b/src/Visitor/ImmediatelyCalledCallableVisitor.php index 9e856c0..f81017b 100644 --- a/src/Visitor/ImmediatelyCalledCallableVisitor.php +++ b/src/Visitor/ImmediatelyCalledCallableVisitor.php @@ -3,6 +3,7 @@ namespace ShipMonk\PHPStan\Visitor; use PhpParser\Node; +use PhpParser\Node\Expr\ArrowFunction; use PhpParser\Node\Expr\CallLike; use PhpParser\Node\Expr\Closure; use PhpParser\Node\Expr\FuncCall; @@ -102,7 +103,7 @@ private function resolveMethodCall(CallLike $node): void continue; } - if (!$this->isFirstClassCallableOrClosure($argument->value)) { + if (!$this->isFirstClassCallableOrClosureOrArrowFunction($argument->value)) { continue; } @@ -115,7 +116,7 @@ private function resolveMethodCall(CallLike $node): void private function resolveFuncCall(FuncCall $node): void { - if ($this->isFirstClassCallableOrClosure($node->name)) { + if ($this->isFirstClassCallableOrClosureOrArrowFunction($node->name)) { // phpcs:ignore Squiz.PHP.CommentedOutCode.Found $node->name->setAttribute(self::CALLABLE_ALLOWING_CHECKED_EXCEPTION, true); // immediately called closure syntax, e.g. (function(){})() return; @@ -139,7 +140,7 @@ private function resolveFuncCall(FuncCall $node): void continue; } - if (!$this->isFirstClassCallableOrClosure($argument->value)) { + if (!$this->isFirstClassCallableOrClosureOrArrowFunction($argument->value)) { continue; } @@ -147,9 +148,10 @@ private function resolveFuncCall(FuncCall $node): void } } - private function isFirstClassCallableOrClosure(Node $node): bool + private function isFirstClassCallableOrClosureOrArrowFunction(Node $node): bool { return $node instanceof Closure + || $node instanceof ArrowFunction || ($node instanceof MethodCall && $node->isFirstClassCallable()) || ($node instanceof NullsafeMethodCall && $node->isFirstClassCallable()) || ($node instanceof StaticCall && $node->isFirstClassCallable()) diff --git a/tests/Extension/data/ImmediatelyCalledCallableThrowTypeExtension/code.php b/tests/Extension/data/ImmediatelyCalledCallableThrowTypeExtension/code.php index 1089c49..5f15830 100644 --- a/tests/Extension/data/ImmediatelyCalledCallableThrowTypeExtension/code.php +++ b/tests/Extension/data/ImmediatelyCalledCallableThrowTypeExtension/code.php @@ -80,6 +80,24 @@ public function testClosureWithoutThrow(): void } } + public function testArrowFunction(): void + { + try { + $result = Immediate::method(fn () => throw new \Exception()); + } finally { + assertVariableCertainty(TrinaryLogic::createMaybe(), $result); + } + } + + public function testArrowFunctionWithoutThrow(): void + { + try { + $result = Immediate::method(fn () => 42); + } finally { + assertVariableCertainty(TrinaryLogic::createYes(), $result); + } + } + public function testFirstClassCallable(): void { try { @@ -181,6 +199,24 @@ public function testClosureWithoutThrow(): void } } + public function testArrowFunction(): void + { + try { + $result = array_map(fn () => throw new \Exception(), []); + } finally { + assertVariableCertainty(TrinaryLogic::createMaybe(), $result); + } + } + + public function testArrowFunctionWithoutThrow(): void + { + try { + $result = array_map(fn () => 42, []); + } finally { + assertVariableCertainty(TrinaryLogic::createYes(), $result); + } + } + public function testFirstClassCallable(): void { try { diff --git a/tests/Rule/ForbidCheckedExceptionInCallableRuleTest.php b/tests/Rule/ForbidCheckedExceptionInCallableRuleTest.php index 678cdfa..00fe662 100644 --- a/tests/Rule/ForbidCheckedExceptionInCallableRuleTest.php +++ b/tests/Rule/ForbidCheckedExceptionInCallableRuleTest.php @@ -4,6 +4,7 @@ use LogicException; use Nette\Neon\Neon; +use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Exceptions\DefaultExceptionTypeResolver; use PHPStan\Rules\Rule; @@ -35,6 +36,7 @@ protected function getRule(): Rule $visitorConfig = Neon::decodeFile(self::getVisitorConfigFilePath()); return new ForbidCheckedExceptionInCallableRule( + self::getContainer()->getByType(NodeScopeResolver::class), self::getContainer()->getByType(ReflectionProvider::class), new DefaultExceptionTypeResolver( // @phpstan-ignore-line ignore BC promise self::getContainer()->getByType(ReflectionProvider::class), diff --git a/tests/Rule/data/ForbidCheckedExceptionInCallableRule/code.php b/tests/Rule/data/ForbidCheckedExceptionInCallableRule/code.php index b526a03..e878af4 100644 --- a/tests/Rule/data/ForbidCheckedExceptionInCallableRule/code.php +++ b/tests/Rule/data/ForbidCheckedExceptionInCallableRule/code.php @@ -224,3 +224,73 @@ public function allowThrow(callable $callable): void } } + +class ArrowFunctionTest extends BaseCallableTest { + + public function testDeclarations(): void + { + $fn = fn () => throw new CheckedException(); // error: Throwing checked exception ForbidCheckedExceptionInCallableRule\CheckedException in arrow function! + + $fn2 = fn () => $this->throws(); // error: Throwing checked exception ForbidCheckedExceptionInCallableRule\CheckedException in arrow function! + + $fn3 = fn () => $this->noop(); // implicit throw is ignored + + $fn4 = fn (callable $c) => $c(); // implicit throw is ignored (https://github.com/phpstan/phpstan/issues/9779) + } + + public function testExplicitExecution(): void + { + (fn () => throw new CheckedException())(); + } + + public function testPassedCallbacks(): void + { + $this->immediateThrow(fn () => throw new CheckedException()); + + array_map(fn () => throw new CheckedException(), []); + + array_map(fn () => $this->throws(), []); + + $this->allowThrow(fn () => $this->throws()); + + $this->allowThrowInBaseClass(fn () => $this->throws()); + + $this->allowThrowInInterface(fn () => $this->throws()); + + $this->denied(fn () => throw new CheckedException()); // error: Throwing checked exception ForbidCheckedExceptionInCallableRule\CheckedException in arrow function! + + $this?->denied(fn () => throw new CheckedException()); // error: Throwing checked exception ForbidCheckedExceptionInCallableRule\CheckedException in arrow function! + } + + private function noop(): void + { + } + + /** + * @throws CheckedException + */ + private function throws(): void + { + throw new CheckedException(); + } + + private function denied(callable $callable): void + { + + } + + public function immediateThrow(callable $callable): void + { + $callable(); + } + + public function allowThrow(callable $callable): void + { + try { + $callable(); + } catch (\Exception $e) { + + } + } + +} diff --git a/tests/Rule/data/ForbidCheckedExceptionInCallableRule/visitor.neon b/tests/Rule/data/ForbidCheckedExceptionInCallableRule/visitor.neon index ae243c1..d2e27e2 100644 --- a/tests/Rule/data/ForbidCheckedExceptionInCallableRule/visitor.neon +++ b/tests/Rule/data/ForbidCheckedExceptionInCallableRule/visitor.neon @@ -6,10 +6,12 @@ services: 'array_map': 0 'ForbidCheckedExceptionInCallableRule\ClosureTest::immediateThrow': 0 'ForbidCheckedExceptionInCallableRule\FirstClassCallableTest::immediateThrow': 1 + 'ForbidCheckedExceptionInCallableRule\ArrowFunctionTest::immediateThrow': 0 allowedCheckedExceptionCallables: 'ForbidCheckedExceptionInCallableRule\CallableTest::allowThrowInInterface': [0] 'ForbidCheckedExceptionInCallableRule\BaseCallableTest::allowThrowInBaseClass': [0] 'ForbidCheckedExceptionInCallableRule\ClosureTest::allowThrow': [0] 'ForbidCheckedExceptionInCallableRule\FirstClassCallableTest::allowThrow': [1] + 'ForbidCheckedExceptionInCallableRule\ArrowFunctionTest::allowThrow': [0] tags: - phpstan.parser.richParserNodeVisitor