refactor(license): improve license expression normalization (#8257)

Signed-off-by: knqyf263 <knqyf263@gmail.com>
Co-authored-by: DmitriyLewen <dmitriy.lewen@smartforce.io>

Author: knqyf263

pkg/licensing/expression/expression.go

@@ -11,7 +11,7 @@ var (
 	ErrInvalidExpression = xerrors.New("invalid expression error")
 )
 
-type NormalizeFunc func(license string) SimpleExpr
+type NormalizeFunc func(license Expression) Expression
 
 func parse(license string) (Expression, error) {
 	l := NewLexer(strings.NewReader(license))
@@ -24,42 +24,48 @@ func parse(license string) (Expression, error) {
 	return l.result, nil
 }
 
-func Normalize(license string, fn ...NormalizeFunc) (string, error) {
+func Normalize(license string, funcs ...NormalizeFunc) (string, error) {
 	expr, err := parse(license)
 	if err != nil {
 		return "", xerrors.Errorf("license (%s) parse error: %w", license, err)
 	}
-	expr = normalize(expr, fn...)
+	for _, fn := range funcs {
+		expr = normalize(expr, fn)
+	}
 
 	return expr.String(), nil
 }
 
-func normalize(expr Expression, fn ...NormalizeFunc) Expression {
-	switch e := expr.(type) {
+func normalize(expr Expression, fn NormalizeFunc) Expression {
+	// Apply normalization function first
+	normalized := fn(expr)
+
+	switch e := normalized.(type) {
 	case SimpleExpr:
-		for _, f := range fn {
-			normalized := f(e.License)
-			e.License = normalized.License
-			e.HasPlus = e.HasPlus || normalized.HasPlus
-		}
-		return e
+		// No further normalization for SimpleExpr
 	case CompoundExpr:
-		e.left = normalize(e.left, fn...)
-		e.right = normalize(e.right, fn...)
+		// Only recursively process if the result is a CompoundExpr
+		e.left = normalize(e.left, fn)
+		e.right = normalize(e.right, fn)
 		e.conjunction.literal = strings.ToUpper(e.conjunction.literal) // e.g. "and" => "AND"
 		return e
 	}
 
-	return expr
+	return normalized
 }
 
 // NormalizeForSPDX replaces ' ' to '-' in license-id.
 // SPDX license MUST NOT have white space between a license-id.
 // There MUST be white space on either side of the operator "WITH".
 // ref: https://spdx.github.io/spdx-spec/v2.3/SPDX-license-expressions
-func NormalizeForSPDX(s string) SimpleExpr {
+func NormalizeForSPDX(expr Expression) Expression {
+	e, ok := expr.(SimpleExpr)
+	if !ok {
+		return expr // do not normalize compound expressions
+	}
+
 	var b strings.Builder
-	for _, c := range s {
+	for _, c := range e.License {
 		switch {
 		// spec: idstring = 1*(ALPHA / DIGIT / "-" / "." )
 		case isAlphabet(c) || unicode.IsNumber(c) || c == '-' || c == '.':
@@ -72,7 +78,7 @@ func NormalizeForSPDX(s string) SimpleExpr {
 			_, _ = b.WriteRune('-')
 		}
 	}
-	return SimpleExpr{License: b.String(), HasPlus: false}
+	return SimpleExpr{License: b.String(), HasPlus: e.HasPlus}
 }
 
 func isAlphabet(r rune) bool {

pkg/licensing/expression/expression_test.go

@@ -37,7 +37,7 @@ func TestNormalize(t *testing.T) {
 		{
 			name:    "upper",
 			license: "LGPL-2.1-only OR MIT",
-			fn:      func(license string) SimpleExpr { return SimpleExpr{strings.ToUpper(license), false} },
+			fn:      func(license Expression) Expression { return SimpleExpr{strings.ToUpper(license.String()), false} },
 			want:    "LGPL-2.1-ONLY OR MIT",
 		},
 	}

pkg/licensing/expression/types.go

@@ -5,6 +5,13 @@ import (
 	"slices"
 )
 
+var (
+	TokenIdent = Token{token: IDENT, literal: "IDENT"}
+	TokenAnd   = Token{token: AND, literal: "AND"}
+	TokenOR    = Token{token: OR, literal: "OR"}
+	TokenWith  = Token{token: WITH, literal: "WITH"}
+)
+
 type Expression interface {
 	String() string
 }
@@ -41,6 +48,14 @@ type CompoundExpr struct {
 	right       Expression
 }
 
+func NewCompoundExpr(left Expression, conjunction Token, right Expression) CompoundExpr {
+	return CompoundExpr{left: left, conjunction: conjunction, right: right}
+}
+
+func (c CompoundExpr) Conjunction() Token {
+	return c.conjunction
+}
+
 func (c CompoundExpr) String() string {
 	left := c.left.String()
 	if l, ok := c.left.(CompoundExpr); ok {

pkg/licensing/normalize.go

@@ -674,17 +674,38 @@ func standardizeKeyAndSuffix(name string) expr.SimpleExpr {
 }
 
 func Normalize(name string) string {
-	return NormalizeLicense(name).String()
+	return NormalizeLicense(expr.SimpleExpr{License: name}).String()
 }
 
-func NormalizeLicense(name string) expr.SimpleExpr {
+func NormalizeLicense(exp expr.Expression) expr.Expression {
+	switch e := exp.(type) {
+	case expr.SimpleExpr:
+		return normalizeSimpleExpr(e)
+	case expr.CompoundExpr:
+		return normalizeCompoundExpr(e)
+	}
+	return exp
+}
+
+func normalizeSimpleExpr(e expr.SimpleExpr) expr.Expression {
 	// Always trim leading and trailing spaces, even if we don't find this license in `mapping`.
-	name = strings.TrimSpace(name)
+	name := strings.TrimSpace(e.License)
 	normalized := standardizeKeyAndSuffix(name)
+	if found, ok := mapping[normalized.License]; ok {
+		return expr.SimpleExpr{License: found.License, HasPlus: e.HasPlus || found.HasPlus || normalized.HasPlus}
+	}
+	return expr.SimpleExpr{License: name, HasPlus: e.HasPlus}
+}
+
+func normalizeCompoundExpr(e expr.CompoundExpr) expr.Expression {
+	if e.Conjunction() != expr.TokenWith {
+		return e // Do not normalize compound expressions other than "WITH"
+	}
+	normalized := standardizeKeyAndSuffix(e.String())
 	if found, ok := mapping[normalized.License]; ok {
 		return expr.SimpleExpr{License: found.License, HasPlus: found.HasPlus || normalized.HasPlus}
 	}
-	return expr.SimpleExpr{License: name, HasPlus: false}
+	return e // Do not normalize compound expressions that are not found in `mapping`
 }
 
 func SplitLicenses(str string) []string {