Refactor parser to handle element ID maps

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

4 files changed, 120 insertions, 149 deletions

diff --git a/tvsaver/saver2v1/save_document.go b/tvsaver/saver2v1/save_document.go
index 97891f5..e573ad3 100644
--- a/tvsaver/saver2v1/save_document.go
+++ b/tvsaver/saver2v1/save_document.go
@@ -22,12 +22,15 @@ func RenderDocument2_1(doc *spdx.Document2_1, w io.Writer) error {
 
 	renderCreationInfo2_1(doc.CreationInfo, w)
 
-	for _, pkg := range doc.Packages {
-		if pkg.IsUnpackaged == true {
-			fmt.Fprintf(w, "##### Unpackaged files\n\n")
-		} else {
-			fmt.Fprintf(w, "##### Package: %s\n\n", pkg.PackageName)
+	if len(doc.UnpackagedFiles) > 0 {
+		fmt.Fprintf(w, "##### Unpackaged files\n\n")
+		for _, fi := range doc.UnpackagedFiles {
+			renderFile2_1(fi, w)
 		}
+	}
+
+	for _, pkg := range doc.Packages {
+		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 fd47b9c..f75093d 100644
--- a/tvsaver/saver2v1/save_document_test.go
+++ b/tvsaver/saver2v1/save_document_test.go
@@ -25,7 +25,7 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 		Created: "2018-10-10T06:20:00Z",
 	}
 
-	// Package 1: unpackaged files
+	// unpackaged files
 	f1 := &spdx.File2_1{
 		FileName:           "/tmp/whatever1.txt",
 		FileSPDXIdentifier: "SPDXRef-File1231",
@@ -44,15 +44,12 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 		FileCopyrightText:  "Copyright (c) John Doe",
 	}
 
-	pkgUn := &spdx.Package2_1{
-		IsUnpackaged: true,
-		Files: []*spdx.File2_1{
-			f1,
-			f2,
-		},
+	unFiles := map[spdx.ElementID]*spdx.File2_1{
+		"File1231": f1,
+		"File1232": f2,
 	}
 
-	// Package 2: packaged files with snippets
+	// Package 1: packaged files with snippets
 	sn1 := &spdx.Snippet2_1{
 		SnippetSPDXIdentifier:         "SPDXRef-Snippet19",
 		SnippetFromFileSPDXIdentifier: "SPDXRef-FileHasSnippets",
@@ -82,9 +79,9 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 			"WTFPL",
 		},
 		FileCopyrightText: "Copyright (c) Jane Doe",
-		Snippets: []*spdx.Snippet2_1{
-			sn1,
-			sn2,
+		Snippets: map[spdx.ElementID]*spdx.Snippet2_1{
+			"Snippet19": sn1,
+			"Snippet20": sn2,
 		},
 	}
 
@@ -98,7 +95,6 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 	}
 
 	pkgWith := &spdx.Package2_1{
-		IsUnpackaged:              false,
 		PackageName:               "p1",
 		PackageSPDXIdentifier:     "SPDXRef-p1",
 		PackageDownloadLocation:   "http://example.com/p1/p1-0.1.0-master.tar.gz",
@@ -114,9 +110,9 @@ func TestSaver2_1DocumentSavesText(t *testing.T) {
 		},
 		PackageLicenseDeclared: "Apache-2.0 OR GPL-2.0-or-later",
 		PackageCopyrightText:   "Copyright (c) John Doe, Inc.",
-		Files: []*spdx.File2_1{
-			f3,
-			f4,
+		Files: map[spdx.ElementID]*spdx.File2_1{
+			"FileHasSnippets": f3,
+			"FileAnother":     f4,
 		},
 	}
 
@@ -189,10 +185,10 @@ blah blah blah blah`,
 	// now, build the document
 	doc := &spdx.Document2_1{
 		CreationInfo: ci,
-		Packages: []*spdx.Package2_1{
-			pkgUn,
+		Packages: map[spdx.ElementID]*spdx.Package2_1{
 			pkgWith,
 		},
+		UnpackagedFiles: unFiles,
 		OtherLicenses: []*spdx.OtherLicense2_1{
 			ol1,
 			ol2,
diff --git a/tvsaver/saver2v1/save_package.go b/tvsaver/saver2v1/save_package.go
index 3c83547..080c362 100644
--- a/tvsaver/saver2v1/save_package.go
+++ b/tvsaver/saver2v1/save_package.go
@@ -10,106 +10,104 @@ import (
 )
 
 func renderPackage2_1(pkg *spdx.Package2_1, w io.Writer) error {
-	if pkg.IsUnpackaged == false {
-		if pkg.PackageName != "" {
-			fmt.Fprintf(w, "PackageName: %s\n", pkg.PackageName)
-		}
-		if pkg.PackageSPDXIdentifier != "" {
-			fmt.Fprintf(w, "SPDXID: %s\n", pkg.PackageSPDXIdentifier)
-		}
-		if pkg.PackageVersion != "" {
-			fmt.Fprintf(w, "PackageVersion: %s\n", pkg.PackageVersion)
-		}
-		if pkg.PackageFileName != "" {
-			fmt.Fprintf(w, "PackageFileName: %s\n", pkg.PackageFileName)
-		}
-		if pkg.PackageSupplierPerson != "" {
-			fmt.Fprintf(w, "PackageSupplier: Person: %s\n", pkg.PackageSupplierPerson)
-		}
-		if pkg.PackageSupplierOrganization != "" {
-			fmt.Fprintf(w, "PackageSupplier: Organization: %s\n", pkg.PackageSupplierOrganization)
-		}
-		if pkg.PackageSupplierNOASSERTION == true {
-			fmt.Fprintf(w, "PackageSupplier: NOASSERTION\n")
-		}
-		if pkg.PackageOriginatorPerson != "" {
-			fmt.Fprintf(w, "PackageOriginator: Person: %s\n", pkg.PackageOriginatorPerson)
-		}
-		if pkg.PackageOriginatorOrganization != "" {
-			fmt.Fprintf(w, "PackageOriginator: Organization: %s\n", pkg.PackageOriginatorOrganization)
-		}
-		if pkg.PackageOriginatorNOASSERTION == true {
-			fmt.Fprintf(w, "PackageOriginator: NOASSERTION\n")
-		}
-		if pkg.PackageDownloadLocation != "" {
-			fmt.Fprintf(w, "PackageDownloadLocation: %s\n", pkg.PackageDownloadLocation)
+	if pkg.PackageName != "" {
+		fmt.Fprintf(w, "PackageName: %s\n", pkg.PackageName)
+	}
+	if pkg.PackageSPDXIdentifier != "" {
+		fmt.Fprintf(w, "SPDXID: %s\n", pkg.PackageSPDXIdentifier)
+	}
+	if pkg.PackageVersion != "" {
+		fmt.Fprintf(w, "PackageVersion: %s\n", pkg.PackageVersion)
+	}
+	if pkg.PackageFileName != "" {
+		fmt.Fprintf(w, "PackageFileName: %s\n", pkg.PackageFileName)
+	}
+	if pkg.PackageSupplierPerson != "" {
+		fmt.Fprintf(w, "PackageSupplier: Person: %s\n", pkg.PackageSupplierPerson)
+	}
+	if pkg.PackageSupplierOrganization != "" {
+		fmt.Fprintf(w, "PackageSupplier: Organization: %s\n", pkg.PackageSupplierOrganization)
+	}
+	if pkg.PackageSupplierNOASSERTION == true {
+		fmt.Fprintf(w, "PackageSupplier: NOASSERTION\n")
+	}
+	if pkg.PackageOriginatorPerson != "" {
+		fmt.Fprintf(w, "PackageOriginator: Person: %s\n", pkg.PackageOriginatorPerson)
+	}
+	if pkg.PackageOriginatorOrganization != "" {
+		fmt.Fprintf(w, "PackageOriginator: Organization: %s\n", pkg.PackageOriginatorOrganization)
+	}
+	if pkg.PackageOriginatorNOASSERTION == true {
+		fmt.Fprintf(w, "PackageOriginator: NOASSERTION\n")
+	}
+	if pkg.PackageDownloadLocation != "" {
+		fmt.Fprintf(w, "PackageDownloadLocation: %s\n", pkg.PackageDownloadLocation)
+	}
+	if pkg.FilesAnalyzed == true {
+		if pkg.IsFilesAnalyzedTagPresent == true {
+			fmt.Fprintf(w, "FilesAnalyzed: true\n")
 		}
-		if pkg.FilesAnalyzed == true {
-			if pkg.IsFilesAnalyzedTagPresent == true {
-				fmt.Fprintf(w, "FilesAnalyzed: true\n")
-			}
+	} else {
+		fmt.Fprintf(w, "FilesAnalyzed: false\n")
+	}
+	if pkg.PackageVerificationCode != "" && pkg.FilesAnalyzed == true {
+		if pkg.PackageVerificationCodeExcludedFile == "" {
+			fmt.Fprintf(w, "PackageVerificationCode: %s\n", pkg.PackageVerificationCode)
 		} else {
-			fmt.Fprintf(w, "FilesAnalyzed: false\n")
-		}
-		if pkg.PackageVerificationCode != "" && pkg.FilesAnalyzed == true {
-			if pkg.PackageVerificationCodeExcludedFile == "" {
-				fmt.Fprintf(w, "PackageVerificationCode: %s\n", pkg.PackageVerificationCode)
-			} else {
-				fmt.Fprintf(w, "PackageVerificationCode: %s (excludes %s)\n", pkg.PackageVerificationCode, pkg.PackageVerificationCodeExcludedFile)
-			}
-		}
-		if pkg.PackageChecksumSHA1 != "" {
-			fmt.Fprintf(w, "PackageChecksum: SHA1: %s\n", pkg.PackageChecksumSHA1)
-		}
-		if pkg.PackageChecksumSHA256 != "" {
-			fmt.Fprintf(w, "PackageChecksum: SHA256: %s\n", pkg.PackageChecksumSHA256)
-		}
-		if pkg.PackageChecksumMD5 != "" {
-			fmt.Fprintf(w, "PackageChecksum: MD5: %s\n", pkg.PackageChecksumMD5)
+			fmt.Fprintf(w, "PackageVerificationCode: %s (excludes %s)\n", pkg.PackageVerificationCode, pkg.PackageVerificationCodeExcludedFile)
 		}
-		if pkg.PackageHomePage != "" {
-			fmt.Fprintf(w, "PackageHomePage: %s\n", pkg.PackageHomePage)
-		}
-		if pkg.PackageSourceInfo != "" {
-			fmt.Fprintf(w, "PackageSourceInfo: %s\n", textify(pkg.PackageSourceInfo))
-		}
-		if pkg.PackageLicenseConcluded != "" {
-			fmt.Fprintf(w, "PackageLicenseConcluded: %s\n", pkg.PackageLicenseConcluded)
-		}
-		if pkg.FilesAnalyzed == true {
-			for _, s := range pkg.PackageLicenseInfoFromFiles {
-				fmt.Fprintf(w, "PackageLicenseInfoFromFiles: %s\n", s)
-			}
-		}
-		if pkg.PackageLicenseDeclared != "" {
-			fmt.Fprintf(w, "PackageLicenseDeclared: %s\n", pkg.PackageLicenseDeclared)
-		}
-		if pkg.PackageLicenseComments != "" {
-			fmt.Fprintf(w, "PackageLicenseComments: %s\n", textify(pkg.PackageLicenseComments))
-		}
-		if pkg.PackageCopyrightText != "" {
-			fmt.Fprintf(w, "PackageCopyrightText: %s\n", pkg.PackageCopyrightText)
-		}
-		if pkg.PackageSummary != "" {
-			fmt.Fprintf(w, "PackageSummary: %s\n", textify(pkg.PackageSummary))
-		}
-		if pkg.PackageDescription != "" {
-			fmt.Fprintf(w, "PackageDescription: %s\n", textify(pkg.PackageDescription))
-		}
-		if pkg.PackageComment != "" {
-			fmt.Fprintf(w, "PackageComment: %s\n", textify(pkg.PackageComment))
+	}
+	if pkg.PackageChecksumSHA1 != "" {
+		fmt.Fprintf(w, "PackageChecksum: SHA1: %s\n", pkg.PackageChecksumSHA1)
+	}
+	if pkg.PackageChecksumSHA256 != "" {
+		fmt.Fprintf(w, "PackageChecksum: SHA256: %s\n", pkg.PackageChecksumSHA256)
+	}
+	if pkg.PackageChecksumMD5 != "" {
+		fmt.Fprintf(w, "PackageChecksum: MD5: %s\n", pkg.PackageChecksumMD5)
+	}
+	if pkg.PackageHomePage != "" {
+		fmt.Fprintf(w, "PackageHomePage: %s\n", pkg.PackageHomePage)
+	}
+	if pkg.PackageSourceInfo != "" {
+		fmt.Fprintf(w, "PackageSourceInfo: %s\n", textify(pkg.PackageSourceInfo))
+	}
+	if pkg.PackageLicenseConcluded != "" {
+		fmt.Fprintf(w, "PackageLicenseConcluded: %s\n", pkg.PackageLicenseConcluded)
+	}
+	if pkg.FilesAnalyzed == true {
+		for _, s := range pkg.PackageLicenseInfoFromFiles {
+			fmt.Fprintf(w, "PackageLicenseInfoFromFiles: %s\n", s)
 		}
-		for _, s := range pkg.PackageExternalReferences {
-			fmt.Fprintf(w, "ExternalRef: %s %s %s\n", s.Category, s.RefType, s.Locator)
-			if s.ExternalRefComment != "" {
-				fmt.Fprintf(w, "ExternalRefComment: %s\n", s.ExternalRefComment)
-			}
+	}
+	if pkg.PackageLicenseDeclared != "" {
+		fmt.Fprintf(w, "PackageLicenseDeclared: %s\n", pkg.PackageLicenseDeclared)
+	}
+	if pkg.PackageLicenseComments != "" {
+		fmt.Fprintf(w, "PackageLicenseComments: %s\n", textify(pkg.PackageLicenseComments))
+	}
+	if pkg.PackageCopyrightText != "" {
+		fmt.Fprintf(w, "PackageCopyrightText: %s\n", pkg.PackageCopyrightText)
+	}
+	if pkg.PackageSummary != "" {
+		fmt.Fprintf(w, "PackageSummary: %s\n", textify(pkg.PackageSummary))
+	}
+	if pkg.PackageDescription != "" {
+		fmt.Fprintf(w, "PackageDescription: %s\n", textify(pkg.PackageDescription))
+	}
+	if pkg.PackageComment != "" {
+		fmt.Fprintf(w, "PackageComment: %s\n", textify(pkg.PackageComment))
+	}
+	for _, s := range pkg.PackageExternalReferences {
+		fmt.Fprintf(w, "ExternalRef: %s %s %s\n", s.Category, s.RefType, s.Locator)
+		if s.ExternalRefComment != "" {
+			fmt.Fprintf(w, "ExternalRefComment: %s\n", s.ExternalRefComment)
 		}
-
-		fmt.Fprintf(w, "\n")
 	}
 
-	// also render any files for this package, even if unpackaged
+	fmt.Fprintf(w, "\n")
+
+	// also render any files for this package
 	for _, f := range pkg.Files {
 		renderFile2_1(f, w)
 	}
diff --git a/tvsaver/saver2v1/save_package_test.go b/tvsaver/saver2v1/save_package_test.go
index deb80a8..3dbd985 100644
--- a/tvsaver/saver2v1/save_package_test.go
+++ b/tvsaver/saver2v1/save_package_test.go
@@ -40,7 +40,6 @@ func TestSaver2_1PackageSavesTextCombo1(t *testing.T) {
 	}
 
 	pkg := &spdx.Package2_1{
-		IsUnpackaged:                        false,
 		PackageName:                         "p1",
 		PackageSPDXIdentifier:               "SPDXRef-p1",
 		PackageVersion:                      "0.1.0",
@@ -129,7 +128,6 @@ func TestSaver2_1PackageSavesTextCombo2(t *testing.T) {
 	// PackageVerificationCodeExcludedFile is empty
 
 	pkg := &spdx.Package2_1{
-		IsUnpackaged:                  false,
 		PackageName:                   "p1",
 		PackageSPDXIdentifier:         "SPDXRef-p1",
 		PackageVersion:                "0.1.0",
@@ -207,7 +205,6 @@ func TestSaver2_1PackageSavesTextCombo3(t *testing.T) {
 	// PackageVerificationCodeExcludedFile is empty
 
 	pkg := &spdx.Package2_1{
-		IsUnpackaged:                 false,
 		PackageName:                  "p1",
 		PackageSPDXIdentifier:        "SPDXRef-p1",
 		PackageVersion:               "0.1.0",
@@ -281,7 +278,6 @@ PackageComment: this is a comment comment
 
 func TestSaver2_1PackageSaveOmitsOptionalFieldsIfEmpty(t *testing.T) {
 	pkg := &spdx.Package2_1{
-		IsUnpackaged:              false,
 		PackageName:               "p1",
 		PackageSPDXIdentifier:     "SPDXRef-p1",
 		PackageDownloadLocation:   "http://example.com/p1/p1-0.1.0-master.tar.gz",
@@ -346,7 +342,6 @@ func TestSaver2_1PackageSavesFilesIfPresent(t *testing.T) {
 	}
 
 	pkg := &spdx.Package2_1{
-		IsUnpackaged:              false,
 		PackageName:               "p1",
 		PackageSPDXIdentifier:     "SPDXRef-p1",
 		PackageDownloadLocation:   "http://example.com/p1/p1-0.1.0-master.tar.gz",
@@ -428,12 +423,9 @@ func TestSaver2_1PackageSavesUnpackagedFilesIfPresent(t *testing.T) {
 		FileCopyrightText:  "Copyright (c) John Doe",
 	}
 
-	pkg := &spdx.Package2_1{
-		IsUnpackaged: true,
-		Files: []*spdx.File2_1{
-			f1,
-			f2,
-		},
+	unFiles := map[spdx.ElementID]*spdx.File2_1{
+		"File1231": f1,
+		"File1232": f2,
 	}
 
 	// what we want to get, as a buffer of bytes
@@ -455,29 +447,11 @@ FileCopyrightText: Copyright (c) John Doe
 
 	// render as buffer of bytes
 	var got bytes.Buffer
-	err := renderPackage2_1(pkg, &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())
-	}
-}
-
-func TestSaver2_1PackageSavesNothingIfUnpackagedAndNoFilesPresent(t *testing.T) {
-	pkg := &spdx.Package2_1{IsUnpackaged: true}
-
-	// what we want to get, as a buffer of bytes
-	want := bytes.NewBufferString("")
-
-	// render as buffer of bytes
-	var got bytes.Buffer
-	err := renderPackage2_1(pkg, &got)
-	if err != nil {
-		t.Errorf("Expected nil error, got %v", err)
+	for fi := unFiles {
+		err := renderFile2_1(fi, &got)
+		if err != nil {
+			t.Errorf("Expected nil error, got %v", err)
+		}
 	}
 
 	// check that they match