Mark Primitive, MetaData.PrimitiveDecode() as deprecated

It's not really needed: if you're not sure of the structure you can use any with type switches, and if you want some custom (un)marshal logic you can use the (Un)Marshal interface. Did some basic performance tests: Primitive isn't really faster. It's also not really more convenient. And it does complicate things quite a bit.
BurntSushi · Oct 1, 2023 · 4f8abaa · 4f8abaa
1 parent b41de93
commit 4f8abaa
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 60 deletions.
diff --git a/decode.go b/decode.go
@@ -57,22 +57,6 @@ func DecodeFS(fsys fs.FS, path string, v any) (MetaData, error) {
 	return NewDecoder(fp).Decode(v)
 }
 
-// Primitive is a TOML value that hasn't been decoded into a Go value.
-//
-// This type can be used for any value, which will cause decoding to be delayed.
-// You can use [PrimitiveDecode] to "manually" decode these values.
-//
-// NOTE: The underlying representation of a `Primitive` value is subject to
-// change. Do not rely on it.
-//
-// NOTE: Primitive values are still parsed, so using them will only avoid the
-// overhead of reflection. They can be useful when you don't know the exact type
-// of TOML data until runtime.
-type Primitive struct {
-	undecoded any
-	context   Key
-}
-
 // The significand precision for float32 and float64 is 24 and 53 bits; this is
 // the range a natural number can be stored in a float without loss of data.
 const (
@@ -180,22 +164,6 @@ func (dec *Decoder) Decode(v any) (MetaData, error) {
 	return md, md.unify(p.mapping, rv)
 }
 
-// PrimitiveDecode is just like the other Decode* functions, except it decodes a
-// TOML value that has already been parsed. Valid primitive values can *only* be
-// obtained from values filled by the decoder functions, including this method.
-// (i.e., v may contain more [Primitive] values.)
-//
-// Meta data for primitive values is included in the meta data returned by the
-// Decode* functions with one exception: keys returned by the Undecoded method
-// will only reflect keys that were decoded. Namely, any keys hidden behind a
-// Primitive will be considered undecoded. Executing this method will update the
-// undecoded keys in the meta data. (See the example.)
-func (md *MetaData) PrimitiveDecode(primValue Primitive, v any) error {
-	md.context = primValue.context
-	defer func() { md.context = nil }()
-	return md.unify(primValue.undecoded, rvalue(v))
-}
-
 // unify performs a sort of type unification based on the structure of `rv`,
 // which is the client representation.
 //

diff --git a/deprecated.go b/deprecated.go
@@ -15,6 +15,11 @@ type TextMarshaler encoding.TextMarshaler
 // Deprecated: use encoding.TextUnmarshaler
 type TextUnmarshaler encoding.TextUnmarshaler
 
+// DecodeReader is an alias for NewDecoder(r).Decode(v).
+//
+// Deprecated: use NewDecoder(reader).Decode(&value).
+func DecodeReader(r io.Reader, v any) (MetaData, error) { return NewDecoder(r).Decode(v) }
+
 // PrimitiveDecode is an alias for MetaData.PrimitiveDecode().
 //
 // Deprecated: use MetaData.PrimitiveDecode.
@@ -23,7 +28,40 @@ func PrimitiveDecode(primValue Primitive, v any) error {
 	return md.unify(primValue.undecoded, rvalue(v))
 }
 
-// DecodeReader is an alias for NewDecoder(r).Decode(v).
+// Primitive is a TOML value that hasn't been decoded into a Go value.
 //
-// Deprecated: use NewDecoder(reader).Decode(&value).
-func DecodeReader(r io.Reader, v any) (MetaData, error) { return NewDecoder(r).Decode(v) }
+// This type can be used for any value, which will cause decoding to be delayed.
+// You can use [PrimitiveDecode] to "manually" decode these values.
+//
+// NOTE: The underlying representation of a `Primitive` value is subject to
+// change. Do not rely on it.
+//
+// NOTE: Primitive values are still parsed, so using them will only avoid the
+// overhead of reflection. They can be useful when you don't know the exact type
+// of TOML data until runtime.
+//
+// Deprecated: use Marshaler interface for customer decoding. Or "any"
+// parameters for varying types.
+type Primitive struct {
+	undecoded any
+	context   Key
+}
+
+// PrimitiveDecode is just like the other Decode* functions, except it decodes a
+// TOML value that has already been parsed. Valid primitive values can *only* be
+// obtained from values filled by the decoder functions, including this method.
+// (i.e., v may contain more [Primitive] values.)
+//
+// Meta data for primitive values is included in the meta data returned by the
+// Decode* functions with one exception: keys returned by the Undecoded method
+// will only reflect keys that were decoded. Namely, any keys hidden behind a
+// Primitive will be considered undecoded. Executing this method will update the
+// undecoded keys in the meta data. (See the example.)
+//
+// Deprecated: use Marshaler interface for customer decoding. Or "any"
+// parameters for varying types.
+func (md *MetaData) PrimitiveDecode(primValue Primitive, v any) error {
+	md.context = primValue.context
+	defer func() { md.context = nil }()
+	return md.unify(primValue.undecoded, rvalue(v))
+}
diff --git a/doc.go b/doc.go
@@ -2,9 +2,6 @@
 //
 // This package supports TOML v1.0.0, as specified at https://toml.io
 //
-// There is also support for delaying decoding with the Primitive type, and
-// querying the set of keys in a TOML document with the MetaData type.
-//
 // The github.com/BurntSushi/toml/cmd/tomlv package implements a TOML validator,
 // and can be used to verify if TOML document is valid. It can also be used to
 // print the type of each key.

diff --git a/encode.go b/encode.go
@@ -115,19 +115,14 @@ type Marshaler interface {
 // NOTE: only exported keys are encoded due to the use of reflection. Unexported
 // keys are silently discarded.
 type Encoder struct {
-	// String to use for a single indentation level; default is two spaces.
-	Indent string
-
+	Indent     string // string for a single indentation level; default is two spaces.
+	hasWritten bool   // written any output to w yet?
 	w          *bufio.Writer
-	hasWritten bool // written any output to w yet?
 }
 
 // NewEncoder create a new Encoder.
 func NewEncoder(w io.Writer) *Encoder {
-	return &Encoder{
-		w:      bufio.NewWriter(w),
-		Indent: "  ",
-	}
+	return &Encoder{w: bufio.NewWriter(w), Indent: "  "}
 }
 
 // Encode writes a TOML representation of the Go value to the [Encoder]'s writer.

diff --git a/encode_test.go b/encode_test.go
@@ -1237,7 +1237,13 @@ ArrayOfMixedSlices = [[1, 2], ["a", "b"]]
 }
 
 func TestEncodeDoubleTags(t *testing.T) {
-	// TODO: this needs fixing; it shouldn't emit two 'a =' keys.
+	// This writes two "a" keys to the TOML doc, which isn't valid. I don't
+	// think it's worth spending effort preventing this: best we can do is issue
+	// an error, and should be clear what the problem is anyway. Not even worth
+	// documenting really.
+	//
+	// The json package silently skips these fields, which is worse behaviour
+	// IMO.
 	s := struct {
 		A int `toml:"a"`
 		B int `toml:"a"`

diff --git a/type_fields.go b/type_fields.go
@@ -25,10 +25,8 @@ type field struct {
 // breaking ties with index sequence.
 type byName []field
 
-func (x byName) Len() int { return len(x) }
-
+func (x byName) Len() int      { return len(x) }
 func (x byName) Swap(i, j int) { x[i], x[j] = x[j], x[i] }
-
 func (x byName) Less(i, j int) bool {
 	if x[i].name != x[j].name {
 		return x[i].name < x[j].name
@@ -45,10 +43,8 @@ func (x byName) Less(i, j int) bool {
 // byIndex sorts field by index sequence.
 type byIndex []field
 
-func (x byIndex) Len() int { return len(x) }
-
+func (x byIndex) Len() int      { return len(x) }
 func (x byIndex) Swap(i, j int) { x[i], x[j] = x[j], x[i] }
-
 func (x byIndex) Less(i, j int) bool {
 	for k, xik := range x[i].index {
 		if k >= len(x[j].index) {

diff --git a/type_toml.go b/type_toml.go
@@ -22,13 +22,8 @@ func typeIsTable(t tomlType) bool {
 
 type tomlBaseType string
 
-func (btype tomlBaseType) typeString() string {
-	return string(btype)
-}
-
-func (btype tomlBaseType) String() string {
-	return btype.typeString()
-}
+func (btype tomlBaseType) typeString() string { return string(btype) }
+func (btype tomlBaseType) String() string     { return btype.typeString() }
 
 var (
 	tomlInteger   tomlBaseType = "Integer"