Require value types for iterables

[thewilkybarkid]

This is a PR to require that the value types of iterables are defined (eg array<string> rather than just array).

I've implemented return types, but will hold for feedback. The only concern I can see is that it's not possible to allow defining @return array<mixed> (which is defining a value type; can't see how to reliably distinguish between implicit and explicit use of mixed).

(Refs libero/php-coding-standard#40.)

[ondrejmirtes]

Hi,
I think you'd be reliably able to add this feature to existing rules, e.g. MissingFunctionParameterTypehintRule, MissingFunctionReturnTypehintRule, MissingMethodParameterTypehintRule, MissingMethodReturnTypehintRule.

Just ask Type::isIterable()->yes() and then Type::getIterableValueType().

The main hurdle is that it differentiates between implicit and explicit mixed, but the array is currently considered an explicit mixed. So this would be helpful only for native array typehint without any phpDoc, which is a good start.

[ondrejmirtes]

You could also fix the existing rules to register them to InClassMethodNode - the correct reflection is then available in $scope->getFunction() and no exceptions for traits are needed.

[adaamz]

btw I also tried at #35 but there were some reflection issues... is it fixed now?

[ondrejmirtes]

What reflection issues?

[adaamz]

https://travis-ci.org/phpstan/phpstan-strict-rules/jobs/490487289
on interface method parameter of type array, key/item is explicitly mixed.

[ondrejmirtes]

I said that above:

The main hurdle is that it differentiates between implicit and explicit mixed, but the array is currently considered an explicit mixed. So this would be helpful only for native array typehint without any phpDoc, which is a good start.

So it's fine for me if the rule currently addreses native typehints without any item specification in the phpDoc.

[thewilkybarkid]

I had to leave this in to get

phpstan-strict-rules/tests/Rules/Methods/MissingMethodReturnTypehintRuleTest.php

Lines 48 to 63 in a1b5b6e

	[
	'Method MissingMethodReturnTypehint\FooTrait::c1() has no return typehint specified.',
	42,
	],
	[
	'Method MissingMethodReturnTypehint\FooTrait::c2() has a return type array with no value type specified.',
	46,
	],
	[
	'Method MissingMethodReturnTypehint\FooTrait::c3() has a return type iterable with no value type specified.',
	50,
	],
	[
	'Method MissingMethodReturnTypehint\FooTrait::c4() has a return type ArrayObject with no value type specified.',
	54,
	],

to work. Would you not expect to check the trait directly?

[ondrejmirtes]

Please follow my advice I gave here: #70 (comment)

[thewilkybarkid]

I can't get that to work... I end up with the using class name but the line number from the trait (eg 42: Method MissingMethodReturnTypehint\Foo::c1() has no return typehint specified.).

[ondrejmirtes]

Can you push this change as a new commit so I can review it?

[thewilkybarkid]

Pushed 10eee51.

[ondrejmirtes]

The results look fine to me, just the declaring class is wrong there - which is something we might fix in PHPStan's core, but it's not a big deal.

[thewilkybarkid]

@ondrejmirtes Thanks; I've tweaked the tests slightly to remove the override, since it's now testing the class rather than the trait directly.

[thewilkybarkid]

@ondrejmirtes Hopefully all done now.

[adaamz]

@ondrejmirtes Is anything needed to merge this PR?

[ondrejmirtes]

FYI I plan to move these mixed-detecting rules from strict-rules to phpstan/phpstan as a new level 6 for the new version 0.12 :) Stay tuned!

[ondrejmirtes]

I just pushed an implementation of this in phpstan/phpstan: phpstan/phpstan@f58624b

Thank you!