From b8c349166e8be37e4340b3faa6e849b900dddd4e Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Thu, 28 Dec 2017 20:40:28 +0100 Subject: [PATCH] Add InvalidCatch and InvalidThrow to prevent erroneous exceptions Fix #411 and fix #412 --- config.xsd | 2 + .../Checker/Statements/Block/TryChecker.php | 25 +++++++++++ src/Psalm/Checker/StatementsChecker.php | 31 ++++++++++++-- src/Psalm/Issue/InvalidCatch.php | 6 +++ src/Psalm/Issue/InvalidThrow.php | 6 +++ tests/TryCatchTest.php | 41 ++++++++++++++++++- 6 files changed, 106 insertions(+), 5 deletions(-) create mode 100644 src/Psalm/Issue/InvalidCatch.php create mode 100644 src/Psalm/Issue/InvalidThrow.php diff --git a/config.xsd b/config.xsd index 7aac4c502..812e83df4 100644 --- a/config.xsd +++ b/config.xsd @@ -106,6 +106,7 @@ + @@ -124,6 +125,7 @@ + diff --git a/src/Psalm/Checker/Statements/Block/TryChecker.php b/src/Psalm/Checker/Statements/Block/TryChecker.php index fd21871ca..5562e8695 100644 --- a/src/Psalm/Checker/Statements/Block/TryChecker.php +++ b/src/Psalm/Checker/Statements/Block/TryChecker.php @@ -2,15 +2,19 @@ namespace Psalm\Checker\Statements\Block; use PhpParser; +use Psalm\Checker\ClassChecker; use Psalm\Checker\ClassLikeChecker; use Psalm\Checker\InterfaceChecker; use Psalm\Checker\ScopeChecker; use Psalm\Checker\StatementsChecker; use Psalm\CodeLocation; use Psalm\Context; +use Psalm\Issue\InvalidCatch; +use Psalm\IssueBuffer; use Psalm\Scope\LoopScope; use Psalm\Type; use Psalm\Type\Atomic\TNamedObject; +use Psalm\Type\Union; class TryChecker { @@ -111,6 +115,27 @@ class TryChecker } } + $exception_type = new Union([new TNamedObject('Exception'), new TNamedObject('Throwable')]); + + if ((ClassChecker::classExists($project_checker, $fq_catch_class) + && strtolower($fq_catch_class) !== 'exception' + && !(ClassChecker::classExtends($project_checker, $fq_catch_class, 'Exception') + || ClassChecker::classImplements($project_checker, $fq_catch_class, 'Throwable'))) + || (InterfaceChecker::interfaceExists($project_checker, $fq_catch_class) + && strtolower($fq_catch_class) !== 'throwable' + && !InterfaceChecker::interfaceExtends($project_checker, $fq_catch_class, 'Throwable')) + ) { + if (IssueBuffer::accepts( + new InvalidCatch( + 'Class/interface ' . $fq_catch_class . ' cannot be caught', + new CodeLocation($statements_checker->getSource(), $stmt) + ), + $statements_checker->getSuppressedIssues() + )) { + return false; + } + } + $fq_catch_classes[] = $fq_catch_class; } diff --git a/src/Psalm/Checker/StatementsChecker.php b/src/Psalm/Checker/StatementsChecker.php index c57b57da0..ea995c6df 100644 --- a/src/Psalm/Checker/StatementsChecker.php +++ b/src/Psalm/Checker/StatementsChecker.php @@ -22,6 +22,7 @@ use Psalm\Issue\ContinueOutsideLoop; use Psalm\Issue\InvalidDocblock; use Psalm\Issue\InvalidGlobal; use Psalm\Issue\InvalidReturnStatement; +use Psalm\Issue\InvalidThrow; use Psalm\Issue\LessSpecificReturnStatement; use Psalm\Issue\MissingFile; use Psalm\Issue\UnevaluatedCode; @@ -31,6 +32,8 @@ use Psalm\IssueBuffer; use Psalm\Scope\LoopScope; use Psalm\StatementsSource; use Psalm\Type; +use Psalm\Type\Atomic\TNamedObject; +use Psalm\Type\Union; class StatementsChecker extends SourceChecker implements StatementsSource { @@ -210,7 +213,7 @@ class StatementsChecker extends SourceChecker implements StatementsSource $this->analyzeReturn($project_checker, $stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Stmt\Throw_) { $has_returned = true; - $this->analyzeThrow($stmt, $context); + $this->analyzeThrow($project_checker, $stmt, $context); } elseif ($stmt instanceof PhpParser\Node\Stmt\Switch_) { SwitchChecker::analyze($this, $stmt, $context, $loop_scope); } elseif ($stmt instanceof PhpParser\Node\Stmt\Break_) { @@ -1065,7 +1068,7 @@ class StatementsChecker extends SourceChecker implements StatementsSource . 'type \'' . $this->local_return_type . '\' for ' . $cased_method_id, new CodeLocation($this->source, $stmt) ), - $this->getSuppressedIssues() + $this->getSuppressedIssues() )) { return false; } @@ -1084,9 +1087,29 @@ class StatementsChecker extends SourceChecker implements StatementsSource * * @return false|null */ - private function analyzeThrow(PhpParser\Node\Stmt\Throw_ $stmt, Context $context) + private function analyzeThrow(ProjectChecker $project_checker, PhpParser\Node\Stmt\Throw_ $stmt, Context $context) { - return ExpressionChecker::analyze($this, $stmt->expr, $context); + if (ExpressionChecker::analyze($this, $stmt->expr, $context) === false) { + return false; + } + + if (isset($stmt->expr->inferredType) && !$stmt->expr->inferredType->isMixed()) { + $throw_type = $stmt->expr->inferredType; + + $exception_type = new Union([new TNamedObject('Exception'), new TNamedObject('Throwable')]); + + if (!TypeChecker::isContainedBy($project_checker, $throw_type, $exception_type)) { + if (IssueBuffer::accepts( + new InvalidThrow( + 'Cannot throw ' . $throw_type, + new CodeLocation($this->source, $stmt) + ), + $this->getSuppressedIssues() + )) { + return false; + } + } + } } /** diff --git a/src/Psalm/Issue/InvalidCatch.php b/src/Psalm/Issue/InvalidCatch.php new file mode 100644 index 000000000..cd420464e --- /dev/null +++ b/src/Psalm/Issue/InvalidCatch.php @@ -0,0 +1,6 @@ + [ + 'PHP7-addThrowableInterfaceType' => [ 'getMessage(); }', ], + 'PHP7-rethrowInterfaceExceptionWithoutInvalidThrow' => [ + ' [ ' 'bool', ], ], + + ]; + } + + /** + * @return array + */ + public function providerFileCheckerInvalidCodeParse() + { + return [ + 'invalidCatchClass' => [ + ' 'InvalidCatch', + ], + 'invalidThrowClass' => [ + ' 'InvalidThrow', + ], ]; } }