Fix optional type $description in ActionError can not be set to `null`

[Dmitrev]

Please see this issue for context: https://github.com/slimphp/Slim-Skeleton/issues/289

[l0gicgate]

This would be a breaking change. Instead, we should correct the type to be properly nullable.

[Dmitrev]

@l0gicgate sorry for the late response, how would it be a breaking change?

Setting the value to null would just cause it to crash, feel free to try it! :D

[l0gicgate]

@Dmitrev you just answered your own question. Changing a nullable constructor parameter to non nullable is a breaking change. That means it’s not backward compatible.

I know that this is a skeleton repo and it wouldn’t break things downstream but that’s not the point.

Also, this parameter should be nullable as you may not always pass an error message.

[Dmitrev]

@Dmitrev you just answered your own question. Changing a nullable constructor parameter to non nullable is a breaking change. That means it’s not backward compatible.

I know that this is a skeleton repo and it wouldn’t break things downstream but that’s not the point.

Also, this parameter should be nullable as you may not always pass an error message.

@l0gicgate Sorry I have to disagree with you here. It is already broken as is. Passing null would cause it to crash. If anyone passed null in the constructor in their code base, they would have had to change this.

See example: https://onlinephp.io/c/507ad

Fatal error: Uncaught TypeError: Cannot assign null to property ActionError::$description of type string in /home/user/scripts/code.php:22
Stack trace:
#0 /home/user/scripts/code.php(58): ActionError->__construct('myType', NULL)
#1 {main}
  thrown in /home/user/scripts/code.php on line 22

Same with the setter

https://onlinephp.io/c/4e553

Fatal error: Uncaught TypeError: Cannot assign null to property ActionError::$description of type string in /home/user/scripts/code.php:43
Stack trace:
#0 /home/user/scripts/code.php(59): ActionError->setDescription()
#1 {main}
  thrown in /home/user/scripts/code.php on line 43

It cannot be a breaking change, if it never worked to begin with...

I am happy for it to be nullable, can change that

[l0gicgate]

@Dmitrev you’re right it’s already broken. The constructor is fine, the property itself should be nullable though as I mentioned.

[Dmitrev]

@l0gicgate updated it with your suggestion

[l0gicgate]

Thank you for your contribution @Dmitrev