Refactor tvsaver to handle element ID maps

Signed-off-by: Steve Winslow <steve@swinslow.net>

14 files changed, 117 insertions, 139 deletions

diff --git a/tvsaver/saver2v1/save_annotation.go b/tvsaver/saver2v1/save_annotation.go
index f46ed4a..8c0ae89 100644
--- a/tvsaver/saver2v1/save_annotation.go
+++ b/tvsaver/saver2v1/save_annotation.go
@@ -19,8 +19,9 @@ func renderAnnotation2_1(ann *spdx.Annotation2_1, w io.Writer) error {
 	if ann.AnnotationType != "" {
 		fmt.Fprintf(w, "AnnotationType: %s\n", ann.AnnotationType)
 	}
-	if ann.AnnotationSPDXIdentifier != "" {
-		fmt.Fprintf(w, "SPDXREF: %s\n", ann.AnnotationSPDXIdentifier)
+	annIDStr := spdx.RenderDocElementID(ann.AnnotationSPDXIdentifier)
+	if annIDStr != "SPDXRef-" {
+		fmt.Fprintf(w, "SPDXREF: %s\n", annIDStr)
 	}
 	if ann.AnnotationComment != "" {
 		fmt.Fprintf(w, "AnnotationComment: %s\n", textify(ann.AnnotationComment))
diff --git a/tvsaver/saver2v1/save_annotation_test.go b/tvsaver/saver2v1/save_annotation_test.go
index 077e9f8..9cb0277 100644
--- a/tvsaver/saver2v1/save_annotation_test.go
+++ b/tvsaver/saver2v1/save_annotation_test.go
@@ -16,7 +16,7 @@ func TestSaver2_1AnnotationSavesTextForPerson(t *testing.T) {
 		AnnotatorType:            "Person",
 		AnnotationDate:           "2018-10-10T17:52:00Z",
 		AnnotationType:           "REVIEW",
-		AnnotationSPDXIdentifier: "SPDXRef-DOCUMENT",
+		AnnotationSPDXIdentifier: spdx.MakeDocElementID("", "DOCUMENT"),
 		AnnotationComment:        "This is an annotation about the SPDX document",
 	}
 
@@ -49,7 +49,7 @@ func TestSaver2_1AnnotationSavesTextForOrganization(t *testing.T) {
 		AnnotatorType:            "Organization",
 		AnnotationDate:           "2018-10-10T17:52:00Z",
 		AnnotationType:           "REVIEW",
-		AnnotationSPDXIdentifier: "SPDXRef-DOCUMENT",
+		AnnotationSPDXIdentifier: spdx.MakeDocElementID("", "DOCUMENT"),
 		AnnotationComment:        "This is an annotation about the SPDX document",
 	}
 
@@ -82,7 +82,7 @@ func TestSaver2_1AnnotationSavesTextForTool(t *testing.T) {
 		AnnotatorType:            "Tool",
 		AnnotationDate:           "2018-10-10T17:52:00Z",
 		AnnotationType:           "REVIEW",
-		AnnotationSPDXIdentifier: "SPDXRef-DOCUMENT",
+		AnnotationSPDXIdentifier: spdx.MakeDocElementID("", "DOCUMENT"),
 		AnnotationComment:        "This is an annotation about the SPDX document",
 	}
 
diff --git a/tvsaver/saver2v1/save_creation_info.go b/tvsaver/saver2v1/save_creation_info.go
index 1592923..17e14c9 100644
--- a/tvsaver/saver2v1/save_creation_info.go
+++ b/tvsaver/saver2v1/save_creation_info.go
@@ -17,7 +17,7 @@ func renderCreationInfo2_1(ci *spdx.CreationInfo2_1, w io.Writer) error {
 		fmt.Fprintf(w, "DataLicense: %s\n", ci.DataLicense)
 	}
 	if ci.SPDXIdentifier != "" {
-		fmt.Fprintf(w, "SPDXID: %s\n", ci.SPDXIdentifier)
+		fmt.Fprintf(w, "SPDXID: %s\n", spdx.RenderElementID(ci.SPDXIdentifier))
 	}
 	if ci.DocumentName != "" {
 		fmt.Fprintf(w, "DocumentName: %s\n", ci.DocumentName)
diff --git a/tvsaver/saver2v1/save_creation_info_test.go b/tvsaver/saver2v1/save_creation_info_test.go
index 45db057..3b961ec 100644
--- a/tvsaver/saver2v1/save_creation_info_test.go
+++ b/tvsaver/saver2v1/save_creation_info_test.go
@@ -14,7 +14,7 @@ func TestSaver2_1CISavesText(t *testing.T) {
 	ci := &spdx.CreationInfo2_1{
 		SPDXVersion:       "SPDX-2.1",
 		DataLicense:       "CC0-1.0",
-		SPDXIdentifier:    "SPDXRef-DOCUMENT",
+		SPDXIdentifier:    spdx.ElementID("DOCUMENT"),
 		DocumentName:      "spdx-go-0.0.1.abcdef",
 		DocumentNamespace: "https://github.com/swinslow/spdx-docs/spdx-go/spdx-go-0.0.1.abcdef.whatever",
 		ExternalDocumentReferences: []string{
@@ -81,7 +81,7 @@ func TestSaver2_1CIOmitsOptionalFieldsIfEmpty(t *testing.T) {
 	ci1 := &spdx.CreationInfo2_1{
 		SPDXVersion:       "SPDX-2.1",
 		DataLicense:       "CC0-1.0",
-		SPDXIdentifier:    "SPDXRef-DOCUMENT",
+		SPDXIdentifier:    spdx.ElementID("DOCUMENT"),
 		DocumentName:      "spdx-go-0.0.1.abcdef",
 		DocumentNamespace: "https://github.com/swinslow/spdx-docs/spdx-go/spdx-go-0.0.1.abcdef.whatever",
 		CreatorPersons: []string{
@@ -118,7 +118,7 @@ Created: 2018-10-10T06:20:00Z
 	ci2 := &spdx.CreationInfo2_1{
 		SPDXVersion:       "SPDX-2.1",
 		DataLicense:       "CC0-1.0",
-		SPDXIdentifier:    "SPDXRef-DOCUMENT",
+		SPDXIdentifier:    spdx.ElementID("DOCUMENT"),
 		DocumentName:      "spdx-go-0.0.1.abcdef",
 		DocumentNamespace: "https://github.com/swinslow/spdx-docs/spdx-go/spdx-go-0.0.1.abcdef.whatever",
 		CreatorOrganizations: []string{
diff --git a/tvsaver/saver2v1/save_document.go b/tvsaver/saver2v1/save_document.go
index e573ad3..67dfddc 100644
--- a/tvsaver/saver2v1/save_document.go
+++ b/tvsaver/saver2v1/save_document.go
@@ -7,6 +7,7 @@ package saver2v1
 import (
 	"fmt"
 	"io"
+	"sort"
 
 	"github.com/spdx/tools-golang/spdx"
 )
@@ -24,12 +25,26 @@ func RenderDocument2_1(doc *spdx.Document2_1, w io.Writer) error {
 
 	if len(doc.UnpackagedFiles) > 0 {
 		fmt.Fprintf(w, "##### Unpackaged files\n\n")
-		for _, fi := range doc.UnpackagedFiles {
+		// get slice of identifiers so we can sort them
+		unpackagedFileKeys := []string{}
+		for k := range doc.UnpackagedFiles {
+			unpackagedFileKeys = append(unpackagedFileKeys, string(k))
+		}
+		sort.Strings(unpackagedFileKeys)
+		for _, fiID := range unpackagedFileKeys {
+			fi := doc.UnpackagedFiles[spdx.ElementID(fiID)]
 			renderFile2_1(fi, w)
 		}
 	}
 
-	for _, pkg := range doc.Packages {
+	// get slice of Package identifiers so we can sort them
+	packageKeys := []string{}
+	for k := range doc.Packages {
+		packageKeys = append(packageKeys, string(k))
+	}
+	sort.Strings(packageKeys)
+	for _, pkgID := range packageKeys {
+		pkg := doc.Packages[spdx.ElementID(pkgID)]
 		fmt.Fprintf(w, "##### Package: %s\n\n", pkg.PackageName)
 		renderPackage2_1(pkg, w)
 	}
diff --git a/tvsaver/saver2v1/save_document_test.go b/tvsaver/saver2v1/save_document_test.go
index f75093d..980c6df 100644
--- a/tvsaver/saver2v1/save_document_test.go
+++ b/tvsaver/saver2v1/save_document_test.go
@@ -16,7 +16,7 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 	ci := &spdx.CreationInfo2_1{
 		SPDXVersion:       "SPDX-2.1",
 		DataLicense:       "CC0-1.0",
-		SPDXIdentifier:    "SPDXRef-DOCUMENT",
+		SPDXIdentifier:    spdx.ElementID("DOCUMENT"),
 		DocumentName:      "spdx-go-0.0.1.abcdef",
 		DocumentNamespace: "https://github.com/swinslow/spdx-docs/spdx-go/spdx-go-0.0.1.abcdef.whatever",
 		CreatorPersons: []string{
@@ -28,7 +28,7 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 	// unpackaged files
 	f1 := &spdx.File2_1{
 		FileName:           "/tmp/whatever1.txt",
-		FileSPDXIdentifier: "SPDXRef-File1231",
+		FileSPDXIdentifier: spdx.ElementID("File1231"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983c",
 		LicenseConcluded:   "Apache-2.0",
 		LicenseInfoInFile:  []string{"Apache-2.0"},
@@ -37,7 +37,7 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 
 	f2 := &spdx.File2_1{
 		FileName:           "/tmp/whatever2.txt",
-		FileSPDXIdentifier: "SPDXRef-File1232",
+		FileSPDXIdentifier: spdx.ElementID("File1232"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983d",
 		LicenseConcluded:   "MIT",
 		LicenseInfoInFile:  []string{"MIT"},
@@ -45,14 +45,14 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 	}
 
 	unFiles := map[spdx.ElementID]*spdx.File2_1{
-		"File1231": f1,
-		"File1232": f2,
+		spdx.ElementID("File1231"): f1,
+		spdx.ElementID("File1232"): f2,
 	}
 
 	// Package 1: packaged files with snippets
 	sn1 := &spdx.Snippet2_1{
-		SnippetSPDXIdentifier:         "SPDXRef-Snippet19",
-		SnippetFromFileSPDXIdentifier: "SPDXRef-FileHasSnippets",
+		SnippetSPDXIdentifier:         "Snippet19",
+		SnippetFromFileSPDXIdentifier: spdx.MakeDocElementID("", "FileHasSnippets"),
 		SnippetByteRangeStart:         17,
 		SnippetByteRangeEnd:           209,
 		SnippetLicenseConcluded:       "GPL-2.0-or-later",
@@ -60,8 +60,8 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 	}
 
 	sn2 := &spdx.Snippet2_1{
-		SnippetSPDXIdentifier:         "SPDXRef-Snippet20",
-		SnippetFromFileSPDXIdentifier: "SPDXRef-FileHasSnippets",
+		SnippetSPDXIdentifier:         "Snippet20",
+		SnippetFromFileSPDXIdentifier: spdx.MakeDocElementID("", "FileHasSnippets"),
 		SnippetByteRangeStart:         268,
 		SnippetByteRangeEnd:           309,
 		SnippetLicenseConcluded:       "WTFPL",
@@ -70,7 +70,7 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 
 	f3 := &spdx.File2_1{
 		FileName:           "/tmp/file-with-snippets.txt",
-		FileSPDXIdentifier: "SPDXRef-FileHasSnippets",
+		FileSPDXIdentifier: spdx.ElementID("FileHasSnippets"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983e",
 		LicenseConcluded:   "GPL-2.0-or-later AND WTFPL",
 		LicenseInfoInFile: []string{
@@ -80,14 +80,14 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 		},
 		FileCopyrightText: "Copyright (c) Jane Doe",
 		Snippets: map[spdx.ElementID]*spdx.Snippet2_1{
-			"Snippet19": sn1,
-			"Snippet20": sn2,
+			spdx.ElementID("Snippet19"): sn1,
+			spdx.ElementID("Snippet20"): sn2,
 		},
 	}
 
 	f4 := &spdx.File2_1{
 		FileName:           "/tmp/another-file.txt",
-		FileSPDXIdentifier: "SPDXRef-FileAnother",
+		FileSPDXIdentifier: spdx.ElementID("FileAnother"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983f",
 		LicenseConcluded:   "BSD-3-Clause",
 		LicenseInfoInFile:  []string{"BSD-3-Clause"},
@@ -96,7 +96,7 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 
 	pkgWith := &spdx.Package2_1{
 		PackageName:               "p1",
-		PackageSPDXIdentifier:     "SPDXRef-p1",
+		PackageSPDXIdentifier:     spdx.ElementID("p1"),
 		PackageDownloadLocation:   "http://example.com/p1/p1-0.1.0-master.tar.gz",
 		FilesAnalyzed:             true,
 		IsFilesAnalyzedTagPresent: true,
@@ -111,8 +111,8 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 		PackageLicenseDeclared: "Apache-2.0 OR GPL-2.0-or-later",
 		PackageCopyrightText:   "Copyright (c) John Doe, Inc.",
 		Files: map[spdx.ElementID]*spdx.File2_1{
-			"FileHasSnippets": f3,
-			"FileAnother":     f4,
+			spdx.ElementID("FileHasSnippets"): f3,
+			spdx.ElementID("FileAnother"):     f4,
 		},
 	}
 
@@ -133,20 +133,20 @@ blah blah blah blah`,
 
 	// Relationships
 	rln1 := &spdx.Relationship2_1{
-		RefA:         "SPDXRef-DOCUMENT",
-		RefB:         "SPDXRef-p1",
+		RefA:         spdx.MakeDocElementID("", "DOCUMENT"),
+		RefB:         spdx.MakeDocElementID("", "p1"),
 		Relationship: "DESCRIBES",
 	}
 
 	rln2 := &spdx.Relationship2_1{
-		RefA:         "SPDXRef-DOCUMENT",
-		RefB:         "SPDXRef-File1231",
+		RefA:         spdx.MakeDocElementID("", "DOCUMENT"),
+		RefB:         spdx.MakeDocElementID("", "File1231"),
 		Relationship: "DESCRIBES",
 	}
 
 	rln3 := &spdx.Relationship2_1{
-		RefA:         "SPDXRef-DOCUMENT",
-		RefB:         "SPDXRef-File1232",
+		RefA:         spdx.MakeDocElementID("", "DOCUMENT"),
+		RefB:         spdx.MakeDocElementID("", "File1232"),
 		Relationship: "DESCRIBES",
 	}
 
@@ -156,7 +156,7 @@ blah blah blah blah`,
 		AnnotatorType:            "Person",
 		AnnotationDate:           "2018-10-10T17:52:00Z",
 		AnnotationType:           "REVIEW",
-		AnnotationSPDXIdentifier: "SPDXRef-DOCUMENT",
+		AnnotationSPDXIdentifier: spdx.MakeDocElementID("", "DOCUMENT"),
 		AnnotationComment:        "This is an annotation about the SPDX document",
 	}
 
@@ -165,7 +165,7 @@ blah blah blah blah`,
 		AnnotatorType:            "Organization",
 		AnnotationDate:           "2018-10-10T17:52:00Z",
 		AnnotationType:           "REVIEW",
-		AnnotationSPDXIdentifier: "SPDXRef-p1",
+		AnnotationSPDXIdentifier: spdx.MakeDocElementID("", "p1"),
 		AnnotationComment:        "This is an annotation about Package p1",
 	}
 
@@ -186,7 +186,7 @@ blah blah blah blah`,
 	doc := &spdx.Document2_1{
 		CreationInfo: ci,
 		Packages: map[spdx.ElementID]*spdx.Package2_1{
-			pkgWith,
+			spdx.ElementID("p1"): pkgWith,
 		},
 		UnpackagedFiles: unFiles,
 		OtherLicenses: []*spdx.OtherLicense2_1{
@@ -247,6 +247,13 @@ PackageLicenseInfoFromFiles: BSD-3-Clause
 PackageLicenseDeclared: Apache-2.0 OR GPL-2.0-or-later
 PackageCopyrightText: Copyright (c) John Doe, Inc.
 
+FileName: /tmp/another-file.txt
+SPDXID: SPDXRef-FileAnother
+FileChecksum: SHA1: 85ed0817af83a24ad8da68c2b5094de69833983f
+LicenseConcluded: BSD-3-Clause
+LicenseInfoInFile: BSD-3-Clause
+FileCopyrightText: Copyright (c) Jane Doe LLC
+
 FileName: /tmp/file-with-snippets.txt
 SPDXID: SPDXRef-FileHasSnippets
 FileChecksum: SHA1: 85ed0817af83a24ad8da68c2b5094de69833983e
@@ -268,13 +275,6 @@ SnippetByteRange: 268:309
 SnippetLicenseConcluded: WTFPL
 SnippetCopyrightText: NOASSERTION
 
-FileName: /tmp/another-file.txt
-SPDXID: SPDXRef-FileAnother
-FileChecksum: SHA1: 85ed0817af83a24ad8da68c2b5094de69833983f
-LicenseConcluded: BSD-3-Clause
-LicenseInfoInFile: BSD-3-Clause
-FileCopyrightText: Copyright (c) Jane Doe LLC
-
 ##### Other Licenses
 
 LicenseID: LicenseRef-1
diff --git a/tvsaver/saver2v1/save_file.go b/tvsaver/saver2v1/save_file.go
index a8b50f0..a9df3ed 100644
--- a/tvsaver/saver2v1/save_file.go
+++ b/tvsaver/saver2v1/save_file.go
@@ -5,6 +5,7 @@ package saver2v1
 import (
 	"fmt"
 	"io"
+	"sort"
 
 	"github.com/spdx/tools-golang/spdx"
 )
@@ -14,7 +15,7 @@ func renderFile2_1(f *spdx.File2_1, w io.Writer) error {
 		fmt.Fprintf(w, "FileName: %s\n", f.FileName)
 	}
 	if f.FileSPDXIdentifier != "" {
-		fmt.Fprintf(w, "SPDXID: %s\n", f.FileSPDXIdentifier)
+		fmt.Fprintf(w, "SPDXID: %s\n", spdx.RenderElementID(f.FileSPDXIdentifier))
 	}
 	for _, s := range f.FileType {
 		fmt.Fprintf(w, "FileType: %s\n", s)
@@ -65,7 +66,14 @@ func renderFile2_1(f *spdx.File2_1, w io.Writer) error {
 	fmt.Fprintf(w, "\n")
 
 	// also render any snippets for this file
-	for _, s := range f.Snippets {
+	// get slice of Snippet identifiers so we can sort them
+	snippetKeys := []string{}
+	for k := range f.Snippets {
+		snippetKeys = append(snippetKeys, string(k))
+	}
+	sort.Strings(snippetKeys)
+	for _, sID := range snippetKeys {
+		s := f.Snippets[spdx.ElementID(sID)]
 		renderSnippet2_1(s, w)
 	}
 
diff --git a/tvsaver/saver2v1/save_file_test.go b/tvsaver/saver2v1/save_file_test.go
index 7b9fb87..9e652b6 100644
--- a/tvsaver/saver2v1/save_file_test.go
+++ b/tvsaver/saver2v1/save_file_test.go
@@ -13,7 +13,7 @@ import (
 func TestSaver2_1FileSavesText(t *testing.T) {
 	f := &spdx.File2_1{
 		FileName:           "/tmp/whatever.txt",
-		FileSPDXIdentifier: "SPDXRef-File123",
+		FileSPDXIdentifier: spdx.ElementID("File123"),
 		FileType: []string{
 			"TEXT",
 			"DOCUMENTATION",
@@ -104,8 +104,8 @@ FileDependency: g.txt
 
 func TestSaver2_1FileSavesSnippetsAlso(t *testing.T) {
 	sn1 := &spdx.Snippet2_1{
-		SnippetSPDXIdentifier:         "SPDXRef-Snippet19",
-		SnippetFromFileSPDXIdentifier: "SPDXRef-File123",
+		SnippetSPDXIdentifier:         spdx.ElementID("Snippet19"),
+		SnippetFromFileSPDXIdentifier: spdx.MakeDocElementID("", "File123"),
 		SnippetByteRangeStart:         17,
 		SnippetByteRangeEnd:           209,
 		SnippetLicenseConcluded:       "GPL-2.0-or-later",
@@ -113,22 +113,22 @@ func TestSaver2_1FileSavesSnippetsAlso(t *testing.T) {
 	}
 
 	sn2 := &spdx.Snippet2_1{
-		SnippetSPDXIdentifier:         "SPDXRef-Snippet20",
-		SnippetFromFileSPDXIdentifier: "SPDXRef-File123",
+		SnippetSPDXIdentifier:         spdx.ElementID("Snippet20"),
+		SnippetFromFileSPDXIdentifier: spdx.MakeDocElementID("", "File123"),
 		SnippetByteRangeStart:         268,
 		SnippetByteRangeEnd:           309,
 		SnippetLicenseConcluded:       "WTFPL",
 		SnippetCopyrightText:          "NOASSERTION",
 	}
 
-	sns := []*spdx.Snippet2_1{
-		sn1,
-		sn2,
+	sns := map[spdx.ElementID]*spdx.Snippet2_1{
+		spdx.ElementID("Snippet19"): sn1,
+		spdx.ElementID("Snippet20"): sn2,
 	}
 
 	f := &spdx.File2_1{
 		FileName:           "/tmp/whatever.txt",
-		FileSPDXIdentifier: "SPDXRef-File123",
+		FileSPDXIdentifier: spdx.ElementID("File123"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983c",
 		LicenseConcluded:   "Apache-2.0",
 		LicenseInfoInFile: []string{
@@ -177,7 +177,7 @@ SnippetCopyrightText: NOASSERTION
 func TestSaver2_1FileOmitsOptionalFieldsIfEmpty(t *testing.T) {
 	f := &spdx.File2_1{
 		FileName:           "/tmp/whatever.txt",
-		FileSPDXIdentifier: "SPDXRef-File123",
+		FileSPDXIdentifier: spdx.ElementID("File123"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983c",
 		LicenseConcluded:   "Apache-2.0",
 		LicenseInfoInFile: []string{
@@ -213,7 +213,7 @@ FileCopyrightText: Copyright (c) Jane Doe
 func TestSaver2_1FileWrapsCopyrightMultiLine(t *testing.T) {
 	f := &spdx.File2_1{
 		FileName:           "/tmp/whatever.txt",
-		FileSPDXIdentifier: "SPDXRef-File123",
+		FileSPDXIdentifier: spdx.ElementID("File123"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983c",
 		LicenseConcluded:   "Apache-2.0",
 		LicenseInfoInFile: []string{
diff --git a/tvsaver/saver2v1/save_package.go b/tvsaver/saver2v1/save_package.go
index 080c362..619fa39 100644
--- a/tvsaver/saver2v1/save_package.go
+++ b/tvsaver/saver2v1/save_package.go
@@ -5,6 +5,7 @@ package saver2v1
 import (
 	"fmt"
 	"io"
+	"sort"
 
 	"github.com/spdx/tools-golang/spdx"
 )
@@ -14,7 +15,7 @@ func renderPackage2_1(pkg *spdx.Package2_1, w io.Writer) error {
 		fmt.Fprintf(w, "PackageName: %s\n", pkg.PackageName)
 	}
 	if pkg.PackageSPDXIdentifier != "" {
-		fmt.Fprintf(w, "SPDXID: %s\n", pkg.PackageSPDXIdentifier)
+		fmt.Fprintf(w, "SPDXID: %s\n", spdx.RenderElementID(pkg.PackageSPDXIdentifier))
 	}
 	if pkg.PackageVersion != "" {
 		fmt.Fprintf(w, "PackageVersion: %s\n", pkg.PackageVersion)
@@ -108,8 +109,15 @@ func renderPackage2_1(pkg *spdx.Package2_1, w io.Writer) error {
 	fmt.Fprintf(w, "\n")
 
 	// also render any files for this package
-	for _, f := range pkg.Files {
-		renderFile2_1(f, w)
+	// get slice of File identifiers so we can sort them
+	fileKeys := []string{}
+	for k := range pkg.Files {
+		fileKeys = append(fileKeys, string(k))
+	}
+	sort.Strings(fileKeys)
+	for _, fiID := range fileKeys {
+		fi := pkg.Files[spdx.ElementID(fiID)]
+		renderFile2_1(fi, w)
 	}
 
 	return nil
diff --git a/tvsaver/saver2v1/save_package_test.go b/tvsaver/saver2v1/save_package_test.go
index 3dbd985..fcde876 100644
--- a/tvsaver/saver2v1/save_package_test.go
+++ b/tvsaver/saver2v1/save_package_test.go
@@ -41,7 +41,7 @@ func TestSaver2_1PackageSavesTextCombo1(t *testing.T) {
 
 	pkg := &spdx.Package2_1{
 		PackageName:                         "p1",
-		PackageSPDXIdentifier:               "SPDXRef-p1",
+		PackageSPDXIdentifier:               spdx.ElementID("p1"),
 		PackageVersion:                      "0.1.0",
 		PackageFileName:                     "p1-0.1.0-master.tar.gz",
 		PackageSupplierOrganization:         "John Doe, Inc.",
@@ -129,7 +129,7 @@ func TestSaver2_1PackageSavesTextCombo2(t *testing.T) {
 
 	pkg := &spdx.Package2_1{
 		PackageName:                   "p1",
-		PackageSPDXIdentifier:         "SPDXRef-p1",
+		PackageSPDXIdentifier:         spdx.ElementID("p1"),
 		PackageVersion:                "0.1.0",
 		PackageFileName:               "p1-0.1.0-master.tar.gz",
 		PackageSupplierNOASSERTION:    true,
@@ -206,7 +206,7 @@ func TestSaver2_1PackageSavesTextCombo3(t *testing.T) {
 
 	pkg := &spdx.Package2_1{
 		PackageName:                  "p1",
-		PackageSPDXIdentifier:        "SPDXRef-p1",
+		PackageSPDXIdentifier:        spdx.ElementID("p1"),
 		PackageVersion:               "0.1.0",
 		PackageFileName:              "p1-0.1.0-master.tar.gz",
 		PackageSupplierPerson:        "John Doe",
@@ -279,7 +279,7 @@ PackageComment: this is a comment comment
 func TestSaver2_1PackageSaveOmitsOptionalFieldsIfEmpty(t *testing.T) {
 	pkg := &spdx.Package2_1{
 		PackageName:               "p1",
-		PackageSPDXIdentifier:     "SPDXRef-p1",
+		PackageSPDXIdentifier:     spdx.ElementID("p1"),
 		PackageDownloadLocation:   "http://example.com/p1/p1-0.1.0-master.tar.gz",
 		FilesAnalyzed:             false,
 		IsFilesAnalyzedTagPresent: true,
@@ -325,7 +325,7 @@ PackageCopyrightText: Copyright (c) John Doe, Inc.
 func TestSaver2_1PackageSavesFilesIfPresent(t *testing.T) {
 	f1 := &spdx.File2_1{
 		FileName:           "/tmp/whatever1.txt",
-		FileSPDXIdentifier: "SPDXRef-File1231",
+		FileSPDXIdentifier: spdx.ElementID("File1231"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983c",
 		LicenseConcluded:   "Apache-2.0",
 		LicenseInfoInFile:  []string{"Apache-2.0"},
@@ -334,7 +334,7 @@ func TestSaver2_1PackageSavesFilesIfPresent(t *testing.T) {
 
 	f2 := &spdx.File2_1{
 		FileName:           "/tmp/whatever2.txt",
-		FileSPDXIdentifier: "SPDXRef-File1232",
+		FileSPDXIdentifier: spdx.ElementID("File1232"),
 		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983d",
 		LicenseConcluded:   "MIT",
 		LicenseInfoInFile:  []string{"MIT"},
@@ -343,7 +343,7 @@ func TestSaver2_1PackageSavesFilesIfPresent(t *testing.T) {
 
 	pkg := &spdx.Package2_1{
 		PackageName:               "p1",
-		PackageSPDXIdentifier:     "SPDXRef-p1",
+		PackageSPDXIdentifier:     spdx.ElementID("p1"),
 		PackageDownloadLocation:   "http://example.com/p1/p1-0.1.0-master.tar.gz",
 		FilesAnalyzed:             false,
 		IsFilesAnalyzedTagPresent: true,
@@ -359,9 +359,9 @@ func TestSaver2_1PackageSavesFilesIfPresent(t *testing.T) {
 		},
 		PackageLicenseDeclared: "Apache-2.0 OR GPL-2.0-or-later",
 		PackageCopyrightText:   "Copyright (c) John Doe, Inc.",
-		Files: []*spdx.File2_1{
-			f1,
-			f2,
+		Files: map[spdx.ElementID]*spdx.File2_1{
+			spdx.ElementID("File1231"): f1,
+			spdx.ElementID("File1232"): f2,
 		},
 	}
 
@@ -403,60 +403,3 @@ FileCopyrightText: Copyright (c) John Doe
 		t.Errorf("Expected %v, got %v", want.String(), got.String())
 	}
 }
-
-func TestSaver2_1PackageSavesUnpackagedFilesIfPresent(t *testing.T) {
-	f1 := &spdx.File2_1{
-		FileName:           "/tmp/whatever1.txt",
-		FileSPDXIdentifier: "SPDXRef-File1231",
-		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983c",
-		LicenseConcluded:   "Apache-2.0",
-		LicenseInfoInFile:  []string{"Apache-2.0"},
-		FileCopyrightText:  "Copyright (c) Jane Doe",
-	}
-
-	f2 := &spdx.File2_1{
-		FileName:           "/tmp/whatever2.txt",
-		FileSPDXIdentifier: "SPDXRef-File1232",
-		FileChecksumSHA1:   "85ed0817af83a24ad8da68c2b5094de69833983d",
-		LicenseConcluded:   "MIT",
-		LicenseInfoInFile:  []string{"MIT"},
-		FileCopyrightText:  "Copyright (c) John Doe",
-	}
-
-	unFiles := map[spdx.ElementID]*spdx.File2_1{
-		"File1231": f1,
-		"File1232": f2,
-	}
-
-	// what we want to get, as a buffer of bytes
-	want := bytes.NewBufferString(`FileName: /tmp/whatever1.txt
-SPDXID: SPDXRef-File1231
-FileChecksum: SHA1: 85ed0817af83a24ad8da68c2b5094de69833983c
-LicenseConcluded: Apache-2.0
-LicenseInfoInFile: Apache-2.0
-FileCopyrightText: Copyright (c) Jane Doe
-
-FileName: /tmp/whatever2.txt
-SPDXID: SPDXRef-File1232
-FileChecksum: SHA1: 85ed0817af83a24ad8da68c2b5094de69833983d
-LicenseConcluded: MIT
-LicenseInfoInFile: MIT
-FileCopyrightText: Copyright (c) John Doe
-
-`)
-
-	// render as buffer of bytes
-	var got bytes.Buffer
-	for fi := unFiles {
-		err := renderFile2_1(fi, &got)
-		if err != nil {
-			t.Errorf("Expected nil error, got %v", err)
-		}
-	}
-
-	// check that they match
-	c := bytes.Compare(want.Bytes(), got.Bytes())
-	if c != 0 {
-		t.Errorf("Expected %v, got %v", want.String(), got.String())
-	}
-}
diff --git a/tvsaver/saver2v1/save_relationship.go b/tvsaver/saver2v1/save_relationship.go
index 0ba0670..6bde4b3 100644
--- a/tvsaver/saver2v1/save_relationship.go
+++ b/tvsaver/saver2v1/save_relationship.go
@@ -10,8 +10,10 @@ import (
 )
 
 func renderRelationship2_1(rln *spdx.Relationship2_1, w io.Writer) error {
-	if rln.RefA != "" && rln.RefB != "" && rln.Relationship != "" {
-		fmt.Fprintf(w, "Relationship: %s %s %s\n", rln.RefA, rln.Relationship, rln.RefB)
+	rlnAStr := spdx.RenderDocElementID(rln.RefA)
+	rlnBStr := spdx.RenderDocElementID(rln.RefB)
+	if rlnAStr != "SPDXRef-" && rlnBStr != "SPDXRef-" && rln.Relationship != "" {
+		fmt.Fprintf(w, "Relationship: %s %s %s\n", rlnAStr, rln.Relationship, rlnBStr)
 	}
 	if rln.RelationshipComment != "" {
 		fmt.Fprintf(w, "RelationshipComment: %s\n", rln.RelationshipComment)
diff --git a/tvsaver/saver2v1/save_relationship_test.go b/tvsaver/saver2v1/save_relationship_test.go
index 9f4191a..e8635b1 100644
--- a/tvsaver/saver2v1/save_relationship_test.go
+++ b/tvsaver/saver2v1/save_relationship_test.go
@@ -12,8 +12,8 @@ import (
 // ===== Relationship section Saver tests =====
 func TestSaver2_1RelationshipSavesText(t *testing.T) {
 	rln := &spdx.Relationship2_1{
-		RefA:                "SPDXRef-DOCUMENT",
-		RefB:                "SPDXRef-2",
+		RefA:                spdx.MakeDocElementID("", "DOCUMENT"),
+		RefB:                spdx.MakeDocElementID("", "2"),
 		Relationship:        "DESCRIBES",
 		RelationshipComment: "this is a comment",
 	}
@@ -40,8 +40,8 @@ RelationshipComment: this is a comment
 
 func TestSaver2_1RelationshipOmitsOptionalFieldsIfEmpty(t *testing.T) {
 	rln := &spdx.Relationship2_1{
-		RefA:         "SPDXRef-DOCUMENT",
-		RefB:         "SPDXRef-2",
+		RefA:         spdx.MakeDocElementID("", "DOCUMENT"),
+		RefB:         spdx.MakeDocElementID("", "2"),
 		Relationship: "DESCRIBES",
 	}
 
diff --git a/tvsaver/saver2v1/save_snippet.go b/tvsaver/saver2v1/save_snippet.go
index d42282b..e82bc5a 100644
--- a/tvsaver/saver2v1/save_snippet.go
+++ b/tvsaver/saver2v1/save_snippet.go
@@ -11,10 +11,11 @@ import (
 
 func renderSnippet2_1(sn *spdx.Snippet2_1, w io.Writer) error {
 	if sn.SnippetSPDXIdentifier != "" {
-		fmt.Fprintf(w, "SnippetSPDXIdentifier: %s\n", sn.SnippetSPDXIdentifier)
+		fmt.Fprintf(w, "SnippetSPDXIdentifier: %s\n", spdx.RenderElementID(sn.SnippetSPDXIdentifier))
 	}
-	if sn.SnippetFromFileSPDXIdentifier != "" {
-		fmt.Fprintf(w, "SnippetFromFileSPDXID: %s\n", sn.SnippetFromFileSPDXIdentifier)
+	snFromFileIDStr := spdx.RenderDocElementID(sn.SnippetFromFileSPDXIdentifier)
+	if snFromFileIDStr != "" {
+		fmt.Fprintf(w, "SnippetFromFileSPDXID: %s\n", snFromFileIDStr)
 	}
 	if sn.SnippetByteRangeStart != 0 && sn.SnippetByteRangeEnd != 0 {
 		fmt.Fprintf(w, "SnippetByteRange: %d:%d\n", sn.SnippetByteRangeStart, sn.SnippetByteRangeEnd)
diff --git a/tvsaver/saver2v1/save_snippet_test.go b/tvsaver/saver2v1/save_snippet_test.go
index e4c76e9..88b39c9 100644
--- a/tvsaver/saver2v1/save_snippet_test.go
+++ b/tvsaver/saver2v1/save_snippet_test.go
@@ -12,8 +12,8 @@ import (
 // ===== Snippet section Saver tests =====
 func TestSaver2_1SnippetSavesText(t *testing.T) {
 	sn := &spdx.Snippet2_1{
-		SnippetSPDXIdentifier:         "SPDXRef-Snippet17",
-		SnippetFromFileSPDXIdentifier: "SPDXRef-File292",
+		SnippetSPDXIdentifier:         spdx.ElementID("Snippet17"),
+		SnippetFromFileSPDXIdentifier: spdx.MakeDocElementID("", "File292"),
 		SnippetByteRangeStart:         17,
 		SnippetByteRangeEnd:           209,
 		SnippetLineRangeStart:         3,
@@ -60,8 +60,8 @@ SnippetName: from John's program
 
 func TestSaver2_1SnippetOmitsOptionalFieldsIfEmpty(t *testing.T) {
 	sn := &spdx.Snippet2_1{
-		SnippetSPDXIdentifier:         "SPDXRef-Snippet17",
-		SnippetFromFileSPDXIdentifier: "SPDXRef-File292",
+		SnippetSPDXIdentifier:         spdx.ElementID("Snippet17"),
+		SnippetFromFileSPDXIdentifier: spdx.MakeDocElementID("", "File292"),
 		SnippetByteRangeStart:         17,
 		SnippetByteRangeEnd:           209,
 		SnippetLicenseConcluded:       "GPL-2.0-or-later",