coding-standard
coding-standard copied to clipboard
DisallowArrayTypeHintSyntax issue with collection objects
/**
* @param \Propel\Runtime\Collection\ObjectCollection|\Orm\Zed\ExampleStateMachine\Persistence\PyzExampleStateMachineItem[] $exampleStateMachineItems
*
* @return array<\Generated\Shared\Transfer\StateMachineItemTransfer>
*/
protected function hydrateTransferFromPersistence($exampleStateMachineItems)
results in
54 | ERROR | [x] Usage of array type hint syntax in
| | "\Orm\Zed\ExampleStateMachine\Persistence\PyzExampleStateMachineItem[]"
| | is disallowed, use generic type hint syntax
| | instead.
| | (SlevomatCodingStandard.TypeHints.DisallowArrayTypeHintSyntax.DisallowedArrayTypeHintSyntax)
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
We would like to use the sniff to fix all trivial cases of array vs generics, but we do need to keep object ones for IDE and PHPStan compatibility.
In other words: The sniff fixes it to proposed \Propel\Runtime\Collection\ObjectCollection|array<\Orm\Zed\ExampleStateMachine\Persistence\PyzExampleStateMachineItem>
which is invalid as per PHPStan. Rightfully so, as this is now a collection OR some iterable array instead of the "old legacy syntax of saying "iterable object of this type".
PHPDoc tag @param for parameter $entities with type array<DateTimeImmutable>|Test\Collection is not subtype of native type Test\Collection.andhas parameter $entities with no value type specified in iterable type Test\CollectionIt does however not complain with legacy syntax\Object|type[].
If anything it would need to be \Propel\Runtime\Collection\ObjectCollection<\Orm\Zed\ExampleStateMachine\Persistence\PyzExampleStateMachineItem> to be valid, but this is not yet understood by most IDEs.
So we would like to only let it fix the trivial cases of generics (specifically array/iterable), maybe some of the trivial collections IDEs already understand like ArrayObject/ArrayAccess etc, but that does not need to be.
So far it doesnt seem to have an option to toggle this behavior. It would be awesome if you could consider providing a config value for this - and also if the above would fix it to the correct generic type then instead of making this PHPStan/Psalm invalid.
I solved it locally by adding
/**
* The following classes are supported for object generics by IDEs like PHPStorm already.
* E.g. `\ArrayObject<type>` instead of legacy syntax `\ArrayObject|type[]`.
*
* @var array<string>
*/
protected static $genericCollectionClasses = [
'\\Traversable',
'\\ArrayAccess',
'\\ArrayObject',
'\\Generator',
'\\Iterator',
];
and adding
/**
* These generic object collections are not yet understood by IDEs like PHPStorm.
*
* @param array<\PHPStan\PhpDocParser\Ast\Type\TypeNode> $types
*
* @return bool
*/
protected function isComplexGenericObjectCollection(array $types): bool
{
foreach ($types as $type) {
if (!$type instanceof IdentifierTypeNode) {
continue;
}
$className = (string)$type;
if (strpos((string)$type, '\\') === 0 && !in_array($className, static::$genericCollectionClasses, true)) {
return true;
}
}
return false;
}
/**
* @param array<\PHPStan\PhpDocParser\Ast\Type\TypeNode> $types
*
* @return bool
*/
protected function containsArrayTypeNode(array $types): bool
{
foreach ($types as $type) {
if (!$type instanceof ArrayTypeNode) {
continue;
}
if ($type->type instanceof IdentifierTypeNode) {
return true;
}
}
return false;
}
and doing a check inside main method:
if ($annotation->isInvalid()) {
continue;
}
// here
if ($this->isGenericObject($annotation)) {
continue;
}
Well, this just skipped things
It would still be nice to have this feature to auto convert those "collections" to the new "generic" syntax: Also, e.g.
/**
- * @return \Iterator|int[]
+ * @return \Iterator|array<int>
*/
should be fixed to
/**
- * @return \Iterator|int[]
+ * @return \Iterator<int>
*/
As this is what is meant with the legacy syntax.