encoding/xml: improve []byte handling

Marshalling of []byte in attributes and the general
marshalling of named []byte types was fixed.

A []byte field also won't be nil if an XML element
was mapped to it, even if the element is empty.

Tests were introduced to make sure that *struct{}
fields works correctly for element presence testing.
No changes to the logic made in that regard.

R=rsc
CC=golang-dev
https://golang.org/cl/5539070

src/pkg/encoding/xml/marshal.go

@@ -181,23 +181,43 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo) error {
 		if finfo.flags&fAttr == 0 {
 			continue
 		}
-		var str string
+		fv := val.FieldByIndex(finfo.idx)
-		if fv := val.FieldByIndex(finfo.idx); fv.Kind() == reflect.String {
+		switch fv.Kind() {
-			str = fv.String()
+		case reflect.String, reflect.Array, reflect.Slice:
-		} else {
+			// TODO: Should we really do this once ,omitempty is in?
-			str = fmt.Sprint(fv.Interface())
+			if fv.Len() == 0 {
+				continue
+			}
 		}
-		if str != "" {
+		p.WriteByte(' ')
-			p.WriteByte(' ')
+		p.WriteString(finfo.name)
-			p.WriteString(finfo.name)
+		p.WriteString(`="`)
-			p.WriteString(`="`)
+		if err := p.marshalSimple(fv.Type(), fv); err != nil {
-			Escape(p, []byte(str))
+			return err
-			p.WriteByte('"')
 		}
+		p.WriteByte('"')
 	}
 	p.WriteByte('>')
 
-	switch k := val.Kind(); k {
+	if val.Kind() == reflect.Struct {
+		err = p.marshalStruct(tinfo, val)
+	} else {
+		err = p.marshalSimple(typ, val)
+	}
+	if err != nil {
+		return err
+	}
+
+	p.WriteByte('<')
+	p.WriteByte('/')
+	p.WriteString(name)
+	p.WriteByte('>')
+
+	return nil
+}
+
+func (p *printer) marshalSimple(typ reflect.Type, val reflect.Value) error {
+	switch val.Kind() {
 	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
 		p.WriteString(strconv.FormatInt(val.Int(), 10))
 	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
@@ -205,6 +225,7 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo) error {
 	case reflect.Float32, reflect.Float64:
 		p.WriteString(strconv.FormatFloat(val.Float(), 'g', -1, 64))
 	case reflect.String:
+		// TODO: Add EscapeString.
 		Escape(p, []byte(val.String()))
 	case reflect.Bool:
 		p.WriteString(strconv.FormatBool(val.Bool()))
@@ -217,21 +238,10 @@ func (p *printer) marshalValue(val reflect.Value, finfo *fieldInfo) error {
 		Escape(p, bytes)
 	case reflect.Slice:
 		// will be []byte
-		bytes := val.Interface().([]byte)
+		Escape(p, val.Bytes())
-		Escape(p, bytes)
-	case reflect.Struct:
-		if err := p.marshalStruct(tinfo, val); err != nil {
-			return err
-		}
 	default:
 		return &UnsupportedTypeError{typ}
 	}
-
-	p.WriteByte('<')
-	p.WriteByte('/')
-	p.WriteString(name)
-	p.WriteByte('>')
-
 	return nil
 }
 

src/pkg/encoding/xml/marshal_test.go

@@ -184,6 +184,18 @@ type RecurseB struct {
 	B string
 }
 
+type PresenceTest struct {
+	Exists *struct{}
+}
+
+type MyBytes []byte
+
+type Data struct {
+	Bytes  []byte
+	Attr   []byte `xml:",attr"`
+	Custom MyBytes
+}
+
 type Plain struct {
 	V interface{}
 }
@@ -225,6 +237,44 @@ var marshalTests = []struct {
 	{Value: &Plain{[]int{1, 2, 3}}, ExpectXML: `<Plain><V>1</V><V>2</V><V>3</V></Plain>`},
 	{Value: &Plain{[3]int{1, 2, 3}}, ExpectXML: `<Plain><V>1</V><V>2</V><V>3</V></Plain>`},
 
+	// A pointer to struct{} may be used to test for an element's presence.
+	{
+		Value:     &PresenceTest{new(struct{})},
+		ExpectXML: `<PresenceTest><Exists></Exists></PresenceTest>`,
+	},
+	{
+		Value:     &PresenceTest{},
+		ExpectXML: `<PresenceTest></PresenceTest>`,
+	},
+
+	// A pointer to struct{} may be used to test for an element's presence.
+	{
+		Value:     &PresenceTest{new(struct{})},
+		ExpectXML: `<PresenceTest><Exists></Exists></PresenceTest>`,
+	},
+	{
+		Value:     &PresenceTest{},
+		ExpectXML: `<PresenceTest></PresenceTest>`,
+	},
+
+	// A []byte field is only nil if the element was not found.
+	{
+		Value:         &Data{},
+		ExpectXML:     `<Data></Data>`,
+		UnmarshalOnly: true,
+	},
+	{
+		Value:         &Data{Bytes: []byte{}, Custom: MyBytes{}, Attr: []byte{}},
+		ExpectXML:     `<Data Attr=""><Bytes></Bytes><Custom></Custom></Data>`,
+		UnmarshalOnly: true,
+	},
+
+	// Check that []byte works, including named []byte types.
+	{
+		Value:     &Data{Bytes: []byte("ab"), Custom: MyBytes("cd"), Attr: []byte{'v'}},
+		ExpectXML: `<Data Attr="v"><Bytes>ab</Bytes><Custom>cd</Custom></Data>`,
+	},
+
 	// Test innerxml
 	{
 		Value: &SecretAgent{

src/pkg/encoding/xml/read.go

@@ -134,7 +134,7 @@ import (
 //
 // Unmarshal maps an XML element to a string or []byte by saving the
 // concatenation of that element's character data in the string or
-// []byte.
+// []byte. The saved []byte is never nil.
 //
 // Unmarshal maps an attribute value to a string or []byte by saving
 // the value in the string or slice.
@@ -309,14 +309,12 @@ func (p *Parser) unmarshal(val reflect.Value, start *StartElement) error {
 			case fAttr:
 				strv := sv.FieldByIndex(finfo.idx)
 				// Look for attribute.
-				val := ""
 				for _, a := range start.Attr {
 					if a.Name.Local == finfo.name {
-						val = a.Value
+						copyValue(strv, []byte(a.Value))
 						break
 					}
 				}
-				copyValue(strv, []byte(val))
 
 			case fCharData:
 				if !saveData.IsValid() {
@@ -473,7 +471,11 @@ func copyValue(dst reflect.Value, src []byte) (err error) {
 	case reflect.String:
 		t.SetString(string(src))
 	case reflect.Slice:
-		t.Set(reflect.ValueOf(src))
+		if len(src) == 0 {
+			// non-nil to flag presence
+			src = []byte{}
+		}
+		t.SetBytes(src)
 	}
 	return nil
 }