tinylib / msgp

A Go code generator for MessagePack / msgpack.org[Go]
MIT License
1.77k stars 189 forks source link

Make `shim` directive handle map key #345

Open phqb opened 2 months ago

phqb commented 2 months ago

I'm using tinylib/msgp to add marshaling/unmarshaling to existing structs. This library works very well in my usecase. I appreciate tinylib/msgp's maintainers a lot.

My structs have a lot of map whose keys are not string and MessagePack requires map key to be string. I can replace all these map keys with string but doing this makes my structs lost some of their readability and I have to do some refactoring.

For an example, I have to change the following struct:

type Foo <some type can convert to/from string> 

type MyStruct struct {
    Map map[Foo]*Bar
}

to

type MyStruct struct {
    MapByFoo map[string]*Bar
}

to maintain readability.

tinylib/msgp already has shim directive that allows any type to be (un)marshaled as another type. I use this feature a lot to avoid refactoring my structs. It will be nice that shim directive also transform non-string map key to string so the following code:

//msgp:shim Foo as:string using:FooToString/StringToFoo

type MyStruct struct {
    Map map[Foo]*Bar
}

will be generated as

// EncodeMsg implements msgp.Encodable
func (z *MyStruct) EncodeMsg(en *msgp.Writer) (err error) {
        ...
    for za0001, za0002 := range z.Map {
        err = en.WriteString(FooToString(za0001)) <-----
        if err != nil {
            err = msgp.WrapError(err, "Map")
            return
        }
        if za0002 == nil {
            err = en.WriteNil()
            if err != nil {
                return
            }
        } else {
            ...
        }
    }
    return
}

// DecodeMsg implements msgp.Decodable
func (z *MyStruct) DecodeMsg(dc *msgp.Reader) (err error) {
    var field []byte
    _ = field
    var zb0001 uint32
    zb0001, err = dc.ReadMapHeader()
    if err != nil {
        err = msgp.WrapError(err)
        return
    }
    for zb0001 > 0 {
        zb0001--
        field, err = dc.ReadMapKeyPtr()
        if err != nil {
            err = msgp.WrapError(err)
            return
        }
        switch msgp.UnsafeString(field) {
        case "Map":
            var zb0002 uint32
            zb0002, err = dc.ReadMapHeader()
            if err != nil {
                err = msgp.WrapError(err, "Map")
                return
            }
            if z.Map == nil {
                z.Map = make(map[string]*Bar, zb0002)
            } else if len(z.Map) > 0 {
                for key := range z.Map {
                    delete(z.Map, key)
                }
            }
            for zb0002 > 0 {
                zb0002--
                var za0001 Foo <-----------
                var za0002 *Bar
                za0001, err = StringToFoo(dc.ReadString()) <-----------
                if err != nil {
                    err = msgp.WrapError(err, "Map")
                    return
                }
                if dc.IsNil() {
                    err = dc.ReadNil()
                    if err != nil {
                        err = msgp.WrapError(err, "Map", za0001)
                        return
                    }
                    za0002 = nil
                } else {
                    ...
                }
                z.Map[za0001] = za0002
            }
                        ...
}
klauspost commented 2 months ago

This looks like a quite unobtrusive change. I prefer that to https://github.com/tinylib/msgp/pull/331 which forces an interface that can be difficult to satisfy for external types.

Ultimately I think "msgp" should allow for all built-in types (except maps) to be keys - perhaps as a commandline opt-in.