From 450807412c0c6811e4849f1f46ec8040e6536f55 Mon Sep 17 00:00:00 2001 From: "E. Lynette Rayle" Date: Tue, 12 May 2026 13:33:41 -0400 Subject: [PATCH 1/5] add function to reconstruct expressions --- spdxexp/node.go | 66 ++++++++++++++++++++++++++++++++++++++++++++ spdxexp/node_test.go | 4 +++ 2 files changed, 70 insertions(+) diff --git a/spdxexp/node.go b/spdxexp/node.go index 2479515..6fe4db2 100644 --- a/spdxexp/node.go +++ b/spdxexp/node.go @@ -1,6 +1,7 @@ package spdxexp import ( + "fmt" "sort" "strings" ) @@ -148,6 +149,8 @@ func (n *node) hasDocumentRef() bool { // TODO: Original had "NOASSERTION". Does that still apply? func (n *node) reconstructedLicenseString() *string { switch n.role { + case expressionNode: + return n.reconstructedExpressionString() case licenseNode: license := *n.license() if n.hasPlus() && !strings.HasSuffix(strings.ToLower(license), "-or-later") { @@ -167,6 +170,69 @@ func (n *node) reconstructedLicenseString() *string { return nil } +func (n *node) reconstructedExpressionString() *string { + if n == nil || !n.isExpression() { + return nil + } + + left := n.left() + right := n.right() + if left == nil || right == nil { + return nil + } + + leftStr := left.reconstructedLicenseString() + rightStr := right.reconstructedLicenseString() + if leftStr == nil || rightStr == nil { + return nil + } + + conj := n.conjunction() + if conj == nil { + return nil + } + + operator := strings.ToUpper(*conj) + if operator != "AND" && operator != "OR" { + return nil + } + + parentPrec := nodePrecedence(n) + leftRendered := *leftStr + if left.isExpression() && nodePrecedence(left) < parentPrec { + leftRendered = "(" + leftRendered + ")" + } + rightRendered := *rightStr + if right.isExpression() && nodePrecedence(right) < parentPrec { + rightRendered = "(" + rightRendered + ")" + } + + s := fmt.Sprintf("%s %s %s", leftRendered, operator, rightRendered) + return &s +} + +func nodePrecedence(n *node) int { + if n == nil { + return 0 + } + if !n.isExpression() { + // atomic (license/licenseRef) + return 3 + } + conj := n.conjunction() + if conj == nil { + return 0 + } + switch strings.ToLower(*conj) { + case "and": + return 2 + case "or": + return 1 + default: + return 0 + } +} + // 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. diff --git a/spdxexp/node_test.go b/spdxexp/node_test.go index 4cabd47..9b589a1 100644 --- a/spdxexp/node_test.go +++ b/spdxexp/node_test.go @@ -45,6 +45,10 @@ func TestReconstructedLicenseString(t *testing.T) { licenseRef: "MIT-Style-2", }, }, "DocumentRef-spdx-tool-1.2:LicenseRef-MIT-Style-2"}, + {"Expression node - AND", getParsedNode("MIT AND Apache-2.0"), "MIT AND Apache-2.0"}, + {"Expression node - parentheses required (OR under AND)", getParsedNode("(MIT OR Apache-2.0) AND BSD-3-Clause"), "(MIT OR Apache-2.0) AND BSD-3-Clause"}, + {"Expression node - parentheses required (OR on right)", getParsedNode("MIT AND (Apache-2.0 OR BSD-3-Clause)"), "MIT AND (Apache-2.0 OR BSD-3-Clause)"}, + {"Expression node - precedence (AND under OR)", getParsedNode("MIT OR Apache-2.0 AND BSD-3-Clause"), "MIT OR Apache-2.0 AND BSD-3-Clause"}, } for _, test := range tests { From 43cb8935012c1740bc2b94c04aae909cb28e1581 Mon Sep 17 00:00:00 2001 From: "E. Lynette Rayle" Date: Tue, 12 May 2026 14:40:41 -0400 Subject: [PATCH 2/5] Add ability to get normalized licenses when validating --- spdxexp/satisfies.go | 74 +++++++++---- spdxexp/satisfies_test.go | 216 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 269 insertions(+), 21 deletions(-) diff --git a/spdxexp/satisfies.go b/spdxexp/satisfies.go index 5ad0d60..1a0c69a 100644 --- a/spdxexp/satisfies.go +++ b/spdxexp/satisfies.go @@ -34,8 +34,36 @@ type ValidateLicensesOptions struct { // Returns all the invalid licenses contained in the `licenses` argument. func ValidateLicensesWithOptions(licenses []string, options ValidateLicensesOptions) (bool, []string) { // handle all other cases with parsing, which will cover both single and multiple licenses and expressions - valid := true - invalidLicenses := []string{} + _, invalidLicenses := ValidateAndNormalizeLicensesWithOptions(licenses, options) + return len(invalidLicenses) == 0, invalidLicenses +} + +// ValidateAndNormalizeLicensesWithOptions checks if given licenses are valid according to SPDX. +// Supports validation options as defined in ValidateLicensesOptions. +// Returns all validated licenses in their normalized form as the first return value. +// Returns any invalid licenses as the second return value. +func ValidateAndNormalizeLicensesWithOptions(licenses []string, options ValidateLicensesOptions) (normalizedLicenses, invalidLicenses []string) { + normalizedLicenses = []string{} + invalidLicenses = []string{} + seenNormalized := make(map[string]struct{}, len(licenses)) + seenInvalid := make(map[string]struct{}, len(licenses)) + + addNormalized := func(license string) { + if _, ok := seenNormalized[license]; ok { + return + } + seenNormalized[license] = struct{}{} + normalizedLicenses = append(normalizedLicenses, license) + } + + addInvalid := func(license string) { + if _, ok := seenInvalid[license]; ok { + return + } + seenInvalid[license] = struct{}{} + invalidLicenses = append(invalidLicenses, license) + } + for _, license := range licenses { // MIT is the most common license, so check for it first before doing any processing to optimize for this case. // By putting the isMIT check here, we can avoid the overhead of parsing for the most common case of MIT. @@ -43,6 +71,7 @@ func ValidateLicensesWithOptions(licenses []string, options ValidateLicensesOpti // as MIT by isMIT, but will still be correctly identified using activeLicense. As this is uncommon, it // is an acceptable tradeoff to avoid the overhead of trimming for the more common case. if isMIT(license) { + addNormalized("MIT") continue } @@ -50,32 +79,31 @@ func ValidateLicensesWithOptions(licenses []string, options ValidateLicensesOpti isAtomic := isAtomicLicense(license) if isAtomic { - if ok, _ := activeLicense(license); ok { + if ok, normalizedLicense := activeLicense(license); ok { + addNormalized(normalizedLicense) continue } - if ok, _ := deprecatedLicense(license); ok { + if ok, normalizedLicense := deprecatedLicense(license); ok { if options.FailDeprecatedLicenses { - valid = false - invalidLicenses = append(invalidLicenses, license) + addInvalid(license) continue } + addNormalized(normalizedLicense) // if FailDeprecatedLicenses is false, then consider the deprecated license valid and continue continue } if options.FailAllLicenseRefs { if strings.HasPrefix(license, "LicenseRef-") { - valid = false - invalidLicenses = append(invalidLicenses, license) + addInvalid(license) continue } } if options.FailAllDocumentRefs { if strings.HasPrefix(license, "DocumentRef-") { - valid = false - invalidLicenses = append(invalidLicenses, license) + addInvalid(license) continue } } @@ -86,18 +114,19 @@ func ValidateLicensesWithOptions(licenses []string, options ValidateLicensesOpti if !isAtomic { if hasException, licensePart, exceptionPart := isLicenseWithException(license); hasException { // matches pattern "licensePart WITH exceptionPart", so validate both parts separately - if ok, _ := exceptionLicense(exceptionPart); ok { - if ok, _ := activeLicense(licensePart); ok { + if ok, normalizedException := exceptionLicense(exceptionPart); ok { + if ok, normalizedLicense := activeLicense(licensePart); ok { + addNormalized(normalizedLicense + " WITH " + normalizedException) continue } if !options.FailDeprecatedLicenses { - if ok, _ := deprecatedLicense(licensePart); ok { + if ok, normalizedLicense := deprecatedLicense(licensePart); ok { + addNormalized(normalizedLicense + " WITH " + normalizedException) continue } } } - valid = false - invalidLicenses = append(invalidLicenses, license) + addInvalid(license) continue } } @@ -105,19 +134,22 @@ func ValidateLicensesWithOptions(licenses []string, options ValidateLicensesOpti // all other non-atomic expressions are complex expressions with conjunctions (e.g. "MIT AND Apache-2.0"), // so fail if complex expressions are not allowed if options.FailComplexExpressions && !isAtomic { - valid = false - invalidLicenses = append(invalidLicenses, license) + addInvalid(license) continue } // need to parse if allowing any of LicenseRef, DocumentRef, or complex expressions to be able to determine // whether the license expression is valid - if _, err := parse(license); err != nil { - valid = false - invalidLicenses = append(invalidLicenses, license) + var parsedLicense *node + var err error + if parsedLicense, err = parse(license); err != nil { + addInvalid(license) + } else { + normalizedLicense := *parsedLicense.reconstructedLicenseString() + addNormalized(normalizedLicense) } } - return valid, invalidLicenses + return normalizedLicenses, invalidLicenses } // Satisfies determines if the allowed list of licenses satisfies the test license expression. diff --git a/spdxexp/satisfies_test.go b/spdxexp/satisfies_test.go index cae9fce..7f69ade 100644 --- a/spdxexp/satisfies_test.go +++ b/spdxexp/satisfies_test.go @@ -228,6 +228,222 @@ func TestValidateLicensesWithOptions_AllOptions(t *testing.T) { } } +func TestValidateAndNormalizeLicensesWithOptions_FailComplexExpressions(t *testing.T) { + tests := []struct { + name string + inputLicenses []string + options ValidateLicensesOptions + allValid bool + invalidLicenses []string + normalizedLicenses []string + }{ + { + name: "Expressions rejected", + inputLicenses: []string{"MIT AND Apache-2.0"}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: false, + invalidLicenses: []string{ + "MIT AND Apache-2.0", + }, + normalizedLicenses: []string{}, + }, + { + name: "Mixed list rejects only expressions", + inputLicenses: []string{"mit", "apache-2.0", "LGPL-2.1-only OR MIT"}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: false, + invalidLicenses: []string{ + "LGPL-2.1-only OR MIT", + }, + normalizedLicenses: []string{"MIT", "Apache-2.0"}, + }, + { + name: "WITH exception is not treated as complex expression", + inputLicenses: []string{"gpl-2.0-or-later WITH Bison-exception-2.2"}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: true, + invalidLicenses: []string{}, + normalizedLicenses: []string{"GPL-2.0-or-later WITH Bison-exception-2.2"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + normalizedLicenses, invalidLicenses := ValidateAndNormalizeLicensesWithOptions(test.inputLicenses, test.options) + assert.EqualValues(t, test.invalidLicenses, invalidLicenses, "invalid licenses should match expected") + assert.EqualValues(t, test.normalizedLicenses, normalizedLicenses, "normalized licenses should match expected") + assert.Equal(t, test.allValid, len(invalidLicenses) == 0) + }) + } +} + +func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testing.T) { + // eCos-2.0 is a known deprecated SPDX license ID (see TestDeprecatedLicense). + deprecatedLicense := "eCos-2.0" + + tests := []struct { + name string + inputLicenses []string + options ValidateLicensesOptions + allValid bool + invalidLicenses []string + normalizedLicenses []string + }{ + { + name: "Deprecated license rejected", + inputLicenses: []string{deprecatedLicense}, + options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, + allValid: false, + invalidLicenses: []string{ + deprecatedLicense, + }, + normalizedLicenses: []string{}, + }, + { + name: "Mixed list rejects only deprecated licenses", + inputLicenses: []string{"MIT", "Apache-2.0", deprecatedLicense}, + options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, + allValid: false, + invalidLicenses: []string{ + deprecatedLicense, + }, + normalizedLicenses: []string{"MIT", "Apache-2.0"}, + }, + { + name: "WITH exception rejects deprecated license if FailDeprecatedLicenses is true", + inputLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, + options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, + allValid: false, + invalidLicenses: []string{ + deprecatedLicense + " WITH Bison-exception-2.2", + }, + normalizedLicenses: []string{}, + }, + { + name: "WITH exception allows deprecated license if FailDeprecatedLicenses is false", + inputLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, + options: ValidateLicensesOptions{FailDeprecatedLicenses: false}, + allValid: true, + invalidLicenses: []string{}, + normalizedLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + normalizedLicenses, invalidLicenses := ValidateAndNormalizeLicensesWithOptions(test.inputLicenses, test.options) + assert.EqualValues(t, test.invalidLicenses, invalidLicenses, "invalid licenses should match expected") + assert.EqualValues(t, test.normalizedLicenses, normalizedLicenses, "normalized licenses should match expected") + assert.Equal(t, test.allValid, len(invalidLicenses) == 0) + }) + } +} + +func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { + documentRef := "DocumentRef-spdx-tool-1.2:LicenseRef-MIT-Style-2" + licenseRef := "LicenseRef-MIT-Style-1" + deprecated := "eCos-2.0" + expression := "MIT AND Apache-2.0" + licenseWithException := "GPL-2.0-or-later WITH Bison-exception-2.2" + + tests := []struct { + name string + inputLicenses []string + options ValidateLicensesOptions + allValid bool + invalidLicenses []string + normalizedLicenses []string + }{ + { + name: "Dedups", + inputLicenses: []string{" MIT ", "MIT", "mit"}, + options: ValidateLicensesOptions{}, + allValid: true, + invalidLicenses: []string{}, + normalizedLicenses: []string{"MIT"}, + }, + { + name: "Defaults allow deprecated, refs, and expressions", + inputLicenses: []string{" MIT ", deprecated, "licenseref-mine-MyLicense", licenseRef, documentRef, expression, licenseWithException}, + options: ValidateLicensesOptions{}, + allValid: false, + invalidLicenses: []string{"licenseref-mine-MyLicense"}, + normalizedLicenses: []string{"MIT", deprecated, licenseRef, documentRef, expression, licenseWithException}, + }, + { + name: "FailDeprecatedLicenses rejects deprecated IDs", + inputLicenses: []string{deprecated, "Apache-2.0"}, + options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, + allValid: false, + invalidLicenses: []string{deprecated}, + normalizedLicenses: []string{"Apache-2.0"}, + }, + { + name: "FailComplexExpressions rejects conjunction expressions", + inputLicenses: []string{expression, licenseWithException}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: false, + invalidLicenses: []string{expression}, + normalizedLicenses: []string{licenseWithException}, + }, + { + name: "FailComplexExpressions does not duplicate invalid entries", + inputLicenses: []string{"MIT AND APCHE-2.0"}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: false, + invalidLicenses: []string{"MIT AND APCHE-2.0"}, + normalizedLicenses: []string{}, + }, + { + name: "FailAllLicenseRefs rejects LicenseRef but allows DocumentRef", + inputLicenses: []string{licenseRef, documentRef}, + options: ValidateLicensesOptions{FailAllLicenseRefs: true}, + allValid: false, + invalidLicenses: []string{licenseRef}, + normalizedLicenses: []string{documentRef}, + }, + { + name: "FailAllDocumentRefs rejects DocumentRef but allows LicenseRef", + inputLicenses: []string{documentRef, licenseRef}, + options: ValidateLicensesOptions{FailAllDocumentRefs: true}, + allValid: false, + invalidLicenses: []string{documentRef}, + normalizedLicenses: []string{licenseRef}, + }, + { + name: "FailAllLicenseRefs and FailAllDocumentRefs rejects any non-active atomic ref", + inputLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, + options: ValidateLicensesOptions{FailAllLicenseRefs: true, FailAllDocumentRefs: true}, + allValid: false, + invalidLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, + normalizedLicenses: []string{}, + }, + { + name: "All flags together", + inputLicenses: []string{deprecated, licenseRef, documentRef, expression, licenseWithException, "Apache-2.0"}, + options: ValidateLicensesOptions{ + FailComplexExpressions: true, + FailDeprecatedLicenses: true, + FailAllLicenseRefs: true, + FailAllDocumentRefs: true, + }, + allValid: false, + invalidLicenses: []string{deprecated, licenseRef, documentRef, expression}, + normalizedLicenses: []string{licenseWithException, "Apache-2.0"}, + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + normalizedLicenses, invalidLicenses := ValidateAndNormalizeLicensesWithOptions(test.inputLicenses, test.options) + assert.EqualValues(t, test.invalidLicenses, invalidLicenses, "invalid licenses should match expected") + assert.EqualValues(t, test.normalizedLicenses, normalizedLicenses, "normalized licenses should match expected") + assert.Equal(t, test.allValid, len(invalidLicenses) == 0) + }) + } +} + // TestSatisfiesSingle lets you quickly test a single call to Satisfies with a specific license expression and allowed list of licenses. // To test a different expression, change the expression, allowed licenses, and expected result in the function body. // TO RUN: go test ./expression -run TestSatisfiesSingle From 7c92c0741aa6fddc6d208ce41ce0fd9d4510c28b Mon Sep 17 00:00:00 2001 From: "E. Lynette Rayle" Date: Tue, 12 May 2026 15:27:40 -0400 Subject: [PATCH 3/5] fix formatting --- spdxexp/satisfies_test.go | 76 +++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/spdxexp/satisfies_test.go b/spdxexp/satisfies_test.go index 7f69ade..f021290 100644 --- a/spdxexp/satisfies_test.go +++ b/spdxexp/satisfies_test.go @@ -258,11 +258,11 @@ func TestValidateAndNormalizeLicensesWithOptions_FailComplexExpressions(t *testi normalizedLicenses: []string{"MIT", "Apache-2.0"}, }, { - name: "WITH exception is not treated as complex expression", - inputLicenses: []string{"gpl-2.0-or-later WITH Bison-exception-2.2"}, - options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: true, - invalidLicenses: []string{}, + name: "WITH exception is not treated as complex expression", + inputLicenses: []string{"gpl-2.0-or-later WITH Bison-exception-2.2"}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: true, + invalidLicenses: []string{}, normalizedLicenses: []string{"GPL-2.0-or-later WITH Bison-exception-2.2"}, }, } @@ -320,11 +320,11 @@ func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testi normalizedLicenses: []string{}, }, { - name: "WITH exception allows deprecated license if FailDeprecatedLicenses is false", - inputLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, - options: ValidateLicensesOptions{FailDeprecatedLicenses: false}, - allValid: true, - invalidLicenses: []string{}, + name: "WITH exception allows deprecated license if FailDeprecatedLicenses is false", + inputLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, + options: ValidateLicensesOptions{FailDeprecatedLicenses: false}, + allValid: true, + invalidLicenses: []string{}, normalizedLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, }, } @@ -379,47 +379,47 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { normalizedLicenses: []string{"Apache-2.0"}, }, { - name: "FailComplexExpressions rejects conjunction expressions", - inputLicenses: []string{expression, licenseWithException}, - options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: false, - invalidLicenses: []string{expression}, + name: "FailComplexExpressions rejects conjunction expressions", + inputLicenses: []string{expression, licenseWithException}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: false, + invalidLicenses: []string{expression}, normalizedLicenses: []string{licenseWithException}, }, { - name: "FailComplexExpressions does not duplicate invalid entries", - inputLicenses: []string{"MIT AND APCHE-2.0"}, - options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: false, - invalidLicenses: []string{"MIT AND APCHE-2.0"}, + name: "FailComplexExpressions does not duplicate invalid entries", + inputLicenses: []string{"MIT AND APCHE-2.0"}, + options: ValidateLicensesOptions{FailComplexExpressions: true}, + allValid: false, + invalidLicenses: []string{"MIT AND APCHE-2.0"}, normalizedLicenses: []string{}, }, { - name: "FailAllLicenseRefs rejects LicenseRef but allows DocumentRef", - inputLicenses: []string{licenseRef, documentRef}, - options: ValidateLicensesOptions{FailAllLicenseRefs: true}, - allValid: false, - invalidLicenses: []string{licenseRef}, + name: "FailAllLicenseRefs rejects LicenseRef but allows DocumentRef", + inputLicenses: []string{licenseRef, documentRef}, + options: ValidateLicensesOptions{FailAllLicenseRefs: true}, + allValid: false, + invalidLicenses: []string{licenseRef}, normalizedLicenses: []string{documentRef}, }, { - name: "FailAllDocumentRefs rejects DocumentRef but allows LicenseRef", - inputLicenses: []string{documentRef, licenseRef}, - options: ValidateLicensesOptions{FailAllDocumentRefs: true}, - allValid: false, - invalidLicenses: []string{documentRef}, + name: "FailAllDocumentRefs rejects DocumentRef but allows LicenseRef", + inputLicenses: []string{documentRef, licenseRef}, + options: ValidateLicensesOptions{FailAllDocumentRefs: true}, + allValid: false, + invalidLicenses: []string{documentRef}, normalizedLicenses: []string{licenseRef}, }, { - name: "FailAllLicenseRefs and FailAllDocumentRefs rejects any non-active atomic ref", - inputLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, - options: ValidateLicensesOptions{FailAllLicenseRefs: true, FailAllDocumentRefs: true}, - allValid: false, - invalidLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, + name: "FailAllLicenseRefs and FailAllDocumentRefs rejects any non-active atomic ref", + inputLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, + options: ValidateLicensesOptions{FailAllLicenseRefs: true, FailAllDocumentRefs: true}, + allValid: false, + invalidLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, normalizedLicenses: []string{}, }, { - name: "All flags together", + name: "All flags together", inputLicenses: []string{deprecated, licenseRef, documentRef, expression, licenseWithException, "Apache-2.0"}, options: ValidateLicensesOptions{ FailComplexExpressions: true, @@ -427,8 +427,8 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { FailAllLicenseRefs: true, FailAllDocumentRefs: true, }, - allValid: false, - invalidLicenses: []string{deprecated, licenseRef, documentRef, expression}, + allValid: false, + invalidLicenses: []string{deprecated, licenseRef, documentRef, expression}, normalizedLicenses: []string{licenseWithException, "Apache-2.0"}, }, } From 7d11f4cbb0244dafaf9bb4e910c163f6827fe5d6 Mon Sep 17 00:00:00 2001 From: "E. Lynette Rayle" Date: Tue, 12 May 2026 15:47:07 -0400 Subject: [PATCH 4/5] do not dedup invalid licenses as this represents a behavior change --- spdxexp/node.go | 2 +- spdxexp/satisfies.go | 21 ++++++--------------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/spdxexp/node.go b/spdxexp/node.go index 6fe4db2..388650c 100644 --- a/spdxexp/node.go +++ b/spdxexp/node.go @@ -145,7 +145,7 @@ func (n *node) hasDocumentRef() bool { return n.ref.hasDocumentRef } -// reconstructedLicenseString returns the string representation of the license or license ref. +// reconstructedLicenseString returns the string representation of a license, license ref, or expression. // TODO: Original had "NOASSERTION". Does that still apply? func (n *node) reconstructedLicenseString() *string { switch n.role { diff --git a/spdxexp/satisfies.go b/spdxexp/satisfies.go index 1a0c69a..179b0f8 100644 --- a/spdxexp/satisfies.go +++ b/spdxexp/satisfies.go @@ -46,7 +46,6 @@ func ValidateAndNormalizeLicensesWithOptions(licenses []string, options Validate normalizedLicenses = []string{} invalidLicenses = []string{} seenNormalized := make(map[string]struct{}, len(licenses)) - seenInvalid := make(map[string]struct{}, len(licenses)) addNormalized := func(license string) { if _, ok := seenNormalized[license]; ok { @@ -56,14 +55,6 @@ func ValidateAndNormalizeLicensesWithOptions(licenses []string, options Validate normalizedLicenses = append(normalizedLicenses, license) } - addInvalid := func(license string) { - if _, ok := seenInvalid[license]; ok { - return - } - seenInvalid[license] = struct{}{} - invalidLicenses = append(invalidLicenses, license) - } - for _, license := range licenses { // MIT is the most common license, so check for it first before doing any processing to optimize for this case. // By putting the isMIT check here, we can avoid the overhead of parsing for the most common case of MIT. @@ -86,7 +77,7 @@ func ValidateAndNormalizeLicensesWithOptions(licenses []string, options Validate if ok, normalizedLicense := deprecatedLicense(license); ok { if options.FailDeprecatedLicenses { - addInvalid(license) + invalidLicenses = append(invalidLicenses, license) continue } addNormalized(normalizedLicense) @@ -96,14 +87,14 @@ func ValidateAndNormalizeLicensesWithOptions(licenses []string, options Validate if options.FailAllLicenseRefs { if strings.HasPrefix(license, "LicenseRef-") { - addInvalid(license) + invalidLicenses = append(invalidLicenses, license) continue } } if options.FailAllDocumentRefs { if strings.HasPrefix(license, "DocumentRef-") { - addInvalid(license) + invalidLicenses = append(invalidLicenses, license) continue } } @@ -126,7 +117,7 @@ func ValidateAndNormalizeLicensesWithOptions(licenses []string, options Validate } } } - addInvalid(license) + invalidLicenses = append(invalidLicenses, license) continue } } @@ -134,7 +125,7 @@ func ValidateAndNormalizeLicensesWithOptions(licenses []string, options Validate // all other non-atomic expressions are complex expressions with conjunctions (e.g. "MIT AND Apache-2.0"), // so fail if complex expressions are not allowed if options.FailComplexExpressions && !isAtomic { - addInvalid(license) + invalidLicenses = append(invalidLicenses, license) continue } @@ -143,7 +134,7 @@ func ValidateAndNormalizeLicensesWithOptions(licenses []string, options Validate var parsedLicense *node var err error if parsedLicense, err = parse(license); err != nil { - addInvalid(license) + invalidLicenses = append(invalidLicenses, license) } else { normalizedLicense := *parsedLicense.reconstructedLicenseString() addNormalized(normalizedLicense) From 74a38f630eba978091c904ee53a36749d0834fff Mon Sep 17 00:00:00 2001 From: "E. Lynette Rayle" Date: Wed, 13 May 2026 07:51:37 -0400 Subject: [PATCH 5/5] no need to test for allValid for ValidateAndNormalize --- spdxexp/satisfies_test.go | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/spdxexp/satisfies_test.go b/spdxexp/satisfies_test.go index f021290..30cacdb 100644 --- a/spdxexp/satisfies_test.go +++ b/spdxexp/satisfies_test.go @@ -233,7 +233,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailComplexExpressions(t *testi name string inputLicenses []string options ValidateLicensesOptions - allValid bool invalidLicenses []string normalizedLicenses []string }{ @@ -241,7 +240,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailComplexExpressions(t *testi name: "Expressions rejected", inputLicenses: []string{"MIT AND Apache-2.0"}, options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: false, invalidLicenses: []string{ "MIT AND Apache-2.0", }, @@ -251,7 +249,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailComplexExpressions(t *testi name: "Mixed list rejects only expressions", inputLicenses: []string{"mit", "apache-2.0", "LGPL-2.1-only OR MIT"}, options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: false, invalidLicenses: []string{ "LGPL-2.1-only OR MIT", }, @@ -261,7 +258,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailComplexExpressions(t *testi name: "WITH exception is not treated as complex expression", inputLicenses: []string{"gpl-2.0-or-later WITH Bison-exception-2.2"}, options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: true, invalidLicenses: []string{}, normalizedLicenses: []string{"GPL-2.0-or-later WITH Bison-exception-2.2"}, }, @@ -272,7 +268,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailComplexExpressions(t *testi normalizedLicenses, invalidLicenses := ValidateAndNormalizeLicensesWithOptions(test.inputLicenses, test.options) assert.EqualValues(t, test.invalidLicenses, invalidLicenses, "invalid licenses should match expected") assert.EqualValues(t, test.normalizedLicenses, normalizedLicenses, "normalized licenses should match expected") - assert.Equal(t, test.allValid, len(invalidLicenses) == 0) }) } } @@ -285,7 +280,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testi name string inputLicenses []string options ValidateLicensesOptions - allValid bool invalidLicenses []string normalizedLicenses []string }{ @@ -293,7 +287,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testi name: "Deprecated license rejected", inputLicenses: []string{deprecatedLicense}, options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, - allValid: false, invalidLicenses: []string{ deprecatedLicense, }, @@ -303,7 +296,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testi name: "Mixed list rejects only deprecated licenses", inputLicenses: []string{"MIT", "Apache-2.0", deprecatedLicense}, options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, - allValid: false, invalidLicenses: []string{ deprecatedLicense, }, @@ -313,7 +305,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testi name: "WITH exception rejects deprecated license if FailDeprecatedLicenses is true", inputLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, - allValid: false, invalidLicenses: []string{ deprecatedLicense + " WITH Bison-exception-2.2", }, @@ -323,7 +314,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testi name: "WITH exception allows deprecated license if FailDeprecatedLicenses is false", inputLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, options: ValidateLicensesOptions{FailDeprecatedLicenses: false}, - allValid: true, invalidLicenses: []string{}, normalizedLicenses: []string{deprecatedLicense + " WITH Bison-exception-2.2"}, }, @@ -334,7 +324,6 @@ func TestValidateAndNormalizeLicensesWithOptions_FailDeprecatedLicenses(t *testi normalizedLicenses, invalidLicenses := ValidateAndNormalizeLicensesWithOptions(test.inputLicenses, test.options) assert.EqualValues(t, test.invalidLicenses, invalidLicenses, "invalid licenses should match expected") assert.EqualValues(t, test.normalizedLicenses, normalizedLicenses, "normalized licenses should match expected") - assert.Equal(t, test.allValid, len(invalidLicenses) == 0) }) } } @@ -350,7 +339,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name string inputLicenses []string options ValidateLicensesOptions - allValid bool invalidLicenses []string normalizedLicenses []string }{ @@ -358,7 +346,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "Dedups", inputLicenses: []string{" MIT ", "MIT", "mit"}, options: ValidateLicensesOptions{}, - allValid: true, invalidLicenses: []string{}, normalizedLicenses: []string{"MIT"}, }, @@ -366,7 +353,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "Defaults allow deprecated, refs, and expressions", inputLicenses: []string{" MIT ", deprecated, "licenseref-mine-MyLicense", licenseRef, documentRef, expression, licenseWithException}, options: ValidateLicensesOptions{}, - allValid: false, invalidLicenses: []string{"licenseref-mine-MyLicense"}, normalizedLicenses: []string{"MIT", deprecated, licenseRef, documentRef, expression, licenseWithException}, }, @@ -374,7 +360,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "FailDeprecatedLicenses rejects deprecated IDs", inputLicenses: []string{deprecated, "Apache-2.0"}, options: ValidateLicensesOptions{FailDeprecatedLicenses: true}, - allValid: false, invalidLicenses: []string{deprecated}, normalizedLicenses: []string{"Apache-2.0"}, }, @@ -382,7 +367,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "FailComplexExpressions rejects conjunction expressions", inputLicenses: []string{expression, licenseWithException}, options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: false, invalidLicenses: []string{expression}, normalizedLicenses: []string{licenseWithException}, }, @@ -390,7 +374,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "FailComplexExpressions does not duplicate invalid entries", inputLicenses: []string{"MIT AND APCHE-2.0"}, options: ValidateLicensesOptions{FailComplexExpressions: true}, - allValid: false, invalidLicenses: []string{"MIT AND APCHE-2.0"}, normalizedLicenses: []string{}, }, @@ -398,7 +381,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "FailAllLicenseRefs rejects LicenseRef but allows DocumentRef", inputLicenses: []string{licenseRef, documentRef}, options: ValidateLicensesOptions{FailAllLicenseRefs: true}, - allValid: false, invalidLicenses: []string{licenseRef}, normalizedLicenses: []string{documentRef}, }, @@ -406,7 +388,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "FailAllDocumentRefs rejects DocumentRef but allows LicenseRef", inputLicenses: []string{documentRef, licenseRef}, options: ValidateLicensesOptions{FailAllDocumentRefs: true}, - allValid: false, invalidLicenses: []string{documentRef}, normalizedLicenses: []string{licenseRef}, }, @@ -414,7 +395,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { name: "FailAllLicenseRefs and FailAllDocumentRefs rejects any non-active atomic ref", inputLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, options: ValidateLicensesOptions{FailAllLicenseRefs: true, FailAllDocumentRefs: true}, - allValid: false, invalidLicenses: []string{licenseRef, documentRef, "CustomRef-foo"}, normalizedLicenses: []string{}, }, @@ -427,7 +407,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { FailAllLicenseRefs: true, FailAllDocumentRefs: true, }, - allValid: false, invalidLicenses: []string{deprecated, licenseRef, documentRef, expression}, normalizedLicenses: []string{licenseWithException, "Apache-2.0"}, }, @@ -439,7 +418,6 @@ func TestValidateAndNormalizeLicensesWithOptions_AllOptions(t *testing.T) { normalizedLicenses, invalidLicenses := ValidateAndNormalizeLicensesWithOptions(test.inputLicenses, test.options) assert.EqualValues(t, test.invalidLicenses, invalidLicenses, "invalid licenses should match expected") assert.EqualValues(t, test.normalizedLicenses, normalizedLicenses, "normalized licenses should match expected") - assert.Equal(t, test.allValid, len(invalidLicenses) == 0) }) } }