diff --git a/Exception/UnsupportedPsalmVersion.php b/Exception/UnsupportedPsalmVersion.php new file mode 100644 index 0000000..00c4222 --- /dev/null +++ b/Exception/UnsupportedPsalmVersion.php @@ -0,0 +1,8 @@ +classExtends($classStorage->name, TestCase::class)) { - if (self::hasInitializers($classStorage, $classNode)) { - $classStorage->suppressed_issues[] = 'MissingConstructor'; + if (!$codebase->classExtends($classStorage->name, TestCase::class)) { + return; + } + + if (self::hasInitializers($classStorage, $classNode)) { + $classStorage->suppressed_issues[] = 'MissingConstructor'; + } + } + + /** + * {@inheritDoc} + */ + public static function afterStatementAnalysis( + ClassLike $class_node, + ClassLikeStorage $class_storage, + StatementsSource $statements_source, + Codebase $codebase, + array &$file_replacements = [] + ) { + if (!$codebase->classExtends($class_storage->name, TestCase::class)) { + return null; + } + + // add a fake reference to test class to prevent it from being marked as unused + // it would have been easier to add a suppression, but that's only possible + // since 3.0.17 (vimeo/psalm#1353) + // + // This should always pass, we're calling it for the side-effect + // of adding self-reference + + if (!$codebase->classOrInterfaceExists($class_storage->name, $class_storage->location)) { + return null; + } + + foreach ($class_storage->methods as $method_name => $method_storage) { + if (!$method_storage->location) { + continue; + } + + $stmt_method = $class_node->getMethod($method_name); + + if (!$stmt_method) { + throw new \RuntimeException('Failed to find ' . $method_name); + } + + $specials = self::getSpecials($stmt_method); + + $method_id = $class_storage->name . '::' . $method_storage->cased_name; + + if (0 !== strpos($method_storage->cased_name, 'test') + && !isset($specials['test'])) { + continue; // skip non-test methods + } + + $method_storage->suppressed_issues[] = 'PossiblyUnusedMethod'; + + if (!isset($specials['dataProvider'])) { + continue; + } + + foreach ($specials['dataProvider'] as $line => $provider) { + $provider_method_id = $class_storage->name . '::' . (string) $provider; + + $provider_docblock_location = clone $method_storage->location; + $provider_docblock_location->setCommentLine($line); + + // methodExists also can mark methods as used (weird, but handy) + if (!$codebase->methodExists($provider_method_id, $provider_docblock_location, $method_id)) { + IssueBuffer::accepts(new Issue\UndefinedMethod( + 'Provider method ' . $provider_method_id . ' is not defined', + $provider_docblock_location, + $provider_method_id + )); + + continue; + } + + $provider_return_type = $codebase->getMethodReturnType($provider_method_id, $classStorage->name); + assert(null !== $provider_return_type); + + $provider_return_type_string = $provider_return_type->getId(); + + $provider_return_type_location = $codebase->getMethodReturnTypeLocation($provider_method_id); + assert(null !== $provider_return_type_location); + + $expected_provider_return_type = new Type\Atomic\TIterable([ + Type::combineUnionTypes(Type::getInt(), Type::getString()), + Type::getArray(), + ]); + + foreach ($provider_return_type->getTypes() as $type) { + if (!$type->isIterable($codebase)) { + IssueBuffer::accepts(new Issue\InvalidReturnType( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + continue; + } + } + + // unionize iterable so that instead of array|Traversable + // we get iterable + // + // TODO: this may get implemented in a future Psalm version, remove it then + $provider_return_type = self::unionizeIterables($codebase, $provider_return_type); + + if (!self::isTypeContainedByType( + $codebase, + $provider_return_type->type_params[0], + $expected_provider_return_type->type_params[0] + ) || !self::isTypeContainedByType( + $codebase, + $provider_return_type->type_params[1], + $expected_provider_return_type->type_params[1] + )) { + if ($provider_return_type->type_params[0]->hasMixed() + || $provider_return_type->type_params[1]->hasMixed()) { + IssueBuffer::accepts(new Issue\InvalidReturnType( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', possibly different ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + } else { + IssueBuffer::accepts(new Issue\InvalidReturnType( + 'Providers must return ' . $expected_provider_return_type->getId() + . ', ' . $provider_return_type_string . ' provided', + $provider_return_type_location + )); + } + continue; + } + + $checkParam = + /** @return void */ + function ( + Type\Union $potential_argument_type, + FunctionLikeParameter $param, + int $param_offset + ) use ( + $codebase, + $method_id, + $provider_method_id, + $provider_return_type_string, + $provider_docblock_location + ) { + assert(null !== $param->type); + $param_type = clone $param->type; + if ($param->default_type) { + $param_type->possibly_undefined = true; + } + if (self::isTypeContainedByType($codebase, $potential_argument_type, $param_type)) { + // ok + } elseif (self::canTypeBeContainedByType($codebase, $potential_argument_type, $param_type)) { + IssueBuffer::accepts(new Issue\PossiblyInvalidArgument( + 'Argument ' . ($param_offset + 1) . ' of ' . $method_id + . ' expects ' . $param_type->getId() . ', ' + . $potential_argument_type->getId() . ' provided' + . ' by ' . $provider_method_id . '():(' . $provider_return_type_string . ')', + $provider_docblock_location + )); + } elseif ($potential_argument_type->possibly_undefined && !$param->default_type) { + IssueBuffer::accepts(new Issue\InvalidArgument( + 'Argument ' . ($param_offset + 1) . ' of ' . $method_id + . ' has no default value, but possibly undefined ' + . $potential_argument_type->getId() . ' provided' + . ' by ' . $provider_method_id . '():(' . $provider_return_type_string . ')', + $provider_docblock_location + )); + } else { + IssueBuffer::accepts(new Issue\InvalidArgument( + 'Argument ' . ($param_offset + 1) . ' of ' . $method_id + . ' expects ' . $param_type->getId() . ', ' + . $potential_argument_type->getId() . ' provided' + . ' by ' . $provider_method_id . '():(' . $provider_return_type_string . ')', + $provider_docblock_location + )); + } + }; + + /** @var Type\Atomic\TArray|Type\Atomic\ObjectLike $dataset_type */ + $dataset_type = $provider_return_type->type_params[1]->getTypes()['array']; + + if ($dataset_type instanceof Type\Atomic\TArray) { + // check that all of the required (?) params accept value type + $potential_argument_type = $dataset_type->type_params[1]; + foreach ($method_storage->params as $param_offset => $param) { + $checkParam($potential_argument_type, $param, $param_offset); + } + } else { + // iterate over all params checking if corresponding value type is acceptable + // let's hope properties are sorted in array order + $potential_argument_types = array_values($dataset_type->properties); + + if (count($potential_argument_types) < $method_storage->required_param_count) { + IssueBuffer::accepts(new Issue\TooFewArguments( + 'Too few arguments for ' . $method_id + . ' - expecting ' . $method_storage->required_param_count + . ' but saw ' . count($potential_argument_types) + . ' provided by ' . $provider_method_id . '()' + . ':(' . $provider_return_type_string . ')', + $provider_docblock_location, + $method_id + )); + } + + foreach ($method_storage->params as $param_offset => $param) { + if (!isset($potential_argument_types[$param_offset])) { + break; + } + $potential_argument_type = $potential_argument_types[$param_offset]; + + $checkParam($potential_argument_type, $param, $param_offset); + } + } } } } + private static function isTypeContainedByType( + Codebase $codebase, + Type\Union $input_type, + Type\Union $container_type + ): bool { + if (method_exists($codebase, 'isTypeContainedByType')) { + return (bool) $codebase->isTypeContainedByType($input_type, $container_type); + } + + /** @psalm-suppress RedundantCondition */ + if (class_exists(\Psalm\Internal\Analyzer\TypeAnalyzer::class, true) + && method_exists(\Psalm\Internal\Analyzer\TypeAnalyzer::class, 'isContainedBy')) { + return \Psalm\Internal\Analyzer\TypeAnalyzer::isContainedBy($codebase, $input_type, $container_type); + } + + throw new UnsupportedPsalmVersion(); + } + + private static function canTypeBeContainedByType( + Codebase $codebase, + Type\Union $input_type, + Type\Union $container_type + ): bool { + if (method_exists($codebase, 'canTypeBeContainedByType')) { + return (bool) $codebase->canTypeBeContainedByType($input_type, $container_type); + } + + /** @psalm-suppress RedundantCondition */ + if (class_exists(\Psalm\Internal\Analyzer\TypeAnalyzer::class, true) + && method_exists(\Psalm\Internal\Analyzer\TypeAnalyzer::class, 'canBeContainedBy')) { + return \Psalm\Internal\Analyzer\TypeAnalyzer::canBeContainedBy($codebase, $input_type, $container_type); + } + + throw new UnsupportedPsalmVersion(); + } + + /** + * @param Type\Atomic\TNamedObject|Type\Atomic\TIterable $type + * @return array{0:Type\Union,1:Type\Union} + */ + private static function getKeyValueParamsForTraversableObject(Codebase $codebase, $type): array + { + if (method_exists($codebase, 'getKeyValueParamsForTraversableObject')) { + $ret = (array) $codebase->getKeyValueParamsForTraversableObject($type); + assert($ret[0] instanceof Type\Union); + assert($ret[1] instanceof Type\Union); + return [$ret[0], $ret[1]]; + } + + /** @psalm-suppress RedundantCondition */ + if (class_exists(\Psalm\Internal\Analyzer\Statements\Block\ForeachAnalyzer::class, true) + && method_exists( + \Psalm\Internal\Analyzer\Statements\Block\ForeachAnalyzer::class, + 'getKeyValueParamsForTraversableObject' + ) + ) { + $iterable_key_type = null; + $iterable_value_type = null; + + \Psalm\Internal\Analyzer\Statements\Block\ForeachAnalyzer::getKeyValueParamsForTraversableObject( + $type, + $codebase, + $iterable_key_type, + $iterable_value_type + ); + + return [ + $iterable_key_type ?? Type::getMixed(), + $iterable_value_type ?? Type::getMixed(), + ]; + } + + throw new UnsupportedPsalmVersion(); + } + + private static function unionizeIterables(Codebase $codebase, Type\Union $iterables): Type\Atomic\TIterable + { + /** @var Type\Union[] $key_types */ + $key_types = []; + + /** @var Type\Union[] $value_types */ + $value_types = []; + + foreach ($iterables->getTypes() as $type) { + if (!$type->isIterable($codebase)) { + throw new \RuntimeException('should be iterable'); + } + + if ($type instanceof Type\Atomic\TArray) { + $key_types[] = $type->type_params[0] ?? Type::getMixed(); + $value_types[] = $type->type_params[1] ?? Type::getMixed(); + } elseif ($type instanceof Type\Atomic\ObjectLike) { + $key_types[] = $type->getGenericKeyType(); + $value_types[] = $type->getGenericValueType(); + } elseif ($type instanceof Type\Atomic\TNamedObject || $type instanceof Type\Atomic\TIterable) { + list($key_types[], $value_types[]) = self::getKeyValueParamsForTraversableObject($codebase, $type); + } else { + throw new \RuntimeException('unexpected type'); + } + } + + if (empty($key_types) || empty($value_types)) { + return new Type\Atomic\TIterable([ + Type::getMixed(), + Type::getMixed(), + ]); + } + + $combine = + /** @param null|Type\Union $a */ + function ($a, Type\Union $b) use ($codebase): Type\Union { + return $a ? Type::combineUnionTypes($a, $b, $codebase) : $b; + }; + + return new Type\Atomic\TIterable([ + array_reduce($key_types, $combine), + array_reduce($value_types, $combine), + ]); + } + + private static function hasInitializers(ClassLikeStorage $storage, ClassLike $stmt): bool { if (isset($storage->methods['setup'])) { @@ -50,20 +391,21 @@ class TestCaseHandler implements AfterClassLikeVisitInterface private static function isBeforeInitializer(ClassMethod $method): bool { - /** @var string[] $comments */ - $comments = $method->getAttribute('comments', []); + $specials = self::getSpecials($method); + return isset($specials['before']); + } - foreach ($comments as $comment) { - if (!$comment instanceof Doc) { - continue; - } + /** @return array> */ + private static function getSpecials(ClassMethod $method): array + { + $docblock = $method->getDocComment(); - $parsed_comment = DocComment::parse((string)$comment->getReformattedText()); - if (isset($parsed_comment['specials']['before'])) { - return true; + if ($docblock) { + $parsed_comment = DocComment::parse((string)$docblock->getReformattedText(), $docblock->getLine()); + if (isset($parsed_comment['specials'])) { + return $parsed_comment['specials']; } } - - return false; + return []; } } diff --git a/phpcs.xml.dist b/phpcs.xml.dist index 3ca3171..729276b 100755 --- a/phpcs.xml.dist +++ b/phpcs.xml.dist @@ -1,11 +1,15 @@ - + Plugin.php + tests/_support/Helper + tests/_support/AcceptanceTester.php + hooks + Exception diff --git a/tests/acceptance/Assert.feature b/tests/acceptance/Assert.feature index 9321dcc..ca3b0a8 100644 --- a/tests/acceptance/Assert.feature +++ b/tests/acceptance/Assert.feature @@ -4,7 +4,17 @@ Feature: Assert I need Psalm to typecheck asserts Background: - Given I have the following code preamble + Given I have the following config + """ + + + + + + + + """ + And I have the following code preamble """ + + + + + + + """ + And I have the following code preamble """ + + + + + + + + + + """ + And I have the following code preamble """ , NS\MyTestCase::class provided | + And I see no other errors Scenario: TestCase::expectException() accepts throwables Given I have the following code """ - class MyTestCase extends TestCase + class MyTestCase extends TestCase { /** @return void */ public function testSomething() { @@ -109,7 +123,7 @@ Feature: TestCase interface I { public function work(): int; } - class MyTestCase extends TestCase + class MyTestCase extends TestCase { /** @var ObjectProphecy */ private $i; @@ -130,4 +144,509 @@ Feature: TestCase When I run Psalm Then I see these errors | Type | Message | - | MissingConstructor | NS\MyTestCase has an uninitialized variable $this->i, but no constructor | + | MissingConstructor | NS\MyTestCase has an uninitialized variable $this->i, but no constructor | + And I see no other errors + + Scenario: Missing data provider is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** + * @param mixed $int + * @return void + * @psalm-suppress UnusedMethod + * @dataProvider provide + */ + public function testSomething($int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | UndefinedMethod | Provider method NS\MyTestCase::provide is not defined | + And I see no other errors + + Scenario: Invalid iterable data provider is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield 1; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidReturnType | Providers must return iterable>, iterable provided | + And I see no other errors + + Scenario: Valid iterable data provider is allowed + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable> */ + public function provide() { + yield [1]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: Invalid generator data provider is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return \Generator */ + public function provide() { + yield 1; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidReturnType | Providers must return iterable>, Generator provided | + And I see no other errors + + Scenario: Valid generator data provider is allowed + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return \Generator,mixed,void> */ + public function provide() { + yield [1]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: Invalid array data provider is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return array */ + public function provide() { + return [1 => 1]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidReturnType | Providers must return iterable>, array provided | + And I see no other errors + + Scenario: Underspecified array data provider is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return array */ + public function provide() { + return [1 => [1]]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidReturnType | Providers must return iterable>, possibly different array provided | + And I see no other errors + + Scenario: Underspecified iterable data provider is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + return [1 => [1]]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidReturnType | Providers must return iterable>, possibly different iterable provided | + And I see no other errors + + Scenario: Underspecified generator data provider is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return \Generator */ + public function provide() { + yield 1 => [1]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidReturnType | Providers must return iterable>, possibly different Generator provided | + And I see no other errors + + Scenario: Valid array data provider is allowed + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return array> */ + public function provide() { + return [ + "data set name" => [1], + ]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: Valid object data provider is allowed + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return \ArrayObject> */ + public function provide() { + return new \ArrayObject([ + "data set name" => [1], + ]); + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: Invalid dataset shape is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield "data set name" => ["str"]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects int, string provided by NS\MyTestCase::provide():(iterable) | + And I see no other errors + + Scenario: Invalid dataset array is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable> */ + public function provide() { + yield "data set name" => ["str"]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | PossiblyInvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects int, string\|int provided by NS\MyTestCase::provide():(iterable>) | + And I see no other errors + + Scenario: Shape dataset with missing params is reported + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield "data set name" => [1]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int, int $i) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | TooFewArguments | Too few arguments for NS\MyTestCase::testSomething - expecting 2 but saw 1 provided by NS\MyTestCase::provide():(iterable) | + And I see no other errors + + Scenario: Referenced providers are not marked as unused + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield "data set name" => [1]; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm with dead code detection + Then I see no errors + + Scenario: Unreferenced providers are marked as unused + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield "data set name" => [1]; + } + /** + * @return void + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm with dead code detection + Then I see these errors + | Type | Message | + | PossiblyUnusedMethod | Cannot find public calls to method NS\MyTestCase::provide | + And I see no other errors + + Scenario: Test method are never marked as unused + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** + * @return void + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + /** + * @return void + * @test + */ + public function somethingElse(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm with dead code detection + Then I see no errors + + Scenario: Unreferenced non-test methods are marked as unused + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** + * @return void + */ + public function somethingElse(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm with dead code detection + Then I see these errors + | Type | Message | + | PossiblyUnusedMethod | Cannot find public calls to method NS\MyTestCase::somethingElse | + And I see no other errors + + Scenario: Unreferenced TestCase descendants are never marked as unused + Given I have the following code + """ + class MyTestCase extends TestCase + { + } + """ + When I run Psalm with dead code detection + Then I see no errors + + Scenario: Unreferenced non-test classes are marked as unused + Given I have the following code + """ + class UtilityClass + { + } + """ + When I run Psalm with dead code detection + Then I see these errors + | Type | Message | + | UnusedClass | Class NS\UtilityClass is never used | + And I see no other errors + + Scenario: Provider returning possibly undefined offset is fine when test method has default for that param + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield "data set name" => rand(0,1) ? [1] : []; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int = 2) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see no errors + + Scenario: Provider returning possibly undefined offset with mismatching type is reported even when test method has default for that param + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield "data set name" => rand(0,1) ? ["1"] : []; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int = 2) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething expects int, string provided by NS\MyTestCase::provide():(iterable) | + And I see no other errors + + Scenario: Provider returning possibly undefined offset is marked when test method has no default for that param + Given I have the following code + """ + class MyTestCase extends TestCase + { + /** @return iterable */ + public function provide() { + yield "data set name" => rand(0,1) ? [1] : []; + } + /** + * @return void + * @dataProvider provide + */ + public function testSomething(int $int) { + $this->assertEquals(1, $int); + } + } + """ + When I run Psalm + Then I see these errors + | Type | Message | + | InvalidArgument | Argument 1 of NS\MyTestCase::testSomething has no default value, but possibly undefined int provided by NS\MyTestCase::provide():(iterable) | + And I see no other errors