search

fsprojects / FSharp.Json

F# JSON Reflection based serialization library
Apache License 2.0
222 stars 26 forks source link

Add transform for System.Uri (#8) #9

Closed danyx23 closed 6 years ago

danyx23 commented 6 years ago

I hereby state that this PR along with all my future contributions to this github repository are made under the Apache License 2.0 terms.

danyx23 commented 6 years ago

Please have a look and let me know if I should change anything. Thanks!

I also realized that there is not a lot of type safety when it comes to these attributes - maybe it would be nice if tranforms could specify a base type they can be applied to?

vsapronov commented 6 years ago

Thank you very much for contribution! Could you please do one thing: could you please specify in the description of this PR that you contribute this on Apache License 2.0 terms. This is needed to maintain license clean from different claims. I will merge it then and run release tonight.

vsapronov commented 6 years ago

Sorry @danyx23, I missed your comment about type-safety of Transform. The problem there is that attribute (JsonField) members can't be parametrized based on type of member they are applied to (Uri). This means that there will be always possibility to wrongly apply to field incompatible by target type transform. You will get run-time exception in this case. But I do not think it's possible to provide compile-time type safety. Let me know if have any ideas there - I would be glad to experiment...

danyx23 commented 6 years ago

Yes you are unfortunately right - I hadn't looked into Attributes in a while and only vaguely remembered that you could restrict them but that unfortunately only by a fixed enum that describes various kinds of targets and is thus not flexible enough for further type checking. FxCop or something similar would work but that is another story. Thanks for your comments!