search

icerpc / icerpc-csharp

A C# RPC framework built for QUIC, with bidirectional streaming, first-class async/await, and Protobuf support.
https://docs.icerpc.dev
Apache License 2.0
102 stars 13 forks source link

Fix Broken Code Generation for Nested Optional Collections (Fixes #3881) #3884

Closed InsertCreativityHere closed 7 months ago

InsertCreativityHere commented 7 months ago

This small PR fixes the bug found in #3881, and fixes an identical bug that was causing dictionaries to break too. In both cases we weren't correctly handled collections of optional collections. We perform a cast on the inner collection, and these casts weren't taking nullability into account when they needed to.

It also adds some 'tests', which just ensures that the generated code compiles. Apart from the small test added in this PR, we have no tests for nested dictionaries in Slice. We should probably add some.

@bernardnormier believes we can avoid these casts altogether by moving them into the factory. But for now I'd like to get the bug fixed and some basic tests in place before we worry about cleaning things up.

bernardnormier commented 7 months ago

@bernardnormier believes we can avoid these casts altogether by moving them into the factory. But for now I'd like to get the bug fixed and some basic tests in place before we worry about cleaning things up.

After thinking more about it, I think it's cleaner/clearer to leave the factory alone and cast the return value.

InsertCreativityHere commented 7 months ago

I agree the inconsistency is weird. I'm going to merge this PR as is just to fix the issue, and do a few rounds of cleanup on slicec-cs. I'll fix the inconsistency in one of those!