Merge branch 'main' into dependabot/github_actions/actions/setup-go-4

CODEOWNERS

@@ -1,2 +1,2 @@
 # This repository is maintained by: 
-* @elrayle @ajhenry
+* @elrayle @ajhenry @dangoor

README.md

@@ -1,3 +1,5 @@
+[![Go Reference](https://pkg.go.dev/badge/github.com/github/go-spdx/v2@v2.1.2/spdxexp.svg)](https://pkg.go.dev/github.com/github/go-spdx/v2@v2.1.2/spdxexp)
+
 # go-spdx
 
 Golang implementation of a checker for determining if a set of SPDX IDs satisfies an SPDX Expression.
@@ -119,6 +121,15 @@ assert.Contains(invalidLicenses, "NON-EXISTENT-LICENSE")
 assert.NotContains(invalidLicenses, "MIT")
 ```
 
+#### Examples: ValidateLicenses works with SPDX expressions
+
+```go
+valid, invalidLicenses := ValidateLicenses([]string{"MIT AND APACHE-2.0"})
+assert.True(valid)
+assert.NotContains(invalidLicenses, "MIT AND APACHE-2.0")
+```
+
+
 ## Background
 
 This package was developed to support testing whether a repository's license requirements are met by an allowed-list of licenses.

spdxexp/compare.go

@@ -1,5 +1,13 @@
 package spdxexp
 
+// The compare methods determine if two ranges are greater than, less than or equal within the same license group.
+// NOTE: Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
+//       Groups have sub-groups of license versions (referred to as the range) where each member is considered
+//       to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
+//       the license group, such that the first sub-group is considered to be less than the second sub-group,
+//       and so on. (e.g. {{GPL-1.0}, {GPL-2.0, GPL-2.0-only}} implies {GPL-1.0} < {GPL-2.0, GPL-2.0-only}).
+
+// compareGT returns true if the first range is greater than the second range within the same license group; otherwise, false.
 func compareGT(first *node, second *node) bool {
 	if !first.isLicense() || !second.isLicense() {
 		return false
@@ -13,6 +21,7 @@ func compareGT(first *node, second *node) bool {
 	return firstRange.location[versionGroup] > secondRange.location[versionGroup]
 }
 
+// compareLT returns true if the first range is less than the second range within the same license group; otherwise, false.
 func compareLT(first *node, second *node) bool {
 	if !first.isLicense() || !second.isLicense() {
 		return false
@@ -26,10 +35,15 @@ func compareLT(first *node, second *node) bool {
 	return firstRange.location[versionGroup] < secondRange.location[versionGroup]
 }
 
+// compareEQ returns true if the first and second range are the same range within the same license group; otherwise, false.
 func compareEQ(first *node, second *node) bool {
 	if !first.isLicense() || !second.isLicense() {
 		return false
 	}
+	if first.lic.license == second.lic.license {
+		return true
+	}
+
 	firstRange := getLicenseRange(*first.license())
 	secondRange := getLicenseRange(*second.license())
 
@@ -39,6 +53,8 @@ func compareEQ(first *node, second *node) bool {
 	return firstRange.location[versionGroup] == secondRange.location[versionGroup]
 }
 
+// sameLicenseGroup returns false if either license isn't in a range or the two ranges are
+// not in the same license group (e.g. group GPL != group Apache); otherwise, true
 func sameLicenseGroup(firstRange *licenseRange, secondRange *licenseRange) bool {
 	if firstRange == nil || secondRange == nil || firstRange.location[licenseGroup] != secondRange.location[licenseGroup] {
 		return false

spdxexp/compare_test.go

@@ -20,9 +20,12 @@ func TestCompareGT(t *testing.T) {
 		{"expect greater than: AGPL-3.0 > AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), true},
 		{"expect equal: GPL-2.0-or-later > GPL-2.0-only", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0-only", false), false},
 		{"expect equal: GPL-2.0-or-later > GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), false},
+		{"expect equal: GPL-2.0-only > GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), false},
 		{"expect equal: GPL-3.0 > GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), false},
+		{"expect equal: MIT > MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), false},
 		{"expect less than: MPL-1.0 > MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), false},
 		{"incompatible: MIT > ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
+		{"incompatible: MIT > GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
 		{"incompatible: OSL-1.0 > OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
 		{"not simple license: (MIT OR ISC) > GPL-3.0", getLicenseNode("(MIT OR ISC)", false), getLicenseNode("GPL-3.0", false), false}, // TODO: should it raise error?
 	}
@@ -49,9 +52,12 @@ func TestCompareEQ(t *testing.T) {
 		{"expect greater than: AGPL-3.0 == AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), false},
 		{"expect equal: GPL-2.0-or-later > GPL-2.0-only", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0-only", false), true},
 		{"expect equal: GPL-2.0-or-later > GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), true},
+		{"expect equal: GPL-2.0-only == GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), true},
 		{"expect equal: GPL-3.0 == GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), true},
+		{"expect equal: MIT == MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), true},
 		{"expect less than: MPL-1.0 == MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), false},
 		{"incompatible: MIT == ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
+		{"incompatible: MIT == GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
 		{"incompatible: OSL-1.0 == OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
 		{"not simple license: (MIT OR ISC) == GPL-3.0", getLicenseNode("(MIT OR ISC)", false), getLicenseNode("GPL-3.0", false), false}, // TODO: should it raise error?
 	}
@@ -78,9 +84,12 @@ func TestCompareLT(t *testing.T) {
 		{"expect greater than: AGPL-3.0 < AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), false},
 		{"expect greater than: GPL-2.0-or-later < GPL-2.0-only", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0-only", false), false},
 		{"expect greater than: GPL-2.0-or-later == GPL-2.0", getLicenseNode("GPL-2.0-or-later", true), getLicenseNode("GPL-2.0", false), false},
+		{"expect equal: GPL-2.0-only < GPL-2.0", getLicenseNode("GPL-2.0-only", false), getLicenseNode("GPL-2.0", false), false},
 		{"expect equal: GPL-3.0 < GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", false), false},
+		{"expect equal: MIT < MIT", getLicenseNode("MIT", false), getLicenseNode("MIT", false), false},
 		{"expect less than: MPL-1.0 < MPL-2.0", getLicenseNode("MPL-1.0", false), getLicenseNode("MPL-2.0", false), true},
 		{"incompatible: MIT < ISC", getLicenseNode("MIT", false), getLicenseNode("ISC", false), false},
+		{"incompatible: MIT < GPL-2.0-only", getLicenseNode("MIT", false), getLicenseNode("GPL-2.0-only", false), false},
 		{"incompatible: OSL-1.0 < OPL-1.0", getLicenseNode("OSL-1.0", false), getLicenseNode("OPL-1.0", false), false},
 		{"not simple license: (MIT OR ISC) < GPL-3.0", getLicenseNode("(MIT OR ISC)", false), getLicenseNode("GPL-3.0", false), false}, // TODO: should it raise error?
 	}

spdxexp/license.go

@@ -640,6 +640,13 @@ func getExceptions() []string {
 	}
 }
 
+// licenseRanges returns a list of license ranges.
+//
+// Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
+// Groups have sub-groups of license versions (referred to as the range) where each member is considered
+// to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
+// the license group, such that the first sub-group is considered to be less than the second sub-group,
+// and so on. (e.g. {{GPL-1.0}, {GPL-2.0, GPL-2.0-only}} implies {GPL-1.0} < {GPL-2.0, GPL-2.0-only}).
 func licenseRanges() [][][]string {
 	return [][][]string{
 		{

spdxexp/node.go

@@ -89,7 +89,7 @@ func (n *node) isLicense() bool {
 	return n.role == licenseNode
 }
 
-// Return the value of the license field.
+// license returns the value of the license field.
 // See also reconstructedLicenseString()
 func (n *node) license() *string {
 	if !n.isLicense() {
@@ -167,7 +167,7 @@ func (n *node) reconstructedLicenseString() *string {
 	return nil
 }
 
-// Sort an array of license and license reference nodes alphabetically based
+// sortLicenses sorts an array of license and license reference nodes alphabetically based
 // on their reconstructedLicenseString() representation.  The sort function does not expect
 // expression nodes, but if one is in the nodes list, it will sort to the end.
 func sortLicenses(nodes []*node) {
@@ -186,29 +186,51 @@ func sortLicenses(nodes []*node) {
 
 // ---------------------- Comparator Methods ----------------------
 
-// Return true if two licenses are compatible; otherwise, false.
+// licensesAreCompatible returns true if two licenses are compatible; otherwise, false.
+// Two licenses are compatible if they are the same license or if they are in the same
+// license group and they meet one of the following rules:
+//
+// * both licenses have the `hasPlus` flag set to true
+// * the first license has the `hasPlus` flag and the second license is in the first license's range or greater
+// * the second license has the `hasPlus` flag and the first license is in the second license's range or greater
+// * both licenses are in the same range
 func (nodes *nodePair) licensesAreCompatible() bool {
+	// checking ranges is expensive, so check for simple cases first
 	if !nodes.firstNode.isLicense() || !nodes.secondNode.isLicense() {
 		return false
 	}
+	if !nodes.exceptionsAreCompatible() {
+		return false
+	}
+	if nodes.licensesExactlyEqual() {
+		return true
+	}
+
+	// simple cases don't apply, so check license ranges
+	// NOTE: Ranges are organized into groups (referred to as license groups) of the same base license (e.g. GPL).
+	//       Groups have sub-groups of license versions (referred to as the range) where each member is considered
+	//       to be the same version (e.g. {GPL-2.0, GPL-2.0-only}). The sub-groups are in ascending order within
+	//       the license group, such that the first sub-group is considered to be less than the second sub-group,
+	//       and so on. (e.g. {{GPL-1.0}, {GPL-2.0, GPL-2.0-only}} implies {GPL-1.0} < {GPL-2.0, GPL-2.0-only}).
 	if nodes.secondNode.hasPlus() {
 		if nodes.firstNode.hasPlus() {
-			// first+, second+
+			// first+, second+ just need to be in same range group
 			return nodes.rangesAreCompatible()
 		}
-		// first, second+
+		// first, second+ requires first to be in range of second
 		return nodes.identifierInRange()
 	}
 	// else secondNode does not have plus
 	if nodes.firstNode.hasPlus() {
-		// first+, second
+		// first+, second requires second to be in range of first
 		revNodes := &nodePair{firstNode: nodes.secondNode, secondNode: nodes.firstNode}
 		return revNodes.identifierInRange()
 	}
-	// first, second
-	return nodes.licensesExactlyEqual()
+	// first, second requires both to be in same range group
+	return nodes.rangesEqual()
 }
 
+// licenseRefsAreCompatible returns true if two license references are compatible; otherwise, false.
 func (nodes *nodePair) licenseRefsAreCompatible() bool {
 	if !nodes.firstNode.isLicenseRef() || !nodes.secondNode.isLicenseRef() {
 		return false
@@ -222,13 +244,9 @@ func (nodes *nodePair) licenseRefsAreCompatible() bool {
 	return compatible
 }
 
-// Return true if two licenses are compatible in the context of their ranges; otherwise, false.
+// licenseRefsAreCompatible returns true if two licenses are in the same license group (e.g. all "GPL" licenses are in the same
+// license group); otherwise, false.
 func (nodes *nodePair) rangesAreCompatible() bool {
-	if nodes.licensesExactlyEqual() {
-		// licenses specify ranges exactly the same (e.g. Apache-1.0+, Apache-1.0+)
-		return true
-	}
-
 	firstNode := *nodes.firstNode
 	secondNode := *nodes.secondNode
 
@@ -238,7 +256,7 @@ func (nodes *nodePair) rangesAreCompatible() bool {
 	// When both licenses allow later versions (i.e. hasPlus==true), being in the same license
 	// group is sufficient for compatibility, as long as, any exception is also compatible
 	// Example: All Apache licenses (e.g. Apache-1.0, Apache-2.0) are in the same license group
-	return sameLicenseGroup(firstRange, secondRange) && nodes.exceptionsAreCompatible()
+	return sameLicenseGroup(firstRange, secondRange)
 }
 
 // identifierInRange returns true if the (first) simple license is in range of the (second)
@@ -247,14 +265,7 @@ func (nodes *nodePair) identifierInRange() bool {
 	simpleLicense := nodes.firstNode
 	plusLicense := nodes.secondNode
 
-	if !compareGT(simpleLicense, plusLicense) && !compareEQ(simpleLicense, plusLicense) {
-		return false
-	}
-
-	// With simpleLicense >= plusLicense, licenses are compatible, as long as, any exception
-	// is also compatible
-	return nodes.exceptionsAreCompatible()
-
+	return compareGT(simpleLicense, plusLicense) || compareEQ(simpleLicense, plusLicense)
 }
 
 // exceptionsAreCompatible returns true if neither license has an exception or they have
@@ -274,10 +285,15 @@ func (nodes *nodePair) exceptionsAreCompatible() bool {
 	}
 
 	return *nodes.firstNode.exception() == *nodes.secondNode.exception()
-
 }
 
-// Return true if the licenses are the same; otherwise, false
+// rangesEqual returns true if the licenses are in the same range; otherwise, false
+// (e.g. GPL-2.0-only == GPL-2.0)
+func (nodes *nodePair) rangesEqual() bool {
+	return compareEQ(nodes.firstNode, nodes.secondNode)
+}
+
+// licensesExactlyEqual returns true if the licenses are the same; otherwise, false
 func (nodes *nodePair) licensesExactlyEqual() bool {
 	return strings.EqualFold(*nodes.firstNode.reconstructedLicenseString(), *nodes.secondNode.reconstructedLicenseString())
 }

spdxexp/parse_test.go

@@ -919,7 +919,7 @@ func TestParse(t *testing.T) {
 			require.Equal(t, test.err, err)
 			if test.err != nil {
 				// when error, check that returned node is nil
-				var nilNode *node = nil
+				var nilNode *node
 				assert.Equal(t, nilNode, startNode, "Expected nil node when error occurs.")
 				return
 			}
@@ -1115,7 +1115,7 @@ func TestParseTokens(t *testing.T) {
 			require.Equal(t, test.err, test.tokens.err)
 			if test.err != nil {
 				// when error, check that returned node is nil
-				var nilNode *node = nil
+				var nilNode *node
 				assert.Equal(t, nilNode, startNode, "Expected nil node when error occurs.")
 				return
 			}
@@ -1257,7 +1257,7 @@ func TestParseWith(t *testing.T) {
 			require.Equal(t, test.err, test.tokens.err)
 			if test.expectNil {
 				// exception license is nil when error occurs or WITH operator is not found
-				var nilString *string = nil
+				var nilString *string
 				assert.Equal(t, nilString, exceptionLicense)
 				return
 			}

spdxexp/satisfies_test.go

@@ -18,6 +18,14 @@ func TestValidateLicenses(t *testing.T) {
 		{"All invalid", []string{"MTI", "Apche-2.0", "0xDEADBEEF", ""}, false, []string{"MTI", "Apche-2.0", "0xDEADBEEF", ""}},
 		{"All valid", []string{"MIT", "Apache-2.0", "GPL-2.0"}, true, []string{}},
 		{"Some invalid", []string{"MTI", "Apche-2.0", "GPL-2.0"}, false, []string{"MTI", "Apche-2.0"}},
+		{"GPL-2.0", []string{"GPL-2.0"}, true, []string{}},
+		{"GPL-2.0-only", []string{"GPL-2.0-only"}, true, []string{}},
+		{"SPDX Expressions are valid", []string{
+			"MIT AND APACHE-2.0",
+			"MIT AND APCHE-2.0",
+			"LGPL-2.1-only OR MIT OR BSD-3-Clause",
+			"GPL-2.0-or-later WITH Bison-exception-2.2",
+		}, false, []string{"MIT AND APCHE-2.0"}},
 	}
 
 	for _, test := range tests {
@@ -110,6 +118,8 @@ func TestSatisfies(t *testing.T) {
 		{"GPL-1.0+ satisfies [GPL-2.0+]", "GPL-1.0+", []string{"GPL-2.0+"}, true, nil},
 		{"! GPL-1.0 satisfies [GPL-2.0+]", "GPL-1.0", []string{"GPL-2.0+"}, false, nil},
 		{"GPL-2.0-only satisfies [GPL-2.0-only]", "GPL-2.0-only", []string{"GPL-2.0-only"}, true, nil},
+		{"GPL-2.0 satisfies [GPL-2.0-only]", "GPL-2.0", []string{"GPL-2.0-only"}, true, nil},
+		{"GPL-2.0 AND GPL-2.0-only satisfies [GPL-2.0-only]", "GPL-2.0 AND GPL-2.0-only", []string{"GPL-2.0-only"}, true, nil},
 		{"GPL-3.0-only satisfies [GPL-2.0+]", "GPL-3.0-only", []string{"GPL-2.0+"}, true, nil},
 
 		{"! GPL-2.0 satisfies [GPL-2.0+ WITH Bison-exception-2.2]",

spdxexp/scan.go

@@ -269,10 +269,8 @@ func (exp *expressionStream) normalizeLicense(license string) *token {
 			return token
 		}
 	}
-	if token := deprecatedLicenseLookup(license); token != nil {
-		return token
-	}
-	return nil
+
+	return deprecatedLicenseLookup(license)
 }
 
 // Lookup license identifier in active and exception lists to determine if it is a supported SPDX id

spdxexp/scan_test.go

@@ -127,7 +127,7 @@ func TestParseToken(t *testing.T) {
 			require.Equal(t, test.err, test.exp.err)
 			if test.err != nil {
 				// token is nil when error occurs or token is not recognized
-				var nilToken *token = nil
+				var nilToken *token
 				assert.Equal(t, nilToken, tokn)
 				return
 			}
@@ -298,7 +298,7 @@ func TestReadDocumentRef(t *testing.T) {
 			require.Equal(t, test.err, test.exp.err)
 			if test.err != nil {
 				// ref should be nil when error occurs or a ref is not found
-				var nilToken *token = nil
+				var nilToken *token
 				assert.Equal(t, nilToken, ref, "Expected nil token when error occurs.")
 				return
 			}
@@ -331,7 +331,7 @@ func TestReadLicenseRef(t *testing.T) {
 			require.Equal(t, test.err, test.exp.err)
 			if test.err != nil {
 				// ref should be nil when error occurs or a ref is not found
-				var nilToken *token = nil
+				var nilToken *token
 				assert.Equal(t, nilToken, ref)
 				return
 			}
@@ -371,7 +371,7 @@ func TestReadLicense(t *testing.T) {
 			require.Equal(t, test.err, test.exp.err)
 			if test.err != nil {
 				// license should be nil when error occurs or a license is not found
-				var nilToken *token = nil
+				var nilToken *token
 				assert.Equal(t, nilToken, license)
 				return
 			}