Skip to content

Global scope support (fixed #65 #64) #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 15, 2019
Merged
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
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
}
}