Skip to content

Commit

Permalink
Global scope support (fixed #65 #64) (#66)
Browse files Browse the repository at this point in the history
  • Loading branch information
pepakriz authored Feb 15, 2019
1 parent 53ebd08 commit 1314ff5
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 13 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ This extension provides following rules and features:
* Unreachable catch statements
* exception has been caught in some previous catch statement ([examples](https://github.com/pepakriz/phpstan-exception-rules/blob/master/tests/src/Rules/data/unreachable-catches.php))
* checked exception is never thrown in the corresponding try block ([examples](https://github.com/pepakriz/phpstan-exception-rules/blob/master/tests/src/Rules/data/unused-catches.php))
* Report throwing checked exceptions in the global scope ([examples](https://github.com/pepakriz/phpstan-exception-rules/blob/master/tests/src/Rules/data/throws-in-global-scope.php))

Features and rules provided by PHPStan core (we rely on):

Expand All @@ -46,6 +47,7 @@ includes:
parameters:
exceptionRules:
reportUnusedCatchesOfUncheckedExceptions: false
reportCheckedThrowsInGlobalScope: false
ignoreDescriptiveUncheckedExceptions: false
checkedExceptions:
- RuntimeException
Expand Down
2 changes: 2 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
parameters:
exceptionRules:
reportUnusedCatchesOfUncheckedExceptions: false
reportCheckedThrowsInGlobalScope: false
ignoreDescriptiveUncheckedExceptions: false
checkedExceptions: []
uncheckedExceptions: []
Expand Down Expand Up @@ -46,6 +47,7 @@ services:
class: Pepakriz\PHPStanExceptionRules\Rules\ThrowsPhpDocRule
arguments:
reportUnusedCatchesOfUncheckedExceptions: %exceptionRules.reportUnusedCatchesOfUncheckedExceptions%
reportCheckedThrowsInGlobalScope: %exceptionRules.reportCheckedThrowsInGlobalScope%
ignoreDescriptiveUncheckedExceptions: %exceptionRules.ignoreDescriptiveUncheckedExceptions%
tags: [phpstan.rules.rule]

Expand Down
1 change: 1 addition & 0 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ parameters:

exceptionRules:
reportUnusedCatchesOfUncheckedExceptions: true
reportCheckedThrowsInGlobalScope: true
uncheckedExceptions:
- LogicException
- PHPStan\ShouldNotHappenException
Expand Down
28 changes: 19 additions & 9 deletions src/Rules/ThrowsPhpDocRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ class ThrowsPhpDocRule implements Rule
*/
private $reportUnusedCatchesOfUncheckedExceptions;

/**
* @var bool
*/
private $reportCheckedThrowsInGlobalScope;

/**
* @var bool
*/
Expand All @@ -105,6 +110,7 @@ public function __construct(
ThrowsAnnotationReader $throwsAnnotationReader,
Broker $broker,
bool $reportUnusedCatchesOfUncheckedExceptions,
bool $reportCheckedThrowsInGlobalScope,
bool $ignoreDescriptiveUncheckedExceptions
)
{
Expand All @@ -115,6 +121,7 @@ public function __construct(
$this->broker = $broker;
$this->throwsScope = new ThrowsScope();
$this->reportUnusedCatchesOfUncheckedExceptions = $reportUnusedCatchesOfUncheckedExceptions;
$this->reportCheckedThrowsInGlobalScope = $reportCheckedThrowsInGlobalScope;
$this->ignoreDescriptiveUncheckedExceptions = $ignoreDescriptiveUncheckedExceptions;
}

Expand All @@ -129,7 +136,7 @@ public function getNodeType(): string
public function processNode(Node $node, Scope $scope): array
{
if ($node instanceof TryCatch) {
return $this->processTryCatch($node, $scope);
return $this->processTryCatch($node);
}

if ($node instanceof TryCatchTryEnd) {
Expand Down Expand Up @@ -178,14 +185,8 @@ public function processNode(Node $node, Scope $scope): array
/**
* @return string[]
*/
private function processTryCatch(TryCatch $node, Scope $scope): array
private function processTryCatch(TryCatch $node): array
{
$classReflection = $scope->getClassReflection();
$methodReflection = $scope->getFunction();
if ($classReflection === null || $methodReflection === null) {
return [];
}

$this->throwsScope->enterToTryCatch($node);

if (!$node->hasAttribute(self::ATTRIBUTE_HAS_TRY_CATCH_END)) {
Expand Down Expand Up @@ -216,7 +217,16 @@ private function processThrow(Throw_ $node, Scope $scope): array
$exceptionClassNames = $this->throwsScope->filterExceptionsByUncaught($exceptionClassNames);
$exceptionClassNames = $this->checkedExceptionService->filterCheckedExceptions($exceptionClassNames);

return array_map(static function (string $exceptionClassName): string {
$isInGlobalScope = $this->throwsScope->isInGlobalScope();
if (!$this->reportCheckedThrowsInGlobalScope && $isInGlobalScope) {
return [];
}

return array_map(static function (string $exceptionClassName) use ($isInGlobalScope): string {
if ($isInGlobalScope) {
return sprintf('Throwing checked exception %s in global scope is prohibited', $exceptionClassName);
}

return sprintf('Missing @throws %s annotation', $exceptionClassName);
}, $exceptionClassNames);
}
Expand Down
13 changes: 9 additions & 4 deletions src/Rules/ThrowsScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,22 @@ class ThrowsScope
/**
* @var int
*/
private $stackIndex = -1;
private $stackIndex = 0;

/**
* @var array<Type|null>
*/
private $throwsAnnotationBlockStack = [];
private $throwsAnnotationBlockStack = [null];

/**
* @var bool[][]
*/
private $usedThrowsAnnotationsStack = [];
private $usedThrowsAnnotationsStack = [[]];

/**
* @var TryCatch[][]
*/
private $tryCatchStack = [];
private $tryCatchStack = [[]];

public function enterToThrowsAnnotationBlock(?Type $type): void
{
Expand All @@ -62,6 +62,11 @@ public function exitFromThrowsAnnotationBlock(): array
return array_keys($usedThrowsAnnotations);
}

public function isInGlobalScope(): bool
{
return $this->stackIndex === 0;
}

public function enterToTryCatch(TryCatch $tryCatch): void
{
$this->tryCatchStack[$this->stackIndex][] = $tryCatch;
Expand Down
1 change: 1 addition & 0 deletions tests/src/Rules/PhpInternalsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ protected function getRule(): Rule
$this->createThrowsAnnotationReader(),
$this->createBroker(),
true,
true,
true
);
}
Expand Down
12 changes: 12 additions & 0 deletions tests/src/Rules/ThrowsPhpDocRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@ class ThrowsPhpDocRuleTest extends RuleTestCase
*/
private $ignoreDescriptiveUncheckedExceptions = false;

/**
* @var bool
*/
private $reportCheckedThrowsInGlobalScope = false;

/**
* @var mixed[]
*/
Expand Down Expand Up @@ -69,6 +74,7 @@ protected function getRule(): Rule
$this->createThrowsAnnotationReader(),
$this->createBroker(),
$this->reportUnusedCatchesOfUncheckedExceptions,
$this->reportCheckedThrowsInGlobalScope,
$this->ignoreDescriptiveUncheckedExceptions
);
}
Expand Down Expand Up @@ -149,4 +155,10 @@ public function testAnonymClass(): void
$this->analyse(__DIR__ . '/data/throws-anonym-class.php');
}

public function testThrowsInGlobalScope(): void
{
$this->reportCheckedThrowsInGlobalScope = true;
$this->analyse(__DIR__ . '/data/throws-in-global-scope.php');
}

}
17 changes: 17 additions & 0 deletions tests/src/Rules/data/throws-in-global-scope.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php declare(strict_types = 1);

namespace Pepakriz\PHPStanExceptionRules\Rules\ThrowsInGlobalScope;

use LogicException;
use RuntimeException;

if (false) {
throw new LogicException();
throw new RuntimeException(); // error: Throwing checked exception RuntimeException in global scope is prohibited

try {
throw new RuntimeException();
} catch (RuntimeException $e) {
// ignore
}
}

0 comments on commit 1314ff5

Please sign in to comment.