estratocloud / edifact

Parser and Serializer for UN/EDIFACT messages in PHP
Apache License 2.0
35 stars 9 forks source link

Strict types breaks serialization #11

Open troelskn opened 4 years ago

troelskn commented 4 years ago

Since strict typing was added, I get this error in my application:

TypeError: Argument 1 passed to Metroplex\Edifact\Serializer::escape() must be of the type string, null given, called in /var/app/gas/vendor/metroplex-systems/edifact/src/Serializer.php on line 61

/var/app/gas/vendor/metroplex-systems/edifact/src/Serializer.php:79
/var/app/gas/vendor/metroplex-systems/edifact/src/Serializer.php:61
/var/app/gas/vendor/metroplex-systems/edifact/src/Message.php:151
...

The problem is that I have added non-strings (null, integers) as values to segments as I build them up. While I could change the code on my end to coerce into string, I don't think it would be unreasonable for the library to do this transparently, as it used to. I suggest changing Serializer::escape as follows:

-     private function escape(string $string): string
-    {
-        $characters = [
+    private function escape($string): string
+    {
+        $string = (string) $string;
+        $characters = [

Would you like me to open a PR for this?

duncan3dc commented 3 years ago

Hi @troelskn sorry for the delay here! Transparently converting here seems reasonable, however I'd like to bundle this with a clear error for when the value cannot be escaped, if you're like to throw up a PR for that then I would be happy to take a look, if not I'll tackle this myself when I get a moment