lerouxrgd / rsgen-avro

Command line and library for generating Rust types from Avro schemas
MIT License
37 stars 29 forks source link

allow mut #14

Closed jvenant closed 3 years ago

jvenant commented 3 years ago

Maybe it could be a good idea to add

#[allow(unused_mut)]

at the begining of the file To prevent some warnings on collections ?

lerouxrgd commented 3 years ago

Do you happen to have an example where this would help ?

jvenant commented 3 years ago

Yes sure : I have an avro containing a field like :

    {
      "name": "labels",
      "type": {
        "type": "map",
        "values": "string"
      },
      "default": {}
    }

which is generated as :

            labels: { let mut m = ::std::collections::HashMap::new();  m },

and produce this warning at complile :

   |
51 |             labels: { let mut m = ::std::collections::HashMap::new();  m },
   |                           ----^
   |                           |
   |                           help: remove this `mut`
   |

because I don't edit the hasmap. I just replace it with code similar to :

labels: job.labels.into()

Saying that, I realized that removing the default from the avro changed the generated code to

labels: ::std::collections::HashMap::new()

and of course, this one don't generate any warning.

But that's not exactly what I want. I need a default to an empty map I totally understand why the no default version is immutable and the default version mutable. But for me this mutable behavior is semantically closer to the concept of an init value than a default value. In my mind :

And also maybe, a map with no default should be an Option initialized to None ?

To summarize, I thing that having :

Would be a good compromise and better fit my understanding of what default means

But it's an open discussion. Your library works very well already ! Thank you very much for your work and your contribution !

lerouxrgd commented 3 years ago

Thank you for the example, it is fixed in 0.9.2.

no default converted to a None Option

Actually None is only for union type where first value is null, this is the standard in Avro. So no default is converted to an empty HashMap.

jvenant commented 3 years ago

You're absolutely right for None. Thank you very much !

lerouxrgd commented 3 years ago

Closing this as the issue should be solved.