Repository: amzn/ion-go
Pull request #146 (2020-08-20 19:58:27)

Fix failing Annotations tests
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



zslayton: Could you merge + resolve conflicts? Then I'll do a final review.

Patch:
diff --git a/ion/binaryreader.go b/ion/binaryreader.go
index a0a1591..0847aa4 100644
--- a/ion/binaryreader.go
+++ b/ion/binaryreader.go
@@ -26,7 +26,6 @@ type binaryReader struct {
 
 	bits bitstream
 	cat  Catalog
-	lst  SymbolTable
 }
 
 func newBinaryReaderBuf(in *bufio.Reader, cat Catalog) Reader {
@@ -37,11 +36,6 @@ func newBinaryReaderBuf(in *bufio.Reader, cat Catalog) Reader {
 	return r
 }
 
-// SymbolTable returns the current symbol table.
-func (r *binaryReader) SymbolTable() SymbolTable {
-	return r.lst
-}
-
 // Next moves the reader to the next value.
 func (r *binaryReader) Next() bool {
 	if r.eof || r.err != nil {
@@ -152,12 +146,11 @@ func (r *binaryReader) next() (bool, error) {
 			if err != nil {
 				return false, err
 			}
-
-			st, err := r.resolveSymbolID(id)
+			st, err := NewSymbolTokenBySID(r.SymbolTable(), int64(id))
 			if err != nil {
 				return false, err
 			}
-			r.value = st
+			r.value = &st
 		}
 		r.valueType = SymbolType
 		return true, nil
@@ -235,8 +228,8 @@ func (r *binaryReader) next() (bool, error) {
 	panic(fmt.Sprintf("invalid bitcode %v", code))
 }
 
-func isIonSymbolTable(as []string) bool {
-	return len(as) > 0 && as[0] == "$ion_symbol_table"
+func isIonSymbolTable(as []SymbolToken) bool {
+	return len(as) > 0 && as[0].Text != nil && *as[0].Text == "$ion_symbol_table"
 }
 
 // ReadBVM reads a BVM, validates it, and resets the local symbol table.
@@ -269,56 +262,25 @@ func (r *binaryReader) readFieldName() error {
 		return err
 	}
 
-	st, err := r.resolveSymbolID(id)
+	st, err := NewSymbolTokenBySID(r.SymbolTable(), int64(id))
 	if err != nil {
 		return err
 	}
-	r.fieldNameSymbol = st
-	return nil
-
-}
-
-// ResolveSymbolID resolves a symbol ID to a symbol token.
-func (r *binaryReader) resolveSymbolID(id uint64) (*SymbolToken, error) {
-	if id > r.SymbolTable().MaxID() {
-		return nil, &UsageError{
-			"Reader.Next",
-			fmt.Sprintf("sid: %v, is greater than max id: %v", id, r.SymbolTable().MaxID()),
-		}
-	}
 
-	text, ok := r.SymbolTable().FindByID(id)
-
-	if !ok {
-		return &SymbolToken{LocalSID: int64(id)}, nil
-	}
-	return &SymbolToken{Text: &text, LocalSID: int64(id)}, nil
+	r.fieldNameSymbol = &st
+	return nil
 }
 
 // ReadAnnotations reads and resolves a set of annotations.
 func (r *binaryReader) readAnnotations() error {
-	ids, err := r.bits.ReadAnnotationIDs()
+	as, err := r.bits.ReadAnnotations(r.SymbolTable())
 	if err != nil {
 		return err
 	}
 
-	as := make([]string, len(ids))
-	for i, id := range ids {
-		as[i] = r.resolve(id)
-	}
-
 	r.annotations = as
-	return nil
-}
 
-// Resolve resolves a symbol ID to a symbol value (possibly ${id} if we're
-// missing the appropriate symbol table).
-func (r *binaryReader) resolve(id uint64) string {
-	s, ok := r.lst.FindByID(id)
-	if !ok {
-		return fmt.Sprintf("$%v", id)
-	}
-	return s
+	return nil
 }
 
 // StepIn steps in to a container-type value
diff --git a/ion/binarywriter.go b/ion/binarywriter.go
index d49547e..94f9930 100644
--- a/ion/binarywriter.go
+++ b/ion/binarywriter.go
@@ -495,10 +495,18 @@ func (w *binaryWriter) beginValue(api string) error {
 		ids := make([]uint64, len(as))
 		idlen := uint64(0)
 
+		var id uint64
+		var err error
 		for i, a := range as {
-			id, err := w.resolve(api, a)
-			if err != nil {
-				return err
+			if a.Text != nil {
+				id, err = w.resolve(api, *a.Text)
+				if err != nil {
+					return err
+				}
+			} else if a.LocalSID != SymbolIDUnknown {
+				id = uint64(a.LocalSID)
+			} else {
+				return &UsageError{api, "invalid annotation symbol token"}
 			}
 
 			ids[i] = id
diff --git a/ion/bitstream.go b/ion/bitstream.go
index 3720363..337ddb5 100644
--- a/ion/bitstream.go
+++ b/ion/bitstream.go
@@ -390,8 +390,8 @@ func (b *bitstream) ReadFieldID() (uint64, error) {
 	return id, nil
 }
 
-// ReadAnnotationIDs reads a set of annotation IDs.
-func (b *bitstream) ReadAnnotationIDs() ([]uint64, error) {
+// ReadAnnotations reads a set of annotation IDs and returns a set of SymbolTokens.
+func (b *bitstream) ReadAnnotations(symbolTable SymbolTable) ([]SymbolToken, error) {
 	if b.code != bitcodeAnnotation {
 		panic("not an annotation")
 	}
@@ -415,14 +415,20 @@ func (b *bitstream) ReadAnnotationIDs() ([]uint64, error) {
 		return nil, &SyntaxError{"malformed annotation", b.pos - lengthOfAnnotFieldLength}
 	}
 
-	var as []uint64
+	var as []SymbolToken
 	for annotFieldLength > 0 {
 		id, idlen, err := b.readVarUintLen(annotFieldLength)
 		if err != nil {
 			return nil, err
 		}
 
-		as = append(as, id)
+		token, err := NewSymbolTokenBySID(symbolTable, int64(id))
+		if err != nil {
+			return nil, err
+		}
+
+		as = append(as, token)
+
 		annotFieldLength -= idlen
 	}
 
diff --git a/ion/reader.go b/ion/reader.go
index 7fd837d..90d35ce 100644
--- a/ion/reader.go
+++ b/ion/reader.go
@@ -74,11 +74,6 @@ import (
 //
 type Reader interface {
 
-	// SymbolTable returns the current symbol table, or nil if there isn't one.
-	// Text Readers do not, generally speaking, have an associated symbol table.
-	// Binary Readers do.
-	SymbolTable() SymbolTable
-
 	// Next advances the Reader to the next position in the current value stream.
 	// It returns true if this is the position of an Ion value, and false if it
 	// is not. On error, it returns false and sets Err.
@@ -171,6 +166,11 @@ type Reader interface {
 	// SymbolValue returns the SymbolToken associated with the current value. It returns an
 	// error if the current value is not an Ion symbol.
 	SymbolValue() (*SymbolToken, error)
+
+	// SymbolTable returns the current symbol table, or nil if there isn't one.
+	// Text Readers do not, generally speaking, have an associated symbol table.
+	// Binary Readers do.
+	SymbolTable() SymbolTable
 }
 
 // NewReader creates a new Ion reader of the appropriate type by peeking
@@ -207,8 +207,9 @@ type reader struct {
 	eof bool
 	err error
 
+	lst             SymbolTable
 	fieldNameSymbol *SymbolToken
-	annotations     []string
+	annotations     []SymbolToken
 	valueType       Type
 	value           interface{}
 }
@@ -238,7 +239,21 @@ func (r *reader) FieldName() *string {
 
 // Annotations returns the current value's annotations.
 func (r *reader) Annotations() []string {
-	return r.annotations
+	var annotations []string
+	for _, an := range r.annotations {
+		if an.Text != nil {
+			sid, systemSymbolName := getSystemSymbolMapping(r.SymbolTable(), *an.Text)
+			if sid != SymbolIDUnknown {
+				annotations = append(annotations, systemSymbolName)
+			} else {
+				annotations = append(annotations, *an.Text)
+			}
+		} else {
+			annotations = append(annotations, "")
+		}
+	}
+
+	return annotations
 }
 
 // BoolValue returns the current value as a bool.
@@ -453,3 +468,8 @@ func (r *reader) FieldNameSymbol() (*SymbolToken, error) {
 
 	return r.fieldNameSymbol, nil
 }
+
+// SymbolTable returns the current symbol table.
+func (r *reader) SymbolTable() SymbolTable {
+	return r.lst
+}
diff --git a/ion/symboltoken.go b/ion/symboltoken.go
index 5e01176..a676878 100644
--- a/ion/symboltoken.go
+++ b/ion/symboltoken.go
@@ -15,7 +15,10 @@
 
 package ion
 
-import "fmt"
+import (
+	"fmt"
+	"strconv"
+)
 
 const (
 	// SymbolIDUnknown is placeholder for when a symbol token has no symbol ID.
@@ -89,22 +92,75 @@ func (st *SymbolToken) Equal(o *SymbolToken) bool {
 	return false
 }
 
+// Parses text of the form '$n' for some integer n.
+func symbolIdentifier(symbolText string) (int64, bool) {
+	if len(symbolText) > 1 && symbolText[0] == '$' {
+		if sid, err := strconv.Atoi(symbolText[1:]); err == nil {
+			return int64(sid), true
+		}
+	}
+
+	return SymbolIDUnknown, false
+}
+
+func getSystemSymbolMapping(symbolTable SymbolTable, symbolName string) (int64, string) {
+	// If we have a symbol name of the form '$n' for some integer n,
+	// then we want to use the corresponding system symbol name.
+	if sid, ok := symbolIdentifier(symbolName); ok {
+		if systemSymbolName, ok := symbolTable.FindByID(uint64(sid)); ok {
+			return sid, systemSymbolName
+		}
+	}
+
+	return SymbolIDUnknown, ""
+}
+
+// NewSymbolTokenBySID will check and return a symbol token if the given id exists in a symbol table,
+// otherwise return a new symbol token.
+func NewSymbolTokenBySID(symbolTable SymbolTable, sid int64) (SymbolToken, error) {
+	if sid < 0 || uint64(sid) > symbolTable.MaxID() {
+		return SymbolToken{}, fmt.Errorf("ion: Symbol token not found for SID '%v' in symbol table %v", sid, symbolTable)
+	}
+
+	text, ok := symbolTable.FindByID(uint64(sid))
+	if !ok {
+		return SymbolToken{LocalSID: sid}, nil
+	}
+
+	return SymbolToken{Text: &text, LocalSID: sid}, nil
+}
+
 // NewSymbolToken will check and return a symbol token if it exists in a symbol table,
 // otherwise return a new symbol token.
-func NewSymbolToken(symbolTable SymbolTable, text string) SymbolToken {
-	token := symbolTable.Find(text)
-	if token == nil {
-		token = &SymbolToken{Text: &text, LocalSID: SymbolIDUnknown}
+func NewSymbolToken(symbolTable SymbolTable, text string) (SymbolToken, error) {
+	if symbolTable == nil {
+		return SymbolToken{}, fmt.Errorf("ion: invalid symbol table")
+	}
+
+	if sid, ok := symbolIdentifier(text); ok {
+		return NewSymbolTokenBySID(symbolTable, sid)
 	}
-	return *token
+
+	sid, ok := symbolTable.FindByName(text)
+	if !ok {
+		return SymbolToken{Text: &text, LocalSID: SymbolIDUnknown}, nil
+	}
+
+	return SymbolToken{Text: &text, LocalSID: int64(sid)}, nil
 }
 
 // NewSymbolTokens will check and return a list of symbol tokens if they exists in a symbol table,
 // otherwise return a list of new symbol tokens.
-func NewSymbolTokens(symbolTable SymbolTable, textVals []string) []SymbolToken {
+func NewSymbolTokens(symbolTable SymbolTable, textVals []string) ([]SymbolToken, error) {
 	var tokens []SymbolToken
 	for _, text := range textVals {
-		tokens = append(tokens, NewSymbolToken(symbolTable, text))
+		token, err := NewSymbolToken(symbolTable, text)
+		if err != nil {
+			return nil, err
+		}
+
+		tokens = append(tokens, token)
 	}
-	return tokens
+
+	return tokens, nil
 }
diff --git a/ion/textreader.go b/ion/textreader.go
index 7bed291..7a5bfc6 100644
--- a/ion/textreader.go
+++ b/ion/textreader.go
@@ -57,24 +57,20 @@ type textReader struct {
 
 	tok   tokenizer
 	state trs
-	lst   SymbolTable
 	cat   Catalog
 }
 
 func newTextReaderBuf(in *bufio.Reader, cat Catalog) Reader {
-	return &textReader{
+	tr := textReader{
 		cat: cat,
 		tok: tokenizer{
 			in: in,
 		},
 		state: trsBeforeTypeAnnotations,
-		lst:   V1SystemSymbolTable,
 	}
-}
+	tr.lst = V1SystemSymbolTable
 
-// SymbolTable returns the current symbol table.
-func (t *textReader) SymbolTable() SymbolTable {
-	return t.lst
+	return &tr
 }
 
 // Next moves the reader to the next value.
@@ -189,11 +185,11 @@ func (t *textReader) nextBeforeFieldName() (bool, error) {
 		if tok == tokenSymbolQuoted {
 			t.fieldNameSymbol = &SymbolToken{Text: &val, LocalSID: SymbolIDUnknown}
 		} else {
-			st, err := t.readSymbol(val)
+			st, err := NewSymbolToken(t.SymbolTable(), val)
 			if err != nil {
 				return false, err
 			}
-			t.fieldNameSymbol = st
+			t.fieldNameSymbol = &st
 		}
 
 		// Skip over the following colon.
@@ -254,7 +250,12 @@ func (t *textReader) nextBeforeTypeAnnotations() (bool, error) {
 					"annotations that include a '" + val + "' must be enclosed in quotes", t.tok.Pos() - 1}
 			}
 
-			t.annotations = append(t.annotations, val)
+			token, err := NewSymbolToken(t.SymbolTable(), val)
+			if err != nil {
+				return false, err
+			}
+
+			t.annotations = append(t.annotations, token)
 			return false, nil
 		}
 
@@ -304,7 +305,7 @@ func (t *textReader) nextBeforeTypeAnnotations() (bool, error) {
 		t.value = StructType
 
 		ctx := t.ctx.peek()
-		if ctx == ctxAtTopLevel && isIonSymbolTable(t.Annotations()) {
+		if ctx == ctxAtTopLevel && isIonSymbolTable(t.annotations) {
 			if t.IsNull() {
 				t.clear()
 				t.lst = V1SystemSymbolTable
@@ -449,11 +450,11 @@ func (t *textReader) onSymbol(val string, tok token, ws bool) error {
 			valueType = FloatType
 			value = math.NaN()
 		default:
-			val, err := t.readSymbol(val)
+			val, err := NewSymbolToken(t.SymbolTable(), val)
 			if err != nil {
 				return err
 			}
-			value = val
+			value = &val
 		}
 	}
 
@@ -464,35 +465,6 @@ func (t *textReader) onSymbol(val string, tok token, ws bool) error {
 	return nil
 }
 
-// readSymbol reads a text as either a symbol identifier e.g., "$42" or a symbol text e.g., "foo" and resolves to a
-// symbol token.
-func (t *textReader) readSymbol(val string) (*SymbolToken, error) {
-	if isSymbolRef(val) {
-		id, err := strconv.Atoi(val[1:])
-		if err != nil {
-			return nil, err
-		}
-
-		if id < 0 || uint64(id) > t.SymbolTable().MaxID() {
-			return nil, &UsageError{
-				"Reader.Next",
-				fmt.Sprintf("sid: %v, is greater than max id: %v", id, t.SymbolTable().MaxID()),
-			}
-		}
-
-		text, ok := t.SymbolTable().FindByID(uint64(id))
-		if !ok {
-			return &SymbolToken{LocalSID: int64(id)}, nil
-		}
-		return &SymbolToken{Text: &text, LocalSID: int64(id)}, nil
-	}
-	id, ok := t.lst.FindByName(val)
-	if ok {
-		return &SymbolToken{Text: &val, LocalSID: int64(id)}, nil
-	}
-	return &SymbolToken{Text: &val, LocalSID: SymbolIDUnknown}, nil
-}
-
 // OnNull handles finding a null token.
 func (t *textReader) onNull(ws bool) (Type, error) {
 	if !ws {
diff --git a/ion/textutils.go b/ion/textutils.go
index dade41a..ca94522 100644
--- a/ion/textutils.go
+++ b/ion/textutils.go
@@ -42,25 +42,6 @@ func symbolNeedsQuoting(sym string) bool {
 	return false
 }
 
-// Is this the text form of a symbol reference ($<integer>)?
-func isSymbolRef(sym string) bool {
-	if len(sym) == 0 || sym[0] != '$' {
-		return false
-	}
-
-	if len(sym) == 1 {
-		return false
-	}
-
-	for i := 1; i < len(sym); i++ {
-		if !isDigit(int(sym[i])) {
-			return false
-		}
-	}
-
-	return true
-}
-
 // Is this a valid first character for an identifier?
 func isIdentifierStart(c int) bool {
 	if c >= 'a' && c <= 'z' {
diff --git a/ion/textwriter.go b/ion/textwriter.go
index 0179eba..401fba5 100644
--- a/ion/textwriter.go
+++ b/ion/textwriter.go
@@ -353,7 +353,7 @@ func (w *textWriter) beginValue(api string) error {
 
 	w.annotations = append(w.annotations, as...)
 	if len(w.annotations) > 0 {
-		if err := w.writeAnnotations(); err != nil {
+		if err := w.writeAnnotations(api); err != nil {
 			return err
 		}
 	}
@@ -411,12 +411,21 @@ func (w *textWriter) writeFieldName(api string) error {
 }
 
 // writeAnnotations writes out the annotations for a value.
-func (w *textWriter) writeAnnotations() error {
+func (w *textWriter) writeAnnotations(api string) error {
 	as := w.annotations
 	w.annotations = nil
 
+	var text string
 	for _, a := range as {
-		if err := writeSymbol(a, w.out); err != nil {
+		if a.Text != nil {
+			text = *a.Text
+		} else if a.LocalSID != SymbolIDUnknown {
+			text = fmt.Sprintf("$%v", a.LocalSID)
+		} else {
+			return &UsageError{api, "invalid annotation symbol token"}
+		}
+
+		if err := writeSymbol(text, w.out); err != nil {
 			return err
 		}
 		if err := writeRawString("::", w.out); err != nil {
diff --git a/ion/writer.go b/ion/writer.go
index d01eb59..cd03610 100644
--- a/ion/writer.go
+++ b/ion/writer.go
@@ -149,7 +149,7 @@ type writer struct {
 	err error
 
 	fieldName   *string
-	annotations []string
+	annotations []SymbolToken
 }
 
 // FieldName sets the field name for the next value written.
@@ -187,18 +187,36 @@ func (w *writer) FieldNameSymbol(val SymbolToken) error {
 
 // Annotation adds an annotation to the next value written.
 func (w *writer) Annotation(val string) error {
-	if w.err == nil {
-		w.annotations = append(w.annotations, val)
+	if w.err != nil {
+		return w.err
 	}
-	return w.err
+
+	var token SymbolToken
+	token, w.err = NewSymbolToken(V1SystemSymbolTable, val)
+	if w.err != nil {
+		return w.err
+	}
+
+	w.annotations = append(w.annotations, token)
+
+	return nil
 }
 
 // Annotations adds one or more annotations to the next value written.
 func (w *writer) Annotations(values ...string) error {
-	if w.err == nil {
-		w.annotations = append(w.annotations, values...)
+	if w.err != nil {
+		return w.err
 	}
-	return w.err
+
+	var tokens []SymbolToken
+	tokens, w.err = NewSymbolTokens(V1SystemSymbolTable, values)
+	if w.err != nil {
+		return w.err
+	}
+
+	w.annotations = append(w.annotations, tokens...)
+
+	return nil
 }
 
 // IsInStruct returns true if we're currently writing a struct.


Test patch:
diff --git a/ion/bitstream_test.go b/ion/bitstream_test.go
index 7d49039..86e745c 100644
--- a/ion/bitstream_test.go
+++ b/ion/bitstream_test.go
@@ -79,12 +79,12 @@ func TestBitstream(t *testing.T) {
 	}
 
 	next(bitcodeAnnotation, false, 31)
-	ids, err := b.ReadAnnotationIDs()
+	as, err := b.ReadAnnotations(V1SystemSymbolTable)
 	if err != nil {
 		t.Fatal(err)
 	}
-	if len(ids) != 1 || ids[0] != 3 { // $ion_symbol_table
-		t.Errorf("expected [3], got %v", ids)
+	if len(as) != 1 || as[0].LocalSID != 3 { // $ion_symbol_table
+		t.Errorf("expected [3], got %v", as)
 	}
 
 	next(bitcodeStruct, false, 27)
diff --git a/ion/integration_test.go b/ion/integration_test.go
index 69948b6..3f10d73 100644
--- a/ion/integration_test.go
+++ b/ion/integration_test.go
@@ -87,14 +87,12 @@ var binaryRoundTripSkipList = []string{
 }
 
 var textRoundTripSkipList = []string{
-	"annotations.ion",
 	"notVersionMarkers.ion",
 	"subfieldVarUInt.ion",
 	"subfieldVarUInt15bit.ion",
 	"subfieldVarUInt16bit.ion",
 	"subfieldVarUInt32bit.ion",
 	"systemSymbols.ion",
-	"systemSymbolsAsAnnotations.ion",
 	"T7-large.10n",
 	"utf16.ion",
 	"utf32.ion",
@@ -102,8 +100,6 @@ var textRoundTripSkipList = []string{
 }
 
 var malformedIonsSkipList = []string{
-	"annotationSymbolIDUnmapped.10n",
-	"annotationSymbolIDUnmapped.ion",
 	"invalidVersionMarker_ion_0_0.ion",
 	"invalidVersionMarker_ion_1234_0.ion",
 	"invalidVersionMarker_ion_1_1.ion",
@@ -123,12 +119,9 @@ var malformedIonsSkipList = []string{
 }
 
 var equivsSkipList = []string{
-	"annotatedIvms.ion",
-	"localSymbolTablesValuesWithAnnotations.ion",
 	"nonIVMNoOps.ion",
 	"stringUtf8.ion", // fails on utf-16 surrogate https://github.com/amzn/ion-go/issues/75
 	"systemSymbols.ion",
-	"systemSymbolsAsAnnotations.ion",
 }
 
 var nonEquivsSkipList = []string{
@@ -801,6 +794,7 @@ func readCurrentValue(t *testing.T, reader Reader) ionItem {
 			t.Fatal(err)
 		}
 	}
+
 	return ionItem
 }
 
diff --git a/ion/symboltoken_test.go b/ion/symboltoken_test.go
index 2290e56..bccbe52 100644
--- a/ion/symboltoken_test.go
+++ b/ion/symboltoken_test.go
@@ -153,7 +153,10 @@ func TestImportSourceEqualsOperator(t *testing.T) {
 func TestNewSymbolTokenThatAlreadyExistInSymbolTable(t *testing.T) {
 	expectedSymbolToken := SymbolToken{Text: newString("$ion"), LocalSID: SymbolIDUnknown}
 
-	actualSymbolToken := NewSymbolToken(V1SystemSymbolTable, "$ion")
+	actualSymbolToken, err := NewSymbolToken(V1SystemSymbolTable, "$ion")
+	if err != nil {
+		t.Errorf("expected NewSymbolToken() to execute without errors; %s", err.Error())
+	}
 
 	if !actualSymbolToken.Equal(&expectedSymbolToken) {
 		t.Errorf("expected %v, got %v", expectedSymbolToken, actualSymbolToken)
@@ -163,7 +166,10 @@ func TestNewSymbolTokenThatAlreadyExistInSymbolTable(t *testing.T) {
 func TestNewSymbolTokenThatDoesNotExistInSymbolTable(t *testing.T) {
 	expectedSymbolToken := SymbolToken{Text: newString("newToken"), LocalSID: SymbolIDUnknown}
 
-	actualSymbolToken := NewSymbolToken(V1SystemSymbolTable, "newToken")
+	actualSymbolToken, err := NewSymbolToken(V1SystemSymbolTable, "newToken")
+	if err != nil {
+		t.Errorf("expected NewSymbolToken() to execute without errors; %s", err.Error())
+	}
 
 	if !actualSymbolToken.Equal(&expectedSymbolToken) {
 		t.Errorf("expected %v, got %v", expectedSymbolToken, actualSymbolToken)
@@ -175,12 +181,14 @@ func TestNewSymbolTokensThatAlreadyExistInSymbolTable(t *testing.T) {
 		{Text: newString("$ion"), LocalSID: SymbolIDUnknown},
 		{Text: newString("$ion_1_0"), LocalSID: SymbolIDUnknown}}
 
-	actualSymbolTokens := NewSymbolTokens(V1SystemSymbolTable, []string{"$ion", "$ion_1_0"})
+	actualSymbolTokens, err := NewSymbolTokens(V1SystemSymbolTable, []string{"$ion", "$ion_1_0"})
+	if err != nil {
+		t.Errorf("expected NewSymbolTokens() to execute without errors; %s", err.Error())
+	}
 
 	for index, actualSymbolToken := range actualSymbolTokens {
 		if !actualSymbolToken.Equal(&expectedSymbolTokens[index]) {
 			t.Errorf("expected %v, got %v", &expectedSymbolTokens[index], actualSymbolToken)
-
 		}
 	}
 }
@@ -190,12 +198,39 @@ func TestNewSymbolTokensThatDoNotExistInSymbolTable(t *testing.T) {
 		{Text: newString("newToken1"), LocalSID: SymbolIDUnknown},
 		{Text: newString("newToken2"), LocalSID: SymbolIDUnknown}}
 
-	actualSymbolTokens := NewSymbolTokens(V1SystemSymbolTable, []string{"newToken1", "newToken2"})
+	actualSymbolTokens, err := NewSymbolTokens(V1SystemSymbolTable, []string{"newToken1", "newToken2"})
+	if err != nil {
+		t.Errorf("expected NewSymbolTokens() to execute without errors; %s", err.Error())
+	}
 
 	for index, actualSymbolToken := range actualSymbolTokens {
 		if !actualSymbolToken.Equal(&expectedSymbolTokens[index]) {
 			t.Errorf("expected %v, got %v", &expectedSymbolTokens[index], actualSymbolToken)
-
 		}
 	}
 }
+
+func TestSymbolIdentifier(t *testing.T) {
+	test := func(sym string, expectedSID int64, expectedOK bool) {
+		t.Run(sym, func(t *testing.T) {
+			sid, ok := symbolIdentifier(sym)
+
+			if ok != expectedOK {
+				t.Errorf("expected %v, got %v", expectedOK, ok)
+			}
+
+			if expectedOK && sid != expectedSID {
+				t.Errorf("expected %v, got %v", expectedSID, sid)
+			}
+		})
+	}
+
+	test("", SymbolIDUnknown, false)
+	test("1", SymbolIDUnknown, false)
+	test("a", SymbolIDUnknown, false)
+	test("$", SymbolIDUnknown, false)
+	test("$1", 1, true)
+	test("$1234567890", 1234567890, true)
+	test("$a", SymbolIDUnknown, false)
+	test("$1234a567890", SymbolIDUnknown, false)
+}
diff --git a/ion/textutils_test.go b/ion/textutils_test.go
index 91585c1..4480772 100644
--- a/ion/textutils_test.go
+++ b/ion/textutils_test.go
@@ -135,26 +135,6 @@ func TestSymbolNeedsQuoting(t *testing.T) {
 	test("abc\"def", true)
 }
 
-func TestIsSymbolRef(t *testing.T) {
-	test := func(sym string, expected bool) {
-		t.Run(sym, func(t *testing.T) {
-			actual := isSymbolRef(sym)
-			if actual != expected {
-				t.Errorf("expected %v, got %v", expected, actual)
-			}
-		})
-	}
-
-	test("", false)
-	test("1", false)
-	test("a", false)
-	test("$", false)
-	test("$1", true)
-	test("$1234567890", true)
-	test("$a", false)
-	test("$1234a567890", false)
-}
-
 func TestWriteEscapedSymbol(t *testing.T) {
 	test := func(sym, expected string) {
 		t.Run(expected, func(t *testing.T) {