Adjusted MsgPack004 to support records #1946
Conversation
|
@AArnott, have there been any changes or decisions regarding records with primary constructors? The second merged test flags a MsgPack007 for the record that has what I believe is a valid primary constructor. Is this intended behavior or a bug? I annotated this in the source. I will proceed based on your response. |
| { | ||
| string input = Preamble + /* lang=c#-test */ @" | ||
| [MessagePackObject] | ||
| public record {|MsgPack007:Foo|}( |
There was a problem hiding this comment.
I think this is a false-positive for records (but a correct one for normal classes, since the semantics of a „class“- primary ctor are different)
There was a problem hiding this comment.
I like the test as you have it here. Primary constructors for records convert parameters to properties and thus should be serialized. Primary constructors on classes do not convert their parameters to properties and thus should not be serialized. That appears to be what you're asserting here, and all the tests pass, so I'm satisfied. Thank you.
|
I merged the contents of the original PR. However, I believe that MsgPack007 is incorrect for records (but not for classes). If that’s the case, I can provide a fix in another PR. |
| { | ||
| string input = Preamble + /* lang=c#-test */ @" | ||
| [MessagePackObject] | ||
| public record {|MsgPack007:Foo|}( |
There was a problem hiding this comment.
I like the test as you have it here. Primary constructors for records convert parameters to properties and thus should be serialized. Primary constructors on classes do not convert their parameters to properties and thus should not be serialized. That appears to be what you're asserting here, and all the tests pass, so I'm satisfied. Thank you.
This merge adds no content because the change in question was already brought in by #1946.
Merging #1932 from master to develop to fix #1924 in v3.