Good morning folks,
I’d like to know your opinion on the following matter. A few years back a lot of people started using and embracing phpstan and its features and applied those in the core. We used phpstan to harden the existing code base, find bugs and prevent new bugs. But we also did something without any consent (at least unknown to me). We used phpstan features in regular phpdoc annotations.
Example 1
phpstan is not only capable of reading @var string $foo and understanding the type of a variable whose type is otherwise unknown, phpstan is capable of being fed information beyond the type system of php itself. @var non-empty-string $foo let’s phpstan know that $foo is expected to be non empty which can then lead to phpstan errors if phpstan detects usages of $foo with potentially empty strings.
Those features of phpstan are great but they have a catch. If we start changing regular phpdoc annotations, we do potentially take the possibility to let other tools properly deal with those. Other tools can be phpdoc itself or IDE’s.
And while it’s great to have phpstan and humans understand non-empty-string, it’s equally important to have IDE’s understand that. But if they don’t we actively remove information from the code base.
Example 2
There is another example of code we do have in the TYPO3 code base. GeneralUtility::makeInstance() expects a $className and to have phpstan understand that we do expect a class name here, we changed @param string $className to @param string|class-string<T> $className. At that point, other tools could still just ignore the class-string<T> and know that $className is the php type string. Later, another patch removed the string type and from that day on my IDE doesn’t know what type $className is expected to have and guesses. It ignores <T> , doesn’t know the global class class-string and thinks it must be \TYPO3\CMS\Core\Utility\class-string.
Sure, using PHPStorm, you can use plugins and meta files and what not and have your IDE understand those phpstan features again or you can hope that Jetbrains integrates native support to have the IDE understand those phpstand features out of the box but what if you use a tool that does not have that support? People loose information which once existed.
IMHO
I am always eager to embrace new tools and features but I think we lost track here and went a bit too far and I’d like us to be a bit more conservative, quite literally, here.
Even though very few use it, there still is phpdocumentor with a defined set of supported types (phpDocumentor) and back then before PHP introduced type declarations for return, param and property types, we usually went with the set of defined types of phpdocumentor.
I think we should stick to those with regular @param, @return and @var annotations and add additional information, that is only consumable by phpstan, via @phpstan--variants. I’d like to put emphasis on add here. Instead of
/**
* @template T of object
* @param class-string<T> $className
* @param array<int, mixed> $constructorArguments
* @return T
*/
public static function makeInstance($className, ...$constructorArguments)
{
}
GeneralUtility::makeInstance() would look like this:
/**
* @param string $className
* @param mixed[] $constructorArguments
* @return object
*
* @template T of object
* @phpstan-param class-string<T> $className
* @phpstan-param array<int, mixed> $constructorArguments
* @phpstan-return T
*/
public static function makeInstance($className, ...$constructorArguments)
{
}
Yes, phpstan and humans find duplicate information here but other tools still work as expected.
What’s your take on this?
I’d like to know what you think about it, without going into detail about what specific annotation should remain, look like and/or replaced/enhanced with phpstan features. First, I’d like to get a feeling about how the community thinks about this in general. If the majority doesn’t share my concerns here, there is no need to discuss details.
I am using PHPStorm myself and I’d like to hear what you think about this, what tool(s) you are working with and how those already made changes affect your work and what you think the best solution would be.
(it’s just a tool)