Make ExtractLicenses handle large expressions

.github/workflows/lint.yaml

@@ -10,19 +10,17 @@ permissions:
 
 jobs:
   build:
-    strategy:
-      fail-fast: false
     runs-on: ubuntu-latest
 
     steps:
       - uses: actions/checkout@v6
       - uses: actions/setup-go@v6
         with:
-          go-version: "1.18.5"
+          go-version: "1.24.7"
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@2226d7cb06a077cd73e56eedd38eecad18e5d837
+        uses: golangci/golangci-lint-action@1e7e51e771db61008b38414a730f564565cf7c20
         with:
-          version: v1.47.3
+          version: v2.11.4
           args: --verbose --config .golangci.yaml
           skip-cache: true

.github/workflows/test.yaml

@@ -11,15 +11,13 @@ permissions:
 
 jobs:
   test:
-    strategy:
-      fail-fast: false
     runs-on: ubuntu-latest
 
     steps:
       - uses: actions/checkout@v6
       - uses: actions/setup-go@v6
         with:
-          go-version: "1.18.5"
+          go-version: "1.24.7"
 
       - name: Build program
         run: go build ./...

.golangci.yaml

@@ -1,48 +1,27 @@
 # prerequisite:
 #   [install golangci-lint](https://golangci-lint.run/usage/install/#local-installation)
-run:
-  # timeout for analysis, e.g. 30s, 5m, default is 1m
-  timeout: 5m
-
-  skip-files:
-    - .peg\.go
-    - .*\.pb\.go
-  skip-dirs:
-    - vendor
-
+version: "2"
 linters:
   enable:
-    - deadcode
-    - depguard
     - errcheck
-    - exportloopref
     - gocritic
     - gocyclo
-    - gofmt
-    - goimports
     - gosec
-    - gosimple
     - govet
     - ineffassign
     - misspell
     - nakedret
-    - prealloc
     - revive
     - staticcheck
-    - structcheck
-    - typecheck
     - unconvert
     - unused
-    - varcheck
   disable:
     - gochecknoglobals # we allow global variables in packages
     - gochecknoinits # we allow inits in packages
     - goconst # we allow repeated values to go un-const'd
     - lll # we allow any line length
     - unparam # we allow function calls to name unused parameters
-
-issues:
-  exclude-rules:
-    # Probably some broken linter for generics?
-    - linters: [ revive ]
-      text: 'receiver-naming: receiver name \S+ should be consistent with previous receiver name \S+ for invalid-type'
+formatters:
+  enable:
+    - gofmt
+    - goimports

cmd/spdx-validate/main.go

@@ -1,3 +1,4 @@
+// Package main validates newline-separated SPDX expressions from stdin or a file.
 package main
 
 import (
@@ -26,14 +27,17 @@ Examples:
   echo "MIT" | spdx-validate
   printf "MIT\nApache-2.0\n" | spdx-validate
   spdx-validate -f licenses.txt`,
-	RunE: func(cmd *cobra.Command, args []string) error {
+	RunE: func(_ *cobra.Command, _ []string) error {
 		var r io.Reader = os.Stdin
 		if filePath != "" {
+			// #nosec G304 -- file path is an explicit CLI input for this command.
 			f, err := os.Open(filePath)
 			if err != nil {
 				return fmt.Errorf("unable to open file: %w", err)
 			}
-			defer f.Close()
+			defer func() {
+				_ = f.Close()
+			}()
 			r = f
 		}
 		ok, err := validateExpressions(r, os.Stderr)
@@ -71,7 +75,7 @@ func validateExpressions(r io.Reader, w io.Writer) (bool, error) {
 		valid, _ := spdxexp.ValidateLicenses([]string{line})
 		if !valid {
 			failures++
-			fmt.Fprintf(w, "line %d: invalid SPDX expression: %q\n", lineNum, line)
+			_, _ = fmt.Fprintf(w, "line %d: invalid SPDX expression: %q\n", lineNum, line)
 		}
 	}
 
@@ -84,7 +88,7 @@ func validateExpressions(r io.Reader, w io.Writer) (bool, error) {
 	}
 
 	if failures > 0 {
-		fmt.Fprintf(w, "%d of %d expressions failed validation\n", failures, lineNum)
+		_, _ = fmt.Fprintf(w, "%d of %d expressions failed validation\n", failures, lineNum)
 		return false, nil
 	}
 

cmd/spdx-validate/main_test.go

@@ -166,11 +166,14 @@ func TestValidateExpressions_FromTempFile(t *testing.T) {
 		t.Fatalf("failed to write temp file: %v", err)
 	}
 
+	// #nosec G304 -- path is created within t.TempDir for this test.
 	f, err := os.Open(path)
 	if err != nil {
 		t.Fatalf("failed to open temp file: %v", err)
 	}
-	defer f.Close()
+	defer func() {
+		_ = f.Close()
+	}()
 
 	var w bytes.Buffer
 	ok, err := validateExpressions(f, &w)
@@ -191,11 +194,14 @@ func TestValidateExpressions_FromTempFileWithFailures(t *testing.T) {
 		t.Fatalf("failed to write temp file: %v", err)
 	}
 
+	// #nosec G304 -- path is created within t.TempDir for this test.
 	f, err := os.Open(path)
 	if err != nil {
 		t.Fatalf("failed to open temp file: %v", err)
 	}
-	defer f.Close()
+	defer func() {
+		_ = f.Close()
+	}()
 
 	var w bytes.Buffer
 	ok, err := validateExpressions(f, &w)

spdxexp/benchmark_setup_test.go

@@ -41,10 +41,10 @@ func TestMain(m *testing.M) {
 			_ = benchFlag.Value.Set("$^")
 		}
 
-		fmt.Fprintln(os.Stdout, "Benchmark summary tables:")
-		fmt.Fprintln(os.Stdout, "- ns/op average: average time per operation")
-		fmt.Fprintln(os.Stdout, "- Scale: relative to a fixed baseline per table")
-		fmt.Fprintln(os.Stdout, "")
+		_, _ = fmt.Fprintln(os.Stdout, "Benchmark summary tables:")
+		_, _ = fmt.Fprintln(os.Stdout, "- ns/op average: average time per operation")
+		_, _ = fmt.Fprintln(os.Stdout, "- Scale: relative to a fixed baseline per table")
+		_, _ = fmt.Fprintln(os.Stdout, "")
 	}
 
 	code := m.Run()
@@ -148,10 +148,10 @@ func printBenchmarkTable(w *os.File, title string, rows []benchmarkTableRow, ben
 	}
 
 	line := func() {
-		fmt.Fprintf(w, "+-%s-+-%s-+-%s-+\n", strings.Repeat("-", col1), strings.Repeat("-", col2), strings.Repeat("-", col3))
+		_, _ = fmt.Fprintf(w, "+-%s-+-%s-+-%s-+\n", strings.Repeat("-", col1), strings.Repeat("-", col2), strings.Repeat("-", col3))
 	}
 	row := func(c1, c2, c3 string) {
-		fmt.Fprintf(w, "| %-*s | %-*s | %-*s |\n", col1, c1, col2, c2, col3, c3)
+		_, _ = fmt.Fprintf(w, "| %-*s | %-*s | %-*s |\n", col1, c1, col2, c2, col3, c3)
 	}
 
 	line()
@@ -162,7 +162,7 @@ func printBenchmarkTable(w *os.File, title string, rows []benchmarkTableRow, ben
 		row(r.label, ns, r.scale)
 	}
 	line()
-	fmt.Fprintln(w, "")
+	_, _ = fmt.Fprintln(w, "")
 }
 
 func nsNumberString(ns float64) string {

spdxexp/doc.go

@@ -1,6 +1,7 @@
 /*
-Spdxexp package validates licenses and determines if a license expression is satisfied by a list of licenses.
-Validity of a license is determined by the [SPDX license list].
+Package spdxexp validates licenses and determines if a license expression is
+satisfied by a list of licenses. Validity of a license is determined by the
+[SPDX license list].
 
 [SPDX license list]: https://spdx.org/licenses/
 */

spdxexp/extracts.go

@@ -1,5 +1,10 @@
 package spdxexp
 
+import (
+	"maps"
+	"slices"
+)
+
 // ExtractLicenses extracts licenses from the given expression without duplicates.
 // Returns an array of licenses or error if error occurs during processing.
 func ExtractLicenses(expression string) ([]string, error) {
@@ -8,14 +13,30 @@ func ExtractLicenses(expression string) ([]string, error) {
 		return nil, err
 	}
 
-	expanded := node.expand(true)
-	licenses := make([]string, 0)
-	allLicenses := flatten(expanded)
-	for _, licenseNode := range allLicenses {
-		licenses = append(licenses, *licenseNode.reconstructedLicenseString())
+	seen := map[string]struct{}{}
+	collectExtractedLicenses(node, seen)
+	return slices.Collect(maps.Keys(seen)), nil
+}
+
+func collectExtractedLicenses(n *node, seen map[string]struct{}) {
+	if n == nil {
+		return
 	}
 
-	licenses = removeDuplicateStrings(licenses)
+	if n.isExpression() {
+		collectExtractedLicenses(n.left(), seen)
+		collectExtractedLicenses(n.right(), seen)
+		return
+	}
 
-	return licenses, nil
+	reconstructed := n.reconstructedLicenseString()
+	if reconstructed == nil {
+		return
+	}
+
+	license := *reconstructed
+	if _, ok := seen[license]; ok {
+		return
+	}
+	seen[license] = struct{}{}
 }

spdxexp/extracts_test.go

@@ -1,11 +1,49 @@
 package spdxexp
 
 import (
+	"context"
+	"os"
+	"os/exec"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 )
 
+const kernelHeadersLicense = `(GPL-2.0-only WITH Linux-syscall-note OR BSD-2-Clause) AND (GPL-2.0-only WITH Linux-syscall-note OR BSD-3-Clause) AND (GPL-2.0-only WITH Linux-syscall-note OR CDDL-1.0) AND (GPL-2.0-only WITH Linux-syscall-note OR Linux-OpenIB) AND (GPL-2.0-only WITH Linux-syscall-note OR MIT) AND (GPL-2.0-or-later WITH Linux-syscall-note OR BSD-3-Clause) AND (GPL-2.0-or-later WITH Linux-syscall-note OR MIT) AND Apache-2.0 AND BSD-2-Clause AND BSD-3-Clause AND BSD-3-Clause-Clear AND GFDL-1.1-no-invariants-or-later AND GPL-1.0-or-later AND (GPL-1.0-or-later OR BSD-3-Clause) AND GPL-1.0-or-later WITH Linux-syscall-note AND GPL-2.0-only AND (GPL-2.0-only OR Apache-2.0) AND (GPL-2.0-only OR BSD-2-Clause) AND (GPL-2.0-only OR BSD-3-Clause) AND (GPL-2.0-only OR CDDL-1.0) AND (GPL-2.0-only OR GFDL-1.1-no-invariants-or-later) AND (GPL-2.0-only OR GFDL-1.2-no-invariants-only) AND GPL-2.0-only WITH Linux-syscall-note AND GPL-2.0-or-later AND (GPL-2.0-or-later OR BSD-2-Clause) AND (GPL-2.0-or-later OR BSD-3-Clause) AND (GPL-2.0-or-later OR CC-BY-4.0) AND GPL-2.0-or-later WITH GCC-exception-2.0 AND GPL-2.0-or-later WITH Linux-syscall-note AND ISC AND LGPL-2.0-or-later AND (LGPL-2.0-or-later OR BSD-2-Clause) AND LGPL-2.0-or-later WITH Linux-syscall-note AND LGPL-2.1-only AND (LGPL-2.1-only OR BSD-2-Clause) AND LGPL-2.1-only WITH Linux-syscall-note AND LGPL-2.1-or-later AND LGPL-2.1-or-later WITH Linux-syscall-note AND (Linux-OpenIB OR GPL-2.0-only) AND (Linux-OpenIB OR GPL-2.0-only OR BSD-2-Clause) AND Linux-man-pages-copyleft AND MIT AND (MIT OR GPL-2.0-only) AND (MIT OR GPL-2.0-or-later) AND (MIT OR LGPL-2.1-only) AND (MPL-1.1 OR GPL-2.0-only) AND (X11 OR GPL-2.0-only) AND (X11 OR GPL-2.0-or-later) AND Zlib AND (copyleft-next-0.3.1 OR GPL-2.0-or-later)`
+
+var expectedKernelHeadersLicenses = []string{
+	"GPL-2.0-only WITH Linux-syscall-note",
+	"BSD-2-Clause",
+	"BSD-3-Clause",
+	"CDDL-1.0",
+	"Linux-OpenIB",
+	"MIT",
+	"GPL-2.0-or-later WITH Linux-syscall-note",
+	"Apache-2.0",
+	"BSD-3-Clause-Clear",
+	"GFDL-1.1-no-invariants-or-later",
+	"GPL-1.0-or-later",
+	"GPL-1.0-or-later WITH Linux-syscall-note",
+	"GPL-2.0-only",
+	"GFDL-1.2-no-invariants-only",
+	"GPL-2.0-or-later",
+	"CC-BY-4.0",
+	"GPL-2.0-or-later WITH GCC-exception-2.0",
+	"ISC",
+	"LGPL-2.0-or-later",
+	"LGPL-2.0-or-later WITH Linux-syscall-note",
+	"LGPL-2.1-only",
+	"LGPL-2.1-only WITH Linux-syscall-note",
+	"LGPL-2.1-or-later",
+	"LGPL-2.1-or-later WITH Linux-syscall-note",
+	"Linux-man-pages-copyleft",
+	"MPL-1.1",
+	"X11",
+	"Zlib",
+	"copyleft-next-0.3.1",
+}
+
 func TestExtractLicenses(t *testing.T) {
 	tests := []struct {
 		name              string
@@ -35,3 +73,32 @@
 		})
 	}
 }
+
+func TestExtractLicensesLicenseRefAndDedup(t *testing.T) {
+	licenses, err := ExtractLicenses("(LicenseRef-custom OR LicenseRef-custom) AND (DocumentRef-spdx-tool-1.2:LicenseRef-custom OR MIT)")
+	assert.NoError(t, err)
+	assert.ElementsMatch(t, []string{"LicenseRef-custom", "DocumentRef-spdx-tool-1.2:LicenseRef-custom", "MIT"}, licenses)
+}
+
+func TestExtractLicensesLongExpressionDoesNotHang(t *testing.T) {
+	if os.Getenv("GO_SPDX_EXTRACT_LICENSES_LONG_CHILD") == "1" {
+		licenses, err := ExtractLicenses(kernelHeadersLicense)
+		assert.NoError(t, err)
+		assert.ElementsMatch(t, expectedKernelHeadersLicenses, licenses)
+		return
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	// #nosec G204 this is only run as part of tests
+	cmd := exec.CommandContext(ctx, os.Args[0], "-test.run", "^TestExtractLicensesLongExpressionDoesNotHang$")
+	cmd.Env = append(os.Environ(), "GO_SPDX_EXTRACT_LICENSES_LONG_CHILD=1")
+	output, err := cmd.CombinedOutput()
+	if ctx.Err() == context.DeadlineExceeded {
+		t.Fatalf("ExtractLicenses timed out on long expression: %s", output)
+	}
+	if err != nil {
+		t.Fatalf("child process failed: %v\n%s", err, output)
+	}
+}

spdxexp/spdxlicenses/doc.go

@@ -1,8 +1,10 @@
 /*
-Spdxlicenses package provides functions to get licenses, deprecated licenses, and exceptions.  These are auto-generated and should not be modified directly.
+Package spdxlicenses provides functions to get licenses, deprecated licenses,
+and exceptions. These are auto-generated and should not be modified directly.
 Licenses are generated from the [SPDX official machine readable license list].
 
-In addition, this package includes a function to return license ranges for sequential licenses and ranges including modifiers (i.e. -only, -or-later).
+In addition, this package includes a function to return license ranges for
+sequential licenses and ranges including modifiers (i.e. -only, -or-later).
 
 [SPDX official machine readable license list]: https://github.com/spdx/license-list-data
 */

spdxexp/spdxlicenses/license_ranges.go

@@ -1,6 +1,6 @@
 package spdxlicenses
 
-// licenseRanges returns a list of license ranges.
+// 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