Testing - Return types, hints, phpstan

[michalcharvat]

At the moment there is a mess in the code. How do you want to test code like:

$entityType = \go\core\orm\EntityType::findById($this->entityTypeId);
$className = $entityType->getClassName();

You cant actually use if (!$entityType) {...} because the return type is bool and entityType could be also true. Perhaps all functions returning something byId, by should return ?stratic. Sure you can create hint to fool phpstan but then you would hide real error. For example

groupoffice/www/go/core/model/Link.php

Lines 127 to 130 in 49b238c

	public function setFromEntity(string $entityName) {
	$e = EntityType::findByName($entityName);
	$this->fromEntity = $e->getName();
	$this->fromEntityTypeId = $e->getId();

would raise the exception to calling getName on bool

Sometimes phpstan is the hell but it really uncover real errors in the code.

I can help you with that but if I am not wrong my last PR with hints went to trash, so I dont want waste my time with sometime you dont want, need or merge. Please let me know.

[mschering]

Hi Michal,

Sorry for the late reply. I appreciate your efforts? Which PR went to trash?

I agree that it's better to return ?static. But I don't understand the problem phpstan is having with false| static?

Best regards,
Merijn

[michalcharvat]

Hi Merijn,

I think in the original code it was bool | static and not false | static. However the biggest problem than hint from these findByX is the usage -> return \go\core\orm\EntityType::findById($this->entityTypeId)->getName(); In this case you have only two options -> throw the exception if there isnt the type with ID or update the code and check whether that entityType exists and if yes return the name. Problem appears in the sitation where you expect the entity but it does not exists. I would say in most cases it would be really solved by throwing the exception but its up to you :-)

[mschering]

Ok I went for the exception too as in the code it's always assumed it's there.