Merge pull request #3974 from greatroar/cleanup

More cleanups and a micro-optimization

internal/archiver/testing_test.go

@@ -217,7 +217,7 @@ func TestTestWalkFiles(t *testing.T) {
 					return err
 				}
 
-				got[p] = fmt.Sprintf("%v", item)
+				got[p] = fmt.Sprint(item)
 				return nil
 			})
 

internal/cache/file.go

@@ -59,13 +59,14 @@ func (c *Cache) load(h restic.Handle, length int, offset int64) (io.ReadCloser,
 		return nil, errors.WithStack(err)
 	}
 
-	if fi.Size() <= int64(crypto.CiphertextLength(0)) {
+	size := fi.Size()
+	if size <= int64(crypto.CiphertextLength(0)) {
 		_ = f.Close()
 		_ = c.remove(h)
 		return nil, errors.Errorf("cached file %v is truncated, removing", h)
 	}
 
-	if fi.Size() < offset+int64(length) {
+	if size < offset+int64(length) {
 		_ = f.Close()
 		_ = c.remove(h)
 		return nil, errors.Errorf("cached file %v is too small, removing", h)

internal/cache/file_test.go

@@ -97,7 +97,7 @@ func TestFiles(t *testing.T) {
 	}
 
 	for _, tpe := range tests {
-		t.Run(fmt.Sprintf("%v", tpe), func(t *testing.T) {
+		t.Run(tpe.String(), func(t *testing.T) {
 			ids := generateRandomFiles(t, tpe, c)
 			id := randomID(ids)
 

internal/restic/blob.go

@@ -114,11 +114,7 @@ func (h BlobHandles) Less(i, j int) bool {
 			continue
 		}
 
-		if b < h[j].ID[k] {
-			return true
-		}
-
-		return false
+		return b < h[j].ID[k]
 	}
 
 	return h[i].Type < h[j].Type
@@ -133,5 +129,5 @@ func (h BlobHandles) String() string {
 	for _, e := range h {
 		elements = append(elements, e.String())
 	}
-	return fmt.Sprintf("%v", elements)
+	return fmt.Sprint(elements)
 }

internal/restic/file.go

@@ -7,18 +7,38 @@ import (
 )
 
 // FileType is the type of a file in the backend.
-type FileType string
+type FileType uint8
 
 // These are the different data types a backend can store.
 const (
-	PackFile     FileType = "data" // use data, as packs are stored under /data in repo
-	KeyFile      FileType = "key"
-	LockFile     FileType = "lock"
-	SnapshotFile FileType = "snapshot"
-	IndexFile    FileType = "index"
-	ConfigFile   FileType = "config"
+	PackFile FileType = 1 + iota
+	KeyFile
+	LockFile
+	SnapshotFile
+	IndexFile
+	ConfigFile
 )
 
+func (t FileType) String() string {
+	s := "invalid"
+	switch t {
+	case PackFile:
+		// Spelled "data" instead of "pack" for historical reasons.
+		s = "data"
+	case KeyFile:
+		s = "key"
+	case LockFile:
+		s = "lock"
+	case SnapshotFile:
+		s = "snapshot"
+	case IndexFile:
+		s = "index"
+	case ConfigFile:
+		s = "config"
+	}
+	return s
+}
+
 // Handle is used to store and access data in a backend.
 type Handle struct {
 	Type              FileType
@@ -36,10 +56,6 @@ func (h Handle) String() string {
 
 // Valid returns an error if h is not valid.
 func (h Handle) Valid() error {
-	if h.Type == "" {
-		return errors.New("type is empty")
-	}
-
 	switch h.Type {
 	case PackFile:
 	case KeyFile:
@@ -48,7 +64,7 @@ func (h Handle) Valid() error {
 	case IndexFile:
 	case ConfigFile:
 	default:
-		return errors.Errorf("invalid Type %q", h.Type)
+		return errors.Errorf("invalid Type %d", h.Type)
 	}
 
 	if h.Type == ConfigFile {

internal/restic/file_test.go

@@ -1,20 +1,28 @@
 package restic
 
-import "testing"
+import (
+	"testing"
 
-var handleTests = []struct {
-	h     Handle
-	valid bool
-}{
-	{Handle{Name: "foo"}, false},
-	{Handle{Type: "foobar"}, false},
-	{Handle{Type: ConfigFile, Name: ""}, true},
-	{Handle{Type: PackFile, Name: ""}, false},
-	{Handle{Type: "", Name: "x"}, false},
-	{Handle{Type: LockFile, Name: "010203040506"}, true},
+	rtest "github.com/restic/restic/internal/test"
+)
+
+func TestHandleString(t *testing.T) {
+	rtest.Equals(t, "<data/foobar>", Handle{Type: PackFile, Name: "foobar"}.String())
+	rtest.Equals(t, "<lock/1>", Handle{Type: LockFile, Name: "1"}.String())
 }
 
 func TestHandleValid(t *testing.T) {
+	var handleTests = []struct {
+		h     Handle
+		valid bool
+	}{
+		{Handle{Name: "foo"}, false},
+		{Handle{Type: 0}, false},
+		{Handle{Type: ConfigFile, Name: ""}, true},
+		{Handle{Type: PackFile, Name: ""}, false},
+		{Handle{Type: LockFile, Name: "010203040506"}, true},
+	}
+
 	for i, test := range handleTests {
 		err := test.h.Valid()
 		if err != nil && test.valid {

internal/restic/id.go

@@ -6,8 +6,6 @@ import (
 	"fmt"
 	"io"
 
-	"github.com/restic/restic/internal/errors"
-
 	"github.com/minio/sha256-simd"
 )
 
@@ -24,14 +22,13 @@ type ID [idSize]byte
 
 // ParseID converts the given string to an ID.
 func ParseID(s string) (ID, error) {
-	b, err := hex.DecodeString(s)
-
-	if err != nil {
-		return ID{}, errors.Wrap(err, "hex.DecodeString")
+	if len(s) != hex.EncodedLen(idSize) {
+		return ID{}, fmt.Errorf("invalid length for ID: %q", s)
 	}
 
-	if len(b) != idSize {
-		return ID{}, errors.New("invalid length for hash")
+	b, err := hex.DecodeString(s)
+	if err != nil {
+		return ID{}, fmt.Errorf("invalid ID: %s", err)
 	}
 
 	id := ID{}
@@ -96,34 +93,21 @@ func (id ID) MarshalJSON() ([]byte, error) {
 // UnmarshalJSON parses the JSON-encoded data and stores the result in id.
 func (id *ID) UnmarshalJSON(b []byte) error {
 	// check string length
-	if len(b) < 2 {
-		return fmt.Errorf("invalid ID: %q", b)
+	if len(b) != len(`""`)+hex.EncodedLen(idSize) {
+		return fmt.Errorf("invalid length for ID: %q", b)
 	}
 
-	if len(b)%2 != 0 {
-		return fmt.Errorf("invalid ID length: %q", b)
-	}
-
-	// check string delimiters
-	if b[0] != '"' && b[0] != '\'' {
+	if b[0] != '"' {
 		return fmt.Errorf("invalid start of string: %q", b[0])
 	}
 
-	last := len(b) - 1
-	if b[0] != b[last] {
-		return fmt.Errorf("starting string delimiter (%q) does not match end (%q)", b[0], b[last])
-	}
-
-	// strip JSON string delimiters
-	b = b[1:last]
-
-	if len(b) != 2*len(id) {
-		return fmt.Errorf("invalid length for ID")
-	}
+	// Strip JSON string delimiters. The json.Unmarshaler contract says we get
+	// a valid JSON value, so we don't need to check that b[len(b)-1] == '"'.
+	b = b[1 : len(b)-1]
 
 	_, err := hex.Decode(id[:], b)
 	if err != nil {
-		return errors.Wrap(err, "hex.Decode")
+		return fmt.Errorf("invalid ID: %s", err)
 	}
 
 	return nil

internal/restic/ids.go

@@ -65,5 +65,5 @@ func (ids IDs) String() string {
 	for _, id := range ids {
 		elements = append(elements, shortID(id))
 	}
-	return fmt.Sprintf("%v", elements)
+	return fmt.Sprint(elements)
 }

internal/restic/tag_list.go

@@ -37,7 +37,7 @@ func (TagList) Type() string {
 type TagLists []TagList
 
 func (l TagLists) String() string {
-	return fmt.Sprintf("%v", []TagList(l))
+	return fmt.Sprint([]TagList(l))
 }
 
 // Flatten returns the list of all tags provided in TagLists