Skip to content

Commit

Permalink
Merge pull request #14 from github/elr/plus
Browse files Browse the repository at this point in the history 
support for multi-version compatibility for licenses
  • Loading branch information
@elrayle
elrayle authored Sep 14, 2022
2 parents 6b5aa11 + 271c9f4 commit 96f5a7c
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 84 deletions.
9 changes: 9 additions & 0 deletions spdxexp/compare.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package spdxexp

func compareGT(first *node, second *node) bool {
if !first.isLicense() || !second.isLicense() {
return false
}
firstRange := getLicenseRange(*first.license())
secondRange := getLicenseRange(*second.license())

Expand All @@ -11,6 +14,9 @@ func compareGT(first *node, second *node) bool {
}

func compareLT(first *node, second *node) bool {
if !first.isLicense() || !second.isLicense() {
return false
}
firstRange := getLicenseRange(*first.license())
secondRange := getLicenseRange(*second.license())

Expand All @@ -21,6 +27,9 @@ func compareLT(first *node, second *node) bool {
}

func compareEQ(first *node, second *node) bool {
if !first.isLicense() || !second.isLicense() {
return false
}
firstRange := getLicenseRange(*first.license())
secondRange := getLicenseRange(*second.license())

Expand Down
8 changes: 4 additions & 4 deletions spdxexp/compare_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ func TestCompareGT(t *testing.T) {
{"expect greater than: LPPL-1.3a > LPPL-1.0", getLicenseNode("LPPL-1.3a", false), getLicenseNode("LPPL-1.0", false), true},
{"expect greater than: LPPL-1.3c > LPPL-1.3a", getLicenseNode("LPPL-1.3c", false), getLicenseNode("LPPL-1.3a", false), true},
{"expect greater than: AGPL-3.0 > AGPL-1.0", getLicenseNode("AGPL-3.0", false), getLicenseNode("AGPL-1.0", false), true},
{"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), true}, // TODO: Double check that -or-later and -only should be true for GT
{"expect greater than: 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-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-3.0 > GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", 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},
Expand Down Expand Up @@ -47,8 +47,8 @@ func TestCompareEQ(t *testing.T) {
{"expect greater than: LPPL-1.3a == LPPL-1.0", getLicenseNode("LPPL-1.3a", false), getLicenseNode("LPPL-1.0", false), false},
{"expect greater than: LPPL-1.3c == LPPL-1.3a", getLicenseNode("LPPL-1.3c", false), getLicenseNode("LPPL-1.3a", false), false},
{"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-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-3.0 == GPL-3.0", getLicenseNode("GPL-3.0", false), getLicenseNode("GPL-3.0", 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},
Expand Down
10 changes: 9 additions & 1 deletion spdxexp/license.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ type licenseRange struct {

// getLicenseRange returns a range of licenses from licenseRanges
func getLicenseRange(id string) *licenseRange {
simpleID := simplifyLicense(id)
allRanges := licenseRanges()
for i, licenseGrp := range allRanges {
for j, versionGrp := range licenseGrp {
for k, license := range versionGrp {
if id == license {
if simpleID == license {
location := map[uint8]int{
licenseGroup: i,
versionGroup: j,
Expand All @@ -63,6 +64,13 @@ func getLicenseRange(id string) *licenseRange {
return nil
}

func simplifyLicense(id string) string {
if strings.HasSuffix(id, "-or-later") {
return id[0 : len(id)-9]
}
return id
}

func getLicenses() []string {
return []string{
"0BSD",
Expand Down
59 changes: 39 additions & 20 deletions spdxexp/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,37 +212,56 @@ func (nodes *nodePair) licensesAreCompatible() bool {
// Return true if two licenses are compatible in the context of their ranges; otherwise, false.
func (nodes *nodePair) rangesAreCompatible() bool {
if nodes.licensesExactlyEqual() {
// licenses specify ranges exactly the same
// licenses specify ranges exactly the same (e.g. Apache-1.0+, Apache-1.0+)
return true
}

firstLicense := *nodes.firstNode.license()
secondLicense := *nodes.secondNode.license()
firstNode := *nodes.firstNode
secondNode := *nodes.secondNode

firstLicenseRange := getLicenseRange(firstLicense)
secondLicenseRange := getLicenseRange(secondLicense)
firstRange := getLicenseRange(*firstNode.license())
secondRange := getLicenseRange(*secondNode.license())

return licenseInRange(firstLicense, secondLicenseRange.licenses) &&
licenseInRange(secondLicense, firstLicenseRange.licenses)
// 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 true if license is found in licenseRange; otherwise, false
func licenseInRange(simpleLicense string, licenseRange []string) bool {
for _, testLicense := range licenseRange {
if simpleLicense == testLicense {
return true
}
}
return false
}

// Return true if the (first) simple license is in range of the (second) ranged license; otherwise, false.
// identifierInRange returns true if the (first) simple license is in range of the (second)
// ranged license; otherwise, false.
func (nodes *nodePair) identifierInRange() bool {
simpleLicense := nodes.firstNode
plusLicense := nodes.secondNode

return compareGT(simpleLicense, plusLicense) ||
compareEQ(simpleLicense, plusLicense)
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()

}

// exceptionsAreCompatible returns true if neither license has an exception or they have
// the same exception; otherwise, false
func (nodes *nodePair) exceptionsAreCompatible() bool {
firstNode := *nodes.firstNode
secondNode := *nodes.secondNode

if !firstNode.hasException() && !secondNode.hasException() {
// if neither has an exception, then licenses are compatible
return true
}

if firstNode.hasException() != secondNode.hasException() {
// if one has and exception and the other does not, then the license are NOT compatible
return false
}

return *nodes.firstNode.exception() == *nodes.secondNode.exception()

}

// Return true if the licenses are the same; otherwise, false
Expand Down
94 changes: 42 additions & 52 deletions spdxexp/node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,24 +68,40 @@ func TestLicensesAreCompatible(t *testing.T) {
{"compatible (diff case equal): Apache-2.0, APACHE-2.0", &nodePair{
getLicenseNode("Apache-2.0", false),
getLicenseNode("APACHE-2.0", false)}, true},
// {"compatible (same version with +): Apache-1.0+, Apache-1.0", &nodePair{
// getLicenseNode("Apache-1.0+", true),
// getLicenseNode("Apache-1.0", false)}, true},
// {"compatible (later version with +): Apache-1.0+, Apache-2.0", &nodePair{
// getLicenseNode("Apache-1.0+", true),
// getLicenseNode("Apache-2.0", false)}, true},
// {"compatible (same version with -or-later): GPL-2.0-or-later, GPL-2.0", &nodePair{
// getLicenseNode("GPL-2.0-or-later", true),
// getLicenseNode("GPL-2.0", false)}, true},
// {"compatible (same version with -or-later and -only): GPL-2.0-or-later, GPL-2.0-only", &nodePair{
// getLicenseNode("GPL-2.0-or-later", true),
// getLicenseNode("GPL-2.0-only", false)}, true}, // TODO: Double check that -or-later and -only should be true for GT
// {"compatible (later version with -or-later): GPL-2.0-or-later, GPL-3.0", &nodePair{
// getLicenseNode("GPL-2.0-or-later", true),
// getLicenseNode("GPL-3.0", false)}, true},
// {"incompatible (different versions using -only): GPL-3.0-only, GPL-2.0-only", &nodePair{
// getLicenseNode("GPL-3.0-only", false),
// getLicenseNode("GPL-2.0-only", false)}, false},
{"compatible (same version with +): Apache-1.0+, Apache-1.0", &nodePair{
getLicenseNode("Apache-1.0", true),
getLicenseNode("Apache-1.0", false)}, true},
{"compatible (later version with +): Apache-1.0+, Apache-2.0", &nodePair{
getLicenseNode("Apache-1.0", true),
getLicenseNode("Apache-2.0", false)}, true},
{"compatible (second version with +): Apache-2.0, Apache-1.0+", &nodePair{
getLicenseNode("Apache-2.0", false),
getLicenseNode("Apache-1.0", true)}, true},
{"compatible (later version with both +): Apache-1.0+, Apache-2.0+", &nodePair{
getLicenseNode("Apache-1.0", true),
getLicenseNode("Apache-2.0", true)}, true},
{"compatible (same version with -or-later): GPL-2.0-or-later, GPL-2.0", &nodePair{
getLicenseNode("GPL-2.0-or-later", true),
getLicenseNode("GPL-2.0", false)}, true},
{"compatible (same version with -or-later and -only): GPL-2.0-or-later, GPL-2.0-only", &nodePair{
getLicenseNode("GPL-2.0-or-later", true),
getLicenseNode("GPL-2.0-only", false)}, true}, // TODO: Double check that -or-later and -only should be true for GT
{"compatible (later version with -or-later): GPL-2.0-or-later, GPL-3.0", &nodePair{
getLicenseNode("GPL-2.0-or-later", true),
getLicenseNode("GPL-3.0", false)}, true},
{"incompatible (same version with -or-later exception): GPL-2.0, GPL-2.0-or-later WITH Bison-exception-2.2", &nodePair{
getLicenseNode("GPL-2.0", true),
&node{
role: licenseNode,
exp: nil,
lic: &licenseNodePartial{
license: "GPL-2.0", hasPlus: true,
hasException: true, exception: "Bison-exception-2.2"},
ref: nil,
}}, false},
{"incompatible (different versions using -only): GPL-3.0-only, GPL-2.0-only", &nodePair{
getLicenseNode("GPL-3.0-only", false),
getLicenseNode("GPL-2.0-only", false)}, false},
{"incompatible (different versions with letter): LPPL-1.3c, LPPL-1.3a", &nodePair{
getLicenseNode("LPPL-1.3c", false),
getLicenseNode("LPPL-1.3a", false)}, false},
Expand All @@ -99,8 +115,8 @@ func TestLicensesAreCompatible(t *testing.T) {
getLicenseNode("MIT", false),
getLicenseNode("ISC", false)}, false},
{"not simple license: (MIT OR ISC), GPL-3.0", &nodePair{
getLicenseNode("(MIT OR ISC)", false),
getLicenseNode("GPL-3.0", false)}, false}, // TODO: should it raise error?
getParsedNode("(MIT OR ISC)"),
getLicenseNode("GPL-3.0", false)}, false},
}

for _, test := range tests {
Expand All @@ -120,9 +136,9 @@ func TestRangesAreCompatible(t *testing.T) {
{"compatible - both use -or-later", &nodePair{
firstNode: getLicenseNode("GPL-1.0-or-later", true),
secondNode: getLicenseNode("GPL-2.0-or-later", true)}, true},
// {"compatible - both use +", &nodePair{ // TODO: fails here and in js, but passes js satisfies
// firstNode: getLicenseNode("Apache-1.0", true),
// secondNode: getLicenseNode("Apache-2.0", true)}, true},
{"compatible - both use +", &nodePair{
firstNode: getLicenseNode("Apache-1.0", true),
secondNode: getLicenseNode("Apache-2.0", true)}, true},
{"not compatible", &nodePair{
firstNode: getLicenseNode("GPL-1.0-or-later", true),
secondNode: getLicenseNode("LGPL-3.0-or-later", true)}, false},
Expand All @@ -136,32 +152,6 @@ func TestRangesAreCompatible(t *testing.T) {
}
}

func TestLicenseInRange(t *testing.T) {
tests := []struct {
name string
license string
licenseRange []string
result bool
}{
{"in range", "GPL-3.0", []string{
"GPL-1.0-or-later",
"GPL-2.0-or-later",
"GPL-3.0",
"GPL-3.0-only",
"GPL-3.0-or-later"}, true},
{"not in range", "GPL-3.0", []string{
"GPL-2.0",
"GPL-2.0-only"}, false},
}

for _, test := range tests {
test := test
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.result, licenseInRange(test.license, test.licenseRange))
})
}
}

func TestIdentifierInRange(t *testing.T) {
tests := []struct {
name string
Expand All @@ -173,10 +163,10 @@ func TestIdentifierInRange(t *testing.T) {
secondNode: getLicenseNode("GPL-2.0-or-later", true)}, true},
{"in or-later range (same)", &nodePair{
firstNode: getLicenseNode("GPL-2.0", false),
secondNode: getLicenseNode("GPL-2.0-or-later", true)}, false}, // TODO: why doesn't this
{"in + range", &nodePair{
secondNode: getLicenseNode("GPL-2.0-or-later", true)}, true},
{"in + range (1.0+)", &nodePair{
firstNode: getLicenseNode("Apache-2.0", false),
secondNode: getLicenseNode("Apache-1.0+", true)}, false}, // TODO: think this doesn't match because Apache doesn't have any -or-later
secondNode: getLicenseNode("Apache-1.0", true)}, true},
{"not in range", &nodePair{
firstNode: getLicenseNode("GPL-1.0", false),
secondNode: getLicenseNode("GPL-2.0-or-later", true)}, false},
Expand Down
13 changes: 7 additions & 6 deletions spdxexp/satisfies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,19 @@ func TestSatisfies(t *testing.T) {

{"MIT AND Apache-2.0 satisfies [MIT, Apache-1.0, Apache-2.0]", "MIT AND Apache-2.0", []string{"MIT", "Apache-1.0", "Apache-2.0"}, true, nil},

// {"Apache-1.0+ satisfies [Apache-2.0+]", "Apache-1.0+", []string{"Apache-2.0+"}, true, nil}, // TODO: Fails here but passes js
{"Apache-1.0+ satisfies [Apache-2.0]", "Apache-1.0+", []string{"Apache-2.0"}, true, nil},
{"Apache-1.0+ satisfies [Apache-2.0+]", "Apache-1.0+", []string{"Apache-2.0+"}, true, nil}, // TODO: Fails here but passes js
{"! Apache-1.0 satisfies [Apache-2.0+]", "Apache-1.0", []string{"Apache-2.0+"}, false, nil},
{"Apache-2.0 satisfies [Apache-2.0+]", "Apache-2.0", []string{"Apache-2.0+"}, true, nil},
// {"Apache-3.0 satisfies [Apache-2.0+]", "Apache-3.0", []string{"Apache-2.0+"}, true, nil}, // TODO: gets error b/c Apache-3.0 doesn't exist -- need better error message
{"! Apache-3.0 satisfies [Apache-2.0+]", "Apache-3.0", []string{"Apache-2.0+"}, false, errors.New("unknown license 'Apache-3.0' at offset 0")},

{"! Apache-1.0 satisfies [Apache-2.0-or-later]", "Apache-1.0", []string{"Apache-2.0-or-later"}, false, nil},
{"Apache-2.0 satisfies [Apache-2.0-or-later]", "Apache-2.0", []string{"Apache-2.0-or-later"}, true, nil},
// {"Apache-3.0 satisfies [Apache-2.0-or-later]", "Apache-3.0", []string{"Apache-2.0-or-later"}, true, nil},
{"! Apache-3.0 satisfies [Apache-2.0-or-later]", "Apache-3.0", []string{"Apache-2.0-or-later"}, false, errors.New("unknown license 'Apache-3.0' at offset 0")},

{"! Apache-1.0 satisfies [Apache-2.0-only]", "Apache-1.0", []string{"Apache-2.0-only"}, false, nil},
{"Apache-2.0 satisfies [Apache-2.0-only]", "Apache-2.0", []string{"Apache-2.0-only"}, true, nil},
// {"Apache-3.0 satisfies [Apache-2.0-only]", "Apache-3.0", []string{"Apache-2.0-only"}, false, nil},
{"! Apache-3.0 satisfies [Apache-2.0-only]", "Apache-3.0", []string{"Apache-2.0-only"}, false, errors.New("unknown license 'Apache-3.0' at offset 0")},

// regression tests from spdx-satisfies.js - assert statements in README
// TODO: Commented out tests are not yet supported.
Expand All @@ -63,8 +64,8 @@ func TestSatisfies(t *testing.T) {
{"MIT satisfies [ISC, MIT]", "MIT", []string{"ISC", "MIT"}, true, nil},
{"Zlib satisfies [ISC, MIT, Zlib]", "Zlib", []string{"ISC", "MIT", "Zlib"}, true, nil},
{"! GPL-3.0 satisfies [ISC, MIT]", "GPL-3.0", []string{"ISC", "MIT"}, false, nil},
// {"GPL-2.0 satisfies [GPL-2.0+]", "GPL-2.0", []string{"GPL-2.0+"}, true, nil}, // TODO: Fails here but passes js
// {"GPL-2.0 satisfies [GPL-2.0-or-later]", "GPL-2.0", []string{"GPL-2.0-or-later"}, true, nil}, // TODO: Fails here and js
{"GPL-2.0 satisfies [GPL-2.0+]", "GPL-2.0", []string{"GPL-2.0+"}, true, nil}, // TODO: Fails here but passes js
{"GPL-2.0 satisfies [GPL-2.0-or-later]", "GPL-2.0", []string{"GPL-2.0-or-later"}, true, nil}, // TODO: Fails here and js
{"GPL-3.0 satisfies [GPL-2.0+]", "GPL-3.0", []string{"GPL-2.0+"}, true, nil},
{"GPL-1.0-or-later satisfies [GPL-2.0-or-later]", "GPL-1.0-or-later", []string{"GPL-2.0-or-later"}, true, nil},
{"GPL-1.0+ satisfies [GPL-2.0+]", "GPL-1.0+", []string{"GPL-2.0+"}, true, nil},
Expand Down
2 changes: 1 addition & 1 deletion spdxexp/scan.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func (exp *expressionStream) normalizeLicense(license string) *token {
if token := licenseLookup(license); token != nil {
// checks active and exception license lists
// deprecated list is checked at the end to avoid a deprecated license being used for +
// (example: GPL-1.0 is on the depcated list, but GPL-1.0+ should become GPL-1.0-or-later)
// (example: GPL-1.0 is on the deprecated list, but GPL-1.0+ should become GPL-1.0-or-later)
return token
}

Expand Down
12 changes: 12 additions & 0 deletions spdxexp/test_helper.go
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package spdxexp

// getLicenseNode is a test helper method that is expected to create a valid
// license node. Use this function when the test data is known to be a valid
// license that would parse successfully.
func getLicenseNode(license string, hasPlus bool) *node {
return &node{
role: licenseNode,
Expand All @@ -13,3 +16,12 @@ func getLicenseNode(license string, hasPlus bool) *node {
ref: nil,
}
}

// getParsedNode is a test helper method that is expected to create a valid node
// and swallow errors. This allows test structures to use parsed node data.
// Use this function when the test data is expected to parse successfully.
func getParsedNode(expression string) *node {
// swallows errors
n, _ := parse(expression)
return n
}

0 comments on commit 96f5a7c

Please sign in to comment.