From 2b2988b072f9b0d28ab43d9ef1f27f54be422e55 Mon Sep 17 00:00:00 2001 From: Brown Date: Tue, 13 Aug 2019 15:44:18 -0400 Subject: [PATCH] Fix #2019 - allow union in @throws --- .../Internal/Analyzer/CommentAnalyzer.php | 8 +++-- .../Analyzer/FunctionLikeAnalyzer.php | 36 +++++++++---------- .../Scanner/FunctionDocblockComment.php | 2 +- .../Internal/Visitor/ReflectorVisitor.php | 23 ++++++++---- src/Psalm/Storage/FunctionLikeStorage.php | 5 +++ tests/ThrowsAnnotationTest.php | 22 +++++++++++- 6 files changed, 67 insertions(+), 29 deletions(-) diff --git a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php index 380ba9bfb..9f588f4a2 100644 --- a/src/Psalm/Internal/Analyzer/CommentAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/CommentAnalyzer.php @@ -523,14 +523,18 @@ class CommentAnalyzer } if (isset($parsed_docblock['specials']['throws'])) { - foreach ($parsed_docblock['specials']['throws'] as $throws_entry) { + foreach ($parsed_docblock['specials']['throws'] as $offset => $throws_entry) { $throws_class = preg_split('/[\s]+/', $throws_entry)[0]; if (!$throws_class) { throw new IncorrectDocblockException('Unexpectedly empty @throws'); } - $info->throws[] = $throws_class; + $info->throws[] = [ + $throws_class, + $offset + $comment->getFilePos(), + $comment->getLine() + substr_count($comment->getText(), "\n", 0, $offset) + ]; } } diff --git a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php index 8f5124b89..64273fd02 100644 --- a/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php @@ -1020,32 +1020,32 @@ abstract class FunctionLikeAnalyzer extends SourceAnalyzer implements Statements } foreach ($storage->throws as $expected_exception => $_) { - if ($storage->location - && ClassLikeAnalyzer::checkFullyQualifiedClassLikeName( + if (isset($storage->throw_locations[$expected_exception])) { + if (ClassLikeAnalyzer::checkFullyQualifiedClassLikeName( $statements_analyzer, $expected_exception, - $storage->location, + $storage->throw_locations[$expected_exception], $statements_analyzer->getSuppressedIssues(), false, false, true, true - ) - ) { - $input_type = new Type\Union([new TNamedObject($expected_exception)]); - $container_type = new Type\Union([new TNamedObject('Exception'), new TNamedObject('Throwable')]); + )) { + $input_type = new Type\Union([new TNamedObject($expected_exception)]); + $container_type = new Type\Union([new TNamedObject('Exception'), new TNamedObject('Throwable')]); - if (!TypeAnalyzer::isContainedBy($codebase, $input_type, $container_type)) { - if (IssueBuffer::accepts( - new \Psalm\Issue\InvalidThrow( - 'Class supplied for @throws ' . $expected_exception - . ' does not implement Throwable', - $storage->location, - $expected_exception - ), - $statements_analyzer->getSuppressedIssues() - )) { - // fall through + if (!TypeAnalyzer::isContainedBy($codebase, $input_type, $container_type)) { + if (IssueBuffer::accepts( + new \Psalm\Issue\InvalidThrow( + 'Class supplied for @throws ' . $expected_exception + . ' does not implement Throwable', + $storage->throw_locations[$expected_exception], + $expected_exception + ), + $statements_analyzer->getSuppressedIssues() + )) { + // fall through + } } } } diff --git a/src/Psalm/Internal/Scanner/FunctionDocblockComment.php b/src/Psalm/Internal/Scanner/FunctionDocblockComment.php index 94c15869f..5711741a9 100644 --- a/src/Psalm/Internal/Scanner/FunctionDocblockComment.php +++ b/src/Psalm/Internal/Scanner/FunctionDocblockComment.php @@ -111,7 +111,7 @@ class FunctionDocblockComment public $suppress = []; /** - * @var array + * @var array */ public $throws = []; diff --git a/src/Psalm/Internal/Visitor/ReflectorVisitor.php b/src/Psalm/Internal/Visitor/ReflectorVisitor.php index b56055916..97dc9e5dd 100644 --- a/src/Psalm/Internal/Visitor/ReflectorVisitor.php +++ b/src/Psalm/Internal/Visitor/ReflectorVisitor.php @@ -2034,16 +2034,25 @@ class ReflectorVisitor extends PhpParser\NodeVisitorAbstract implements PhpParse $storage->suppressed_issues = $docblock_info->suppress; - foreach ($docblock_info->throws as $throw_class) { - $exception_fqcln = Type::getFQCLNFromString( - $throw_class, - $this->aliases + foreach ($docblock_info->throws as [$throw, $offset, $line]) { + $throw_location = new CodeLocation\DocblockTypeLocation( + $this->file_scanner, + $offset, + $offset + \strlen($throw), + $line ); - $this->codebase->scanner->queueClassLikeForScanning($exception_fqcln, $this->file_path); - $this->file_storage->referenced_classlikes[strtolower($exception_fqcln)] = $exception_fqcln; + foreach (\array_map('trim', explode('|', $throw)) as $throw_class) { + $exception_fqcln = Type::getFQCLNFromString( + $throw_class, + $this->aliases + ); - $storage->throws[$exception_fqcln] = true; + $this->codebase->scanner->queueClassLikeForScanning($exception_fqcln, $this->file_path); + $this->file_storage->referenced_classlikes[strtolower($exception_fqcln)] = $exception_fqcln; + $storage->throws[$exception_fqcln] = true; + $storage->throw_locations[$exception_fqcln] = $throw_location; + } } if (!$this->config->use_docblock_types) { diff --git a/src/Psalm/Storage/FunctionLikeStorage.php b/src/Psalm/Storage/FunctionLikeStorage.php index 2b1b259b8..88ea64bd1 100644 --- a/src/Psalm/Storage/FunctionLikeStorage.php +++ b/src/Psalm/Storage/FunctionLikeStorage.php @@ -151,6 +151,11 @@ class FunctionLikeStorage */ public $throws = []; + /** + * @var array + */ + public $throw_locations = []; + /** * @var bool */ diff --git a/tests/ThrowsAnnotationTest.php b/tests/ThrowsAnnotationTest.php index f6296cbcf..362242fe5 100644 --- a/tests/ThrowsAnnotationTest.php +++ b/tests/ThrowsAnnotationTest.php @@ -8,7 +8,7 @@ class ThrowsAnnotationTest extends TestCase { public function testUndefinedClassAsThrows() : void { - $this->expectExceptionMessage('UndefinedDocblockClass'); + $this->expectExceptionMessage('UndefinedDocblockClass - somefile.php:3:28'); $this->expectException(\Psalm\Exception\CodeException::class); $this->addFile( @@ -46,6 +46,26 @@ class ThrowsAnnotationTest extends TestCase $this->analyzeFile('somefile.php', $context); } + public function testInheritedThrowableClassAsThrows() : void + { + $this->addFile( + 'somefile.php', + 'analyzeFile('somefile.php', $context); + } + /** * @return void */