From e687887ba329566cd89a80961e1181dc135b0c8f Mon Sep 17 00:00:00 2001 From: Matthew Brown Date: Sat, 18 Mar 2017 12:18:17 -0400 Subject: [PATCH] Emit an InvalidReturnType when it should contain null, and introduct LessSpecificReturnType --- config.xsd | 1 + src/Psalm/Checker/FunctionLikeChecker.php | 15 ++++++++------- src/Psalm/Issue/LessSpecificReturnType.php | 6 ++++++ tests/JsonOutputTest.php | 6 +++--- tests/ReturnTypeTest.php | 4 ++-- tests/TypeAlgebraTest.php | 4 ++-- 6 files changed, 22 insertions(+), 14 deletions(-) create mode 100644 src/Psalm/Issue/LessSpecificReturnType.php diff --git a/config.xsd b/config.xsd index 02b775ba6..5d965e2e7 100644 --- a/config.xsd +++ b/config.xsd @@ -113,6 +113,7 @@ + diff --git a/src/Psalm/Checker/FunctionLikeChecker.php b/src/Psalm/Checker/FunctionLikeChecker.php index 63b272b04..18e4e4382 100644 --- a/src/Psalm/Checker/FunctionLikeChecker.php +++ b/src/Psalm/Checker/FunctionLikeChecker.php @@ -18,6 +18,7 @@ use Psalm\Issue\InvalidDocblock; use Psalm\Issue\InvalidParamDefault; use Psalm\Issue\InvalidReturnType; use Psalm\Issue\InvalidToString; +use Psalm\Issue\LessSpecificReturnType; use Psalm\Issue\MethodSignatureMismatch; use Psalm\Issue\MisplacedRequiredParam; use Psalm\Issue\MissingClosureReturnType; @@ -1115,7 +1116,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo $inferred_return_type, $declared_return_type, $this->getFileChecker(), - true, + false, $has_scalar_match, $type_coerced )) { @@ -1131,7 +1132,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo if ($type_coerced) { if (IssueBuffer::accepts( new MoreSpecificReturnType( - 'The given return type \'' . $declared_return_type . '\' for ' . $cased_method_id . + 'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id . ' is more specific than the inferred return type \'' . $inferred_return_type . '\'', $secondary_return_type_location ?: $return_type_location ), @@ -1142,7 +1143,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } else { if (IssueBuffer::accepts( new InvalidReturnType( - 'The given return type \'' . $declared_return_type . '\' for ' . $cased_method_id . + 'The declared return type \'' . $declared_return_type . '\' for ' . $cased_method_id . ' is incorrect, got \'' . $inferred_return_type . '\'', $secondary_return_type_location ?: $return_type_location ), @@ -1151,7 +1152,7 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo return false; } } - } elseif ($inferred_return_type->isNullable() !== $declared_return_type->isNullable()) { + } elseif (!$inferred_return_type->isNullable() && $declared_return_type->isNullable()) { if ($update_docblock) { if (!in_array('InvalidReturnType', $this->suppressed_issues)) { $this->addDocblockReturnType($inferred_return_type); @@ -1161,9 +1162,9 @@ abstract class FunctionLikeChecker extends SourceChecker implements StatementsSo } if (IssueBuffer::accepts( - new MoreSpecificReturnType( - 'The given return type \'' . $declared_return_type . '\' for ' . $cased_method_id . - ' is more specific than the inferred return type \'' . $inferred_return_type . '\'', + new LessSpecificReturnType( + 'The inferred return type \'' . $inferred_return_type . '\' for ' . $cased_method_id . + ' is more specific than the declared return type \'' . $declared_return_type . '\'', $secondary_return_type_location ?: $return_type_location ), $this->suppressed_issues diff --git a/src/Psalm/Issue/LessSpecificReturnType.php b/src/Psalm/Issue/LessSpecificReturnType.php new file mode 100644 index 000000000..14942b2ec --- /dev/null +++ b/src/Psalm/Issue/LessSpecificReturnType.php @@ -0,0 +1,6 @@ +assertSame('somefile.php', $issue_data['file_path']); $this->assertSame('error', $issue_data['type']); - $this->assertSame("The given return type 'string' for fooFoo is incorrect, got 'int'", $issue_data['message']); + $this->assertSame("The declared return type 'string' for fooFoo is incorrect, got 'int'", $issue_data['message']); $this->assertSame(2, $issue_data['line_number']); $this->assertSame( 'string', @@ -174,7 +174,7 @@ class JsonOutputTest extends PHPUnit_Framework_TestCase $issue_data = IssueBuffer::getIssueData()[0]; $this->assertSame('somefile.php', $issue_data['file_path']); $this->assertSame('error', $issue_data['type']); - $this->assertSame('The given return type \'int\' for fooFoo is incorrect, got \'string\'', $issue_data['message']); + $this->assertSame('The declared return type \'int\' for fooFoo is incorrect, got \'string\'', $issue_data['message']); $this->assertSame(3, $issue_data['line_number']); $this->assertSame( '@return int', @@ -203,7 +203,7 @@ class JsonOutputTest extends PHPUnit_Framework_TestCase $issue_data = IssueBuffer::getIssueData()[0]; $this->assertSame('somefile.php', $issue_data['file_path']); $this->assertSame('error', $issue_data['type']); - $this->assertSame('The given return type \'int\' for fooFoo is incorrect, got \'string\'', $issue_data['message']); + $this->assertSame('The declared return type \'int\' for fooFoo is incorrect, got \'string\'', $issue_data['message']); $this->assertSame(2, $issue_data['line_number']); $this->assertSame( '@return int', diff --git a/tests/ReturnTypeTest.php b/tests/ReturnTypeTest.php index 989f0bde1..eed7d422f 100644 --- a/tests/ReturnTypeTest.php +++ b/tests/ReturnTypeTest.php @@ -583,7 +583,7 @@ class ReturnTypeTest extends PHPUnit_Framework_TestCase /** * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage MoreSpecificReturnType + * @expectedExceptionMessage InvalidReturnType * @return void */ public function testWrongReturnType2() @@ -621,7 +621,7 @@ class ReturnTypeTest extends PHPUnit_Framework_TestCase /** * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage MoreSpecificReturnType + * @expectedExceptionMessage InvalidReturnType * @return void */ public function testWrongReturnTypeInNamespace2() diff --git a/tests/TypeAlgebraTest.php b/tests/TypeAlgebraTest.php index 240282219..d6854025b 100644 --- a/tests/TypeAlgebraTest.php +++ b/tests/TypeAlgebraTest.php @@ -265,7 +265,7 @@ class TypeAlgebraTest extends PHPUnit_Framework_TestCase /** * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage MoreSpecificReturnType + * @expectedExceptionMessage InvalidReturnType * @return void */ public function testInvertedTwoVarLogicNotNestedWithVarChange() @@ -289,7 +289,7 @@ class TypeAlgebraTest extends PHPUnit_Framework_TestCase /** * @expectedException \Psalm\Exception\CodeException - * @expectedExceptionMessage MoreSpecificReturnType + * @expectedExceptionMessage InvalidReturnType * @return void */ public function testInvertedTwoVarLogicNotNestedWithElseif()