Merge pull request #149 from github/elr/normalize

add function ValidateAndNormalizeLicensesWithOptions

Author: elrayle

spdxexp/node.go

@@ -1,6 +1,7 @@
 package spdxexp
 
 import (
+	"fmt"
 	"sort"
 	"strings"
 )
@@ -144,10 +145,12 @@ 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 {
+	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.

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 {

spdxexp/satisfies.go

@@ -34,47 +34,66 @@ 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))
+
+	addNormalized := func(license string) {
+		if _, ok := seenNormalized[license]; ok {
+			return
+		}
+		seenNormalized[license] = struct{}{}
+		normalizedLicenses = append(normalizedLicenses, 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.
 		// Having it before trimming means that licenses with leading/trailing whitespace will not be validated
 		// 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
 		}
 
 		license = strings.TrimSpace(license)
 
 		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)
 					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)
 					continue
 				}
 			}
 
 			if options.FailAllDocumentRefs {
 				if strings.HasPrefix(license, "DocumentRef-") {
-					valid = false
 					invalidLicenses = append(invalidLicenses, license)
 					continue
 				}
@@ -86,17 +105,18 @@ 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)
 				continue
 			}
@@ -105,19 +125,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)
 			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
+		var parsedLicense *node
+		var err error
+		if parsedLicense, err = parse(license); err != nil {
 			invalidLicenses = append(invalidLicenses, 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.