Add support for named strings as inlined map keys #54
Conversation
| @@ -145,7 +145,7 @@ func makeStructFields(root reflect.Type) (structFields, *SemanticError) { | |||
| switch { | |||
| case tf == jsontextValueType: | |||
| f.fncs = nil // specially handled in arshal_inlined.go | |||
| case tf.Kind() == reflect.Map && tf.Key() == stringType: | |||
| case tf.Kind() == reflect.Map && tf.Key().Kind() == reflect.String: | |||
There was a problem hiding this comment.
What happens if tf.Key() implements encoding.TextMarshaler (or encoding.TextAppender), MarshalerV1 or MarshalerV2?
There was a problem hiding this comment.
At mininum, we should forbit any named string type that can marshal itself, but that does open up the door to problematic situations where someone adds a marshaler to named string type after the fact, causing this to no longer be marshalable. However, I think that situation is okay as it's generally not safe to add a marshaler after the fact.
There was a problem hiding this comment.
What happens if tf.Key() implements encoding.TextMarshaler, MarshalerV1 or MarshalerV2?
Any custom marshalers/unmarshalers were ignored for inlined map keys.
I agree that it makes more sense to forbid key types that implement JSON marshal or unmarshal methods. I've added that restriction and included a couple of tests.
that does open up the door to problematic situations where someone adds a marshaler to named string type after the fact, causing this to no longer be marshalable. However, I think that situation is okay as it's generally not safe to add a marshaler after the fact.
FWIW, this is currently the case for inlined structs as well. So, the proposed change remains consistent with the existing implementation.
There isn't a good reason why map[~string]T fields cannot be inlined, unlike map[string]T. This allows map[K]T fields, where K is a ~string type, to be inlined.
nickkhyl
force-pushed
the
nickkhyl/allow-named-string-key-inlining
branch
from
f9a9eeb
to
44d27d4
Compare
|
PTAL? |
There isn't a good reason why
map[~string]Tfields cannot be inlined, unlikemap[string]T. This allowsmap[K]Tfields, whereKis a~stringtype, to be inlined.