Skip to content

Commit

Permalink
ForbidCheckedExceptionInYieldingMethodRule (#109)
Browse files Browse the repository at this point in the history
  • Loading branch information
janedbal authored Apr 28, 2023
1 parent 3587b44 commit 4717a98
Show file tree
Hide file tree
Showing 5 changed files with 180 additions and 0 deletions.
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ parameters:
forbidCast:
enabled: true
blacklist: ['(array)', '(object)', '(unset)']
forbidCheckedExceptionInYieldingMethod:
enabled: true
forbidCustomFunctions:
enabled: true
list: []
Expand Down Expand Up @@ -331,6 +333,21 @@ parameters:
blacklist!: ['(array)', '(object)', '(unset)']
```

### forbidCheckedExceptionInYieldingMethod
- Denies throwing [checked exception](https://phpstan.org/blog/bring-your-exceptions-under-control) within yielding methods as those exceptions are not throw upon method call, but when generator gets iterated.
- This behaviour cannot be easily reflected within PHPStan exception analysis and may cause [false negatives](https://phpstan.org/r/d07ac0f0-a49d-4f82-b1dd-1939058bbeed).

```php
class Provider {
/** @throws CheckedException */
public static function generate(): iterable
{
yield 1;
throw new CheckedException(); // denied, gets thrown once iterated
}
}
```

### forbidCustomFunctions *
- Allows you to easily deny some approaches within your codebase by denying classes, methods and functions
- Configuration syntax is array where key is method name and value is reason used in error message
Expand Down
9 changes: 9 additions & 0 deletions rules.neon
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ parameters:
forbidCast:
enabled: true
blacklist: ['(array)', '(object)', '(unset)']
forbidCheckedExceptionInYieldingMethod:
enabled: true
forbidCustomFunctions:
enabled: true
list: []
Expand Down Expand Up @@ -102,6 +104,9 @@ parametersSchema:
enabled: bool()
blacklist: arrayOf(string())
])
forbidCheckedExceptionInYieldingMethod: structure([
enabled: bool()
])
forbidCustomFunctions: structure([
enabled: bool()
list: arrayOf(string())
Expand Down Expand Up @@ -187,6 +192,8 @@ conditionalTags:
phpstan.rules.rule: %shipmonkRules.forbidAssignmentNotMatchingVarDoc.enabled%
ShipMonk\PHPStan\Rule\ForbidCastRule:
phpstan.rules.rule: %shipmonkRules.forbidCast.enabled%
ShipMonk\PHPStan\Rule\ForbidCheckedExceptionInYieldingMethodRule:
phpstan.rules.rule: %shipmonkRules.forbidCheckedExceptionInYieldingMethod.enabled%
ShipMonk\PHPStan\Rule\ForbidCustomFunctionsRule:
phpstan.rules.rule: %shipmonkRules.forbidCustomFunctions.enabled%
ShipMonk\PHPStan\Rule\ForbidEnumInFunctionArgumentsRule:
Expand Down Expand Up @@ -266,6 +273,8 @@ services:
allowNarrowing: %shipmonkRules.forbidAssignmentNotMatchingVarDoc.allowNarrowing%
-
class: ShipMonk\PHPStan\Rule\ForbidCastRule
-
class: ShipMonk\PHPStan\Rule\ForbidCheckedExceptionInYieldingMethodRule
-
class: ShipMonk\PHPStan\Rule\ForbidCustomFunctionsRule
arguments:
Expand Down
59 changes: 59 additions & 0 deletions src/Rule/ForbidCheckedExceptionInYieldingMethodRule.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\Rule;

use PhpParser\Node;
use PHPStan\Analyser\Scope;
use PHPStan\Node\MethodReturnStatementsNode;
use PHPStan\Rules\Exceptions\DefaultExceptionTypeResolver;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleError;
use PHPStan\Rules\RuleErrorBuilder;

/**
* @implements Rule<MethodReturnStatementsNode>
*/
class ForbidCheckedExceptionInYieldingMethodRule implements Rule
{

private DefaultExceptionTypeResolver $exceptionTypeResolver;

public function __construct(DefaultExceptionTypeResolver $exceptionTypeResolver)
{
$this->exceptionTypeResolver = $exceptionTypeResolver;
}

public function getNodeType(): string
{
return MethodReturnStatementsNode::class;
}

/**
* @param MethodReturnStatementsNode $node
* @return list<RuleError>
*/
public function processNode(
Node $node,
Scope $scope
): array
{
if (!$node->getStatementResult()->hasYield()) {
return [];
}

$errors = [];

foreach ($node->getStatementResult()->getThrowPoints() as $throwPoint) {
foreach ($throwPoint->getType()->getObjectClassNames() as $exceptionClass) {
if ($this->exceptionTypeResolver->isCheckedException($exceptionClass, $throwPoint->getScope())) {
$errors[] = RuleErrorBuilder::message("Throwing checked exception $exceptionClass in yielding method is denied as it gets thrown upon Generator iteration")
->line($throwPoint->getNode()->getLine())
->build();
}
}
}

return $errors;
}

}
33 changes: 33 additions & 0 deletions tests/Rule/ForbidCheckedExceptionInYieldingMethodRuleTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php declare(strict_types = 1);

namespace ShipMonk\PHPStan\Rule;

use PHPStan\Rules\Exceptions\DefaultExceptionTypeResolver;
use PHPStan\Rules\Rule;
use ShipMonk\PHPStan\RuleTestCase;

/**
* @extends RuleTestCase<ForbidCheckedExceptionInYieldingMethodRule>
*/
class ForbidCheckedExceptionInYieldingMethodRuleTest extends RuleTestCase
{

protected function getRule(): Rule
{
$exceptionTypeResolverMock = self::createMock(DefaultExceptionTypeResolver::class);
$exceptionTypeResolverMock
->expects(self::any())
->method('isCheckedException')
->willReturnCallback(static function (string $className): bool {
return $className === 'ForbidCheckedExceptionInYieldingMethodRule\CheckedException';
});

return new ForbidCheckedExceptionInYieldingMethodRule($exceptionTypeResolverMock);
}

public function testClass(): void
{
$this->analyseFile(__DIR__ . '/data/ForbidCheckedExceptionInYieldingMethodRule/code.php');
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php declare(strict_types = 1);

namespace ForbidCheckedExceptionInYieldingMethodRule;

use Generator;
use LogicException;
use RuntimeException;

class CheckedException extends RuntimeException {}

class A {

/**
* @return iterable<int>
* @throws CheckedException
*/
public static function throwPointOfYieldingMethod(bool $throw): iterable
{
yield 1;
if ($throw) {
throw new CheckedException(); // error: Throwing checked exception ForbidCheckedExceptionInYieldingMethodRule\CheckedException in yielding method is denied as it gets thrown upon Generator iteration
}
}

/**
* @return iterable<int>
* @throws CheckedException
*/
private static function throwPointOfNonYieldingMethod(bool $throw): iterable
{
if ($throw) {
throw new CheckedException();
}

return [2];
}

/**
* @return Generator<int>
*/
private static function methodWithUncheckedException(bool $throw): Generator
{
if ($throw) {
throw new LogicException();
}

yield 3;
}

/**
* @throws CheckedException
* @return Generator<int>
*/
public static function testIt(bool $throw): Generator
{
yield from self::throwPointOfYieldingMethod($throw); // error: Throwing checked exception ForbidCheckedExceptionInYieldingMethodRule\CheckedException in yielding method is denied as it gets thrown upon Generator iteration
yield from self::throwPointOfNonYieldingMethod($throw); // error: Throwing checked exception ForbidCheckedExceptionInYieldingMethodRule\CheckedException in yielding method is denied as it gets thrown upon Generator iteration
yield from self::methodWithUncheckedException($throw);
}

}

0 comments on commit 4717a98

Please sign in to comment.