x/model: add roundtrip for String test

x/model/digest_test.go

@@ -49,5 +49,10 @@ func TestDigestString(t *testing.T) {
 		if got != want {
 			t.Errorf("ParseDigest(%q).String() = %q; want %q", s, got, want)
 		}
+
+		got = ParseDigest(s).String()
+		if got != want {
+			t.Errorf("roundtrip ParseDigest(%q).String() = %q; want %q", s, got, want)
+		}
 	}
 }

x/model/name.go

@@ -44,18 +44,16 @@ const (
 	//
 	// It should be kept as the last part in the list.
 	PartInvalid
-
-	NumParts = PartInvalid
 )
 
 var kindNames = map[NamePart]string{
-	PartInvalid:   "Invalid",
 	PartHost:      "Host",
 	PartNamespace: "Namespace",
 	PartModel:     "Name",
 	PartTag:       "Tag",
 	PartBuild:     "Build",
 	PartDigest:    "Digest",
+	PartInvalid:   "Invalid",
 }
 
 func (k NamePart) String() string {
@@ -93,8 +91,9 @@ func (k NamePart) String() string {
 //
 // To make a Name by filling in missing parts from another Name, use [Fill].
 type Name struct {
-	_     structs.Incomparable
+	_      structs.Incomparable
-	parts [NumParts]string
+	parts  [5]string // host, namespace, model, tag, build
+	digest Digest    // digest is a special part
 
 	// TODO(bmizerany): track offsets and hold s (raw string) here? We
 	// could pack the offests all into a single uint64 since the first
@@ -141,6 +140,13 @@ func ParseName(s string) Name {
 		if kind == PartInvalid {
 			return Name{}
 		}
+		if kind == PartDigest {
+			r.digest = ParseDigest(part)
+			if !r.digest.Valid() {
+				return Name{}
+			}
+			continue
+		}
 		r.parts[kind] = part
 	}
 	if r.Valid() || r.Resolved() {
@@ -233,8 +239,7 @@ var seps = [...]string{
 	PartNamespace: "/",
 	PartModel:     ":",
 	PartTag:       "+",
-	PartBuild:     "@",
+	PartBuild:     "",
-	PartDigest:    "",
 }
 
 // WriteTo implements io.WriterTo. It writes the fullest possible display
@@ -247,25 +252,22 @@ var seps = [...]string{
 // The full digest is always prefixed with "@". That is if [Name.Valid]
 // reports false and [Name.Resolved] reports true, then the string is
 // returned as "@<digest-type>-<digest>".
-func (r Name) WriteTo(w io.Writer) (n int64, err error) {
+func (r Name) writeTo(w io.StringWriter) {
+	var partsWritten int
 	for i := range r.parts {
 		if r.parts[i] == "" {
 			continue
 		}
-		if n > 0 || NamePart(i) == PartDigest {
+		if partsWritten > 0 {
-			n1, err := io.WriteString(w, seps[i-1])
+			w.WriteString(seps[i-1])
-			n += int64(n1)
-			if err != nil {
-				return n, err
-			}
-		}
-		n1, err := io.WriteString(w, r.parts[i])
-		n += int64(n1)
-		if err != nil {
-			return n, err
 		}
+		w.WriteString(r.parts[i])
+		partsWritten++
+	}
+	if r.Resolved() {
+		w.WriteString("@")
+		w.WriteString(r.digest.String())
 	}
-	return n, nil
 }
 
 var builderPool = sync.Pool{
@@ -287,7 +289,7 @@ func (r Name) String() string {
 	defer builderPool.Put(b)
 	b.Reset()
 	b.Grow(50) // arbitrarily long enough for most names
-	_, _ = r.WriteTo(b)
+	r.writeTo(b)
 	return b.String()
 }
 
@@ -296,11 +298,13 @@ func (r Name) String() string {
 // returns a string that includes all parts of the Name, with missing parts
 // replaced with a ("?").
 func (r Name) GoString() string {
-	var v Name
 	for i := range r.parts {
-		v.parts[i] = cmp.Or(r.parts[i], "?")
+		r.parts[i] = cmp.Or(r.parts[i], "?")
 	}
-	return v.String()
+	if !r.Resolved() {
+		r.digest = Digest{"?", "?"}
+	}
+	return r.String()
 }
 
 // LogValue implements slog.Valuer.
@@ -320,10 +324,7 @@ func (r Name) MarshalText() ([]byte, error) {
 	b.Reset()
 	b.Grow(50) // arbitrarily long enough for most names
 	defer bufPool.Put(b)
-	_, err := r.WriteTo(b)
+	r.writeTo(b)
-	if err != nil {
-		return nil, err
-	}
 	// TODO: We can remove this alloc if/when
 	// https://github.com/golang/go/issues/62384 lands.
 	return b.Bytes(), nil
@@ -393,15 +394,15 @@ func (r Name) CompleteNoBuild() bool {
 // It is possible to have a valid Name, or a complete Name that is not
 // resolved.
 func (r Name) Resolved() bool {
-	return r.parts[PartDigest] != ""
+	return r.digest.Valid()
 }
 
 // Digest returns the digest part of the Name, if any.
 //
 // If Digest returns a non-empty string, then [Name.Resolved] will return
 // true, and digest is considered valid.
-func (r Name) Digest() string {
+func (r Name) Digest() Digest {
-	return r.parts[PartDigest]
+	return r.digest
 }
 
 // EqualFold reports whether r and o are equivalent model names, ignoring
@@ -479,24 +480,14 @@ func Parts(s string) iter.Seq2[NamePart, string] {
 			case '@':
 				switch state {
 				case PartDigest:
-					part := s[i+1:]
+					if !yieldValid(PartDigest, s[i+1:j]) {
-					if isValidDigest(part) {
+						return
-						if !yield(PartDigest, part) {
+					}
-							return
+					if i == 0 {
-						}
+						// This is the form
-						if i == 0 {
+						// "@<digest>" which is valid.
-							// The name is in
+						//
-							// the form of
+						// We're done.
-							// "@digest". This
-							// is valid ans so
-							// we want to skip
-							// the final
-							// validation for
-							// any other state.
-							return
-						}
-					} else {
-						yield(PartInvalid, "")
 						return
 					}
 					state, j, partLen = PartBuild, i, 0

x/model/name_test.go

@@ -23,7 +23,7 @@ func fieldsFromName(p Name) fields {
 		model:     p.parts[PartModel],
 		tag:       p.parts[PartTag],
 		build:     p.parts[PartBuild],
-		digest:    p.parts[PartDigest],
+		digest:    p.digest.String(),
 	}
 }
 
@@ -101,8 +101,8 @@ var testNames = map[string]fields{
 
 func TestNameParts(t *testing.T) {
 	var p Name
-	if len(p.Parts()) != int(NumParts) {
+	if w, g := int(PartBuild+1), len(p.Parts()); w != g {
-		t.Errorf("Parts() = %d; want %d", len(p.Parts()), NumParts)
+		t.Errorf("Parts() = %d; want %d", g, w)
 	}
 }
 
@@ -137,7 +137,7 @@ func TestParseName(t *testing.T) {
 
 				// test round-trip
 				if !ParseName(name.String()).EqualFold(name) {
-					t.Errorf("String() = %s; want %s", name.String(), baseName)
+					t.Errorf("ParseName(%q).String() = %s; want %s", s, name.String(), baseName)
 				}
 
 				if name.Valid() && name.DisplayModel() == "" {
@@ -146,9 +146,9 @@ func TestParseName(t *testing.T) {
 					t.Errorf("Valid() = false; Model() = %q; want empty name", got.model)
 				}
 
-				if name.Resolved() && name.Digest() == "" {
+				if name.Resolved() && !name.Digest().Valid() {
 					t.Errorf("Resolved() = true; Digest() = %q; want non-empty digest", got.digest)
-				} else if !name.Resolved() && name.Digest() != "" {
+				} else if !name.Resolved() && name.Digest().Valid() {
 					t.Errorf("Resolved() = false; Digest() = %q; want empty digest", got.digest)
 				}
 			})
@@ -233,7 +233,7 @@ func TestNameDisplay(t *testing.T) {
 			wantLong:     "library/mistral:latest",
 			wantComplete: "example.com/library/mistral:latest",
 			wantModel:    "mistral",
-			wantGoString: "example.com/library/mistral:latest+Q4_0@?",
+			wantGoString: "example.com/library/mistral:latest+Q4_0@?-?",
 		},
 		{
 			name:         "Short Name",
@@ -242,7 +242,7 @@ func TestNameDisplay(t *testing.T) {
 			wantLong:     "mistral:latest",
 			wantComplete: "mistral:latest",
 			wantModel:    "mistral",
-			wantGoString: "?/?/mistral:latest+?@?",
+			wantGoString: "?/?/mistral:latest+?@?-?",
 		},
 		{
 			name:         "Long Name",
@@ -251,7 +251,7 @@ func TestNameDisplay(t *testing.T) {
 			wantLong:     "library/mistral:latest",
 			wantComplete: "library/mistral:latest",
 			wantModel:    "mistral",
-			wantGoString: "?/library/mistral:latest+?@?",
+			wantGoString: "?/library/mistral:latest+?@?-?",
 		},
 		{
 			name:         "Case Preserved",
@@ -260,7 +260,7 @@ func TestNameDisplay(t *testing.T) {
 			wantLong:     "Library/Mistral:Latest",
 			wantComplete: "Library/Mistral:Latest",
 			wantModel:    "Mistral",
-			wantGoString: "?/Library/Mistral:Latest+?@?",
+			wantGoString: "?/Library/Mistral:Latest+?@?-?",
 		},
 		{
 			name:         "With digest",