Use nil as zero value for Multiaddr

Author: MarcoPolo

Diff for: codec.go

@@ -119,7 +119,7 @@ func readComponent(b []byte) (int, Component, error) {
 
 func readMultiaddr(b []byte) (int, Multiaddr, error) {
 	if len(b) == 0 {
-		return 0, Multiaddr{}, fmt.Errorf("empty multiaddr")
+		return 0, nil, fmt.Errorf("empty multiaddr")
 	}
 
 	var res Multiaddr
@@ -128,14 +128,14 @@ func readMultiaddr(b []byte) (int, Multiaddr, error) {
 	for len(b) > 0 {
 		n, c, err := readComponent(b)
 		if err != nil {
-			return 0, Multiaddr{}, err
+			return 0, nil, err
 		}
 		b = b[n:]
 		bytesRead += n
 
 		if sawPathComponent {
 			// It is an error to have another component after a path component.
-			return bytesRead, Multiaddr{}, fmt.Errorf("unexpected component after path component")
+			return bytesRead, nil, fmt.Errorf("unexpected component after path component")
 		}
 		sawPathComponent = c.protocol.Path
 		res = append(res, c)

Diff for: component.go

@@ -17,6 +17,9 @@ type Component struct {
 }
 
 func (c Component) AsMultiaddr() Multiaddr {
+	if c.Empty() {
+		return nil
+	}
 	return []Component{c}
 }
 
@@ -127,6 +130,9 @@ func (c Component) RawValue() []byte {
 }
 
 func (c Component) Value() string {
+	if c.Empty() {
+		return ""
+	}
 	// This Component MUST have been checked by validateComponent when created
 	value, _ := c.valueAndErr()
 	return value
@@ -180,7 +186,6 @@ func NewComponent(protocol, value string) (Component, error) {
 		return Component{}, fmt.Errorf("protocol %s doesn't take a value", p.Name)
 	}
 	return newComponent(p, nil)
-	// TODO: handle path /?
 }
 
 func newComponent(protocol Protocol, bvalue []byte) (Component, error) {

Diff for: multiaddr.go

@@ -37,7 +37,7 @@ func NewMultiaddr(s string) (a Multiaddr, err error) {
 	}()
 	b, err := stringToBytes(s)
 	if err != nil {
-		return Multiaddr{}, err
+		return nil, err
 	}
 	return NewMultiaddrBytes(b)
 }
@@ -53,7 +53,10 @@ func NewMultiaddrBytes(b []byte) (a Multiaddr, err error) {
 	}()
 	bytesRead, m, err := readMultiaddr(b)
 	if bytesRead != len(b) {
-		return Multiaddr{}, fmt.Errorf("Unexpected extra data. %v bytes leftover", len(b)-bytesRead)
+		return nil, fmt.Errorf("Unexpected extra data. %v bytes leftover", len(b)-bytesRead)
+	}
+	if len(m) == 0 {
+		return nil, err
 	}
 	return m, err
 }
@@ -175,6 +178,16 @@ func (m Multiaddr) Encapsulate(o Multiaddr) Multiaddr {
 	return Join(m, o)
 }
 
+func (m Multiaddr) EncapsulateC(c Component) Multiaddr {
+	if c.Empty() {
+		return m
+	}
+	out := make([]Component, 0, len(m)+1)
+	out = append(out, m...)
+	out = append(out, c)
+	return out
+}
+
 // Decapsulate unwraps Multiaddr up until the given Multiaddr is found.
 func (m Multiaddr) Decapsulate(rightParts Multiaddr) Multiaddr {
 	leftParts := m
@@ -200,7 +213,7 @@ func (m Multiaddr) Decapsulate(rightParts Multiaddr) Multiaddr {
 	}
 
 	if lastIndex == 0 {
-		return Multiaddr{}
+		return nil
 	}
 
 	if lastIndex < 0 {
@@ -262,7 +275,7 @@ func Unique(addrs []Multiaddr) []Multiaddr {
 		}
 	}
 	for i := idx; i < len(addrs); i++ {
-		addrs[i] = Multiaddr{}
+		addrs[i] = nil
 	}
 	return addrs[:idx]
 }

Diff for: multiaddr_test.go

@@ -21,6 +21,53 @@ func newMultiaddr(t *testing.T, a string) Multiaddr {
 	return m
 }
 
+func TestReturnsNilOnEmpty(t *testing.T) {
+	a := StringCast("/ip4/1.2.3.4")
+	a, _ = SplitLast(a)
+	require.Nil(t, a)
+	a, _ = SplitLast(a)
+	require.Nil(t, a)
+
+	// Test that empty multiaddr from various operations returns nil
+	a = StringCast("/ip4/1.2.3.4/tcp/1234")
+	_, a = SplitFirst(a)
+	_, a = SplitFirst(a)
+	require.Nil(t, a)
+	_, a = SplitFirst(a)
+	require.Nil(t, a)
+
+	a = StringCast("/ip4/1.2.3.4/tcp/1234")
+	a = a.Decapsulate(a)
+	require.Nil(t, a)
+
+	a = StringCast("/ip4/1.2.3.4/tcp/1234")
+	a = a.Decapsulate(StringCast("/tcp/1234"))
+	a = a.Decapsulate(StringCast("/ip4/1.2.3.4"))
+	require.Nil(t, a)
+
+	// Test that SplitFunc returns nil when we split at beginning and end
+	a = StringCast("/ip4/1.2.3.4/tcp/1234")
+	pre, _ := SplitFunc(a, func(c Component) bool {
+		return c.Protocol().Code == P_IP4
+	})
+	require.Nil(t, pre)
+
+	a = StringCast("/ip4/1.2.3.4/tcp/1234")
+	_, post := SplitFunc(a, func(c Component) bool {
+		return false
+	})
+	require.Nil(t, post)
+
+	_, err := NewMultiaddr("")
+	require.Error(t, err)
+
+	a = JoinComponents()
+	require.Nil(t, a)
+
+	a = Join()
+	require.Nil(t, a)
+}
+
 func TestConstructFails(t *testing.T) {
 	cases := []string{
 		"/ip4",
@@ -459,7 +506,7 @@ func TestEncapsulate(t *testing.T) {
 
 	m4, _ := NewMultiaddr("/ip4/127.0.0.1")
 	d := c.Decapsulate(m4)
-	if !d.Empty() {
+	if d != nil {
 		t.Error("decapsulate /ip4 failed: ", d)
 	}
 }
@@ -480,7 +527,7 @@ func TestDecapsulateComment(t *testing.T) {
 
 	m = StringCast("/ip4/1.2.3.4/tcp/80")
 	rest = m.Decapsulate(StringCast("/ip4/1.2.3.4"))
-	require.True(t, rest.Empty(), "expected a nil multiaddr if we decapsulate everything")
+	require.Nil(t, rest, "expected a nil multiaddr if we decapsulate everything")
 }
 
 func TestDecapsulate(t *testing.T) {
@@ -513,7 +560,7 @@ func TestDecapsulate(t *testing.T) {
 			actualMa := left.Decapsulate(right)
 
 			if tc.expected == "" {
-				require.True(t, actualMa.Empty(), "expected nil")
+				require.Nil(t, actualMa, "expected nil")
 				return
 			}
 

Diff for: net/convert.go

@@ -106,7 +106,7 @@ func FromIPAndZone(ip net.IP, zone string) (ma.Multiaddr, error) {
 	case ip.To4() != nil:
 		c, err := ma.NewComponent("ip4", ip.String())
 		if err != nil {
-			return ma.Multiaddr{}, err
+			return nil, err
 		}
 		return c.AsMultiaddr(), nil
 	case ip.To16() != nil:

Diff for: util.go

@@ -12,6 +12,9 @@ func Split(m Multiaddr) []Component {
 }
 
 func JoinComponents(cs ...Component) Multiaddr {
+	if len(cs) == 0 {
+		return nil
+	}
 	out := make([]Component, 0, len(cs))
 	for _, c := range cs {
 		if !c.Empty() {
@@ -27,6 +30,9 @@ func Join(ms ...Multiaddr) Multiaddr {
 	for _, m := range ms {
 		size += len(m)
 	}
+	if size == 0 {
+		return nil
+	}
 
 	out := make([]Component, 0, size)
 	for _, m := range ms {
@@ -60,15 +66,22 @@ func StringCast(s string) Multiaddr {
 // SplitFirst returns the first component and the rest of the multiaddr.
 func SplitFirst(m Multiaddr) (Component, Multiaddr) {
 	if m.Empty() {
-		return Component{}, Multiaddr{}
+		return Component{}, nil
+	}
+	if len(m) == 1 {
+		return m[0], nil
 	}
 	return m[0], m[1:]
 }
 
 // SplitLast returns the rest of the multiaddr and the last component.
 func SplitLast(m Multiaddr) (Multiaddr, Component) {
 	if m.Empty() {
-		return Multiaddr{}, Component{}
+		return nil, Component{}
+	}
+	if len(m) == 1 {
+		// We want to explicitly return a nil slice if the prefix is now empty.
+		return nil, m[0]
 	}
 	return m[:len(m)-1], m[len(m)-1]
 }
@@ -78,7 +91,7 @@ func SplitLast(m Multiaddr) (Multiaddr, Component) {
 // *second* multiaddr.
 func SplitFunc(m Multiaddr, cb func(Component) bool) (Multiaddr, Multiaddr) {
 	if m.Empty() {
-		return Multiaddr{}, Multiaddr{}
+		return nil, nil
 	}
 
 	idx := len(m)
@@ -88,7 +101,14 @@ func SplitFunc(m Multiaddr, cb func(Component) bool) (Multiaddr, Multiaddr) {
 			break
 		}
 	}
-	return m[:idx], m[idx:]
+	pre, post := m[:idx], m[idx:]
+	if pre.Empty() {
+		pre = nil
+	}
+	if post.Empty() {
+		post = nil
+	}
+	return pre, post
 }
 
 // ForEach walks over the multiaddr, component by component.

Diff for: util_test.go

@@ -22,16 +22,16 @@ func TestSplitFirstLast(t *testing.T) {
 		rest, last := SplitLast(addr)
 		if len(x) == 0 {
 			if !head.Empty() {
-				t.Error("expected head to be nil")
+				t.Error("expected head to be empty")
 			}
-			if !tail.Empty() {
+			if tail != nil {
 				t.Error("expected tail to be nil")
 			}
-			if !rest.Empty() {
+			if rest != nil {
 				t.Error("expected rest to be nil")
 			}
 			if !last.Empty() {
-				t.Error("expected last to be nil")
+				t.Error("expected last to be empty")
 			}
 			continue
 		}
@@ -42,10 +42,10 @@ func TestSplitFirstLast(t *testing.T) {
 			t.Errorf("expected %s to be %s", head, x[len(x)-1])
 		}
 		if len(x) == 1 {
-			if !tail.Empty() {
+			if tail != nil {
 				t.Error("expected tail to be nil")
 			}
-			if !rest.Empty() {
+			if rest != nil {
 				t.Error("expected rest to be nil")
 			}
 			continue
@@ -66,11 +66,11 @@ func TestSplitFirstLast(t *testing.T) {
 	}
 
 	ci, m := SplitFirst(c.AsMultiaddr())
-	if !ci.Equal(c) || !m.Empty() {
+	if !ci.Equal(c) || m != nil {
 		t.Error("split first on component failed")
 	}
 	m, ci = SplitLast(c.AsMultiaddr())
-	if !ci.Equal(c) || !m.Empty() {
+	if !ci.Equal(c) || m != nil {
 		t.Error("split last on component failed")
 	}
 	cis := Split(c.AsMultiaddr())
@@ -80,13 +80,13 @@ func TestSplitFirstLast(t *testing.T) {
 	m1, m2 := SplitFunc(c.AsMultiaddr(), func(c Component) bool {
 		return true
 	})
-	if !m1.Empty() || !m2.Equal(c.AsMultiaddr()) {
+	if m1 != nil || !m2.Equal(c.AsMultiaddr()) {
 		t.Error("split func(true) on component failed")
 	}
 	m1, m2 = SplitFunc(c.AsMultiaddr(), func(c Component) bool {
 		return false
 	})
-	if !m1.Equal(c.AsMultiaddr()) || !m2.Empty() {
+	if !m1.Equal(c.AsMultiaddr()) || m2 != nil {
 		t.Error("split func(false) on component failed")
 	}