more SPDX unit tests to illustrate matching behavior

__tests__/external-config.test.ts

@@ -34,7 +34,7 @@ test('it reads an external config file', async () => {
 
   const config = await readConfig()
   expect(config.fail_on_severity).toEqual('critical')
-  expect(config.allow_licenses).toEqual(['BSD', 'GPL 2'])
+  expect(config.allow_licenses).toEqual(['BSD-3-Clause', 'GPL-2.0'])
 })
 
 test('raises an error when the config file was not found', async () => {

__tests__/fixtures/config-allow-sample.yml

@@ -1,4 +1,4 @@
 fail_on_severity: critical
 allow_licenses:
-  - 'BSD'
-  - 'GPL 2'
+  - 'BSD-3-Clause'
+  - 'GPL-2.0'

__tests__/spdx.test.ts

@@ -1,11 +1,267 @@
-import {expect, jest, test} from '@jest/globals'
+import {expect, test} from '@jest/globals'
 import * as spdx from '../src/spdx'
 
+test('satisfiesAny', () => {
+  const units = [
+    {
+      candidate: 'MIT',
+      licenses: ['MIT'],
+      expected: true
+    },
+    {
+      candidate: 'MIT OR Apache-2.0',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: true
+    },
+    {
+      candidate: '(MIT AND ISC) OR Apache-2.0',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: true
+    },
+    {
+      candidate: 'MIT AND Apache-2.0',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: 'MIT AND BSD-3-Clause',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+
+    // missing params, case sensitivity, syntax problems,
+    // or unknown licenses will return 'false'
+    {
+      candidate: 'MIT OR',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: '',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: 'MIT OR (Apache-2.0 AND ISC)',
+      licenses: [],
+      expected: false
+    },
+    {
+      candidate: 'MIT AND (ISC',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: 'MIT OR ISC',
+      licenses: ['MiT'],
+      expected: false
+    }
+  ]
+
+  for (const unit of units) {
+    let got: boolean = spdx.satisfiesAny(unit.candidate, unit.licenses)
+    if (got != unit.expected) {
+      console.log(
+        `failing unit test inputs: candidate(${unit.candidate}) licenses(${unit.licenses})`
+      )
+    }
+    expect(got).toBe(unit.expected)
+  }
+})
+
+test('satisfiesAll', () => {
+  const units = [
+    {
+      candidate: 'MIT',
+      licenses: ['MIT'],
+      expected: true
+    },
+    {
+      candidate: 'Apache-2.0',
+      licenses: ['MIT', 'ISC', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: 'MIT AND Apache-2.0',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: true
+    },
+    {
+      candidate: '(MIT OR ISC) AND Apache-2.0',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: true
+    },
+    {
+      candidate: 'MIT OR BSD-3-Clause',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: 'BSD-3-Clause OR ISC',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: '(MIT AND ISC) OR Apache-2.0',
+      licenses: ['MIT', 'ISC'],
+      expected: true
+    },
+
+    // missing params, case sensitivity, syntax problems,
+    // or unknown licenses will return 'false'
+    {
+      candidate: 'MIT OR',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: '',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: 'MIT OR (Apache-2.0 AND ISC)',
+      licenses: [],
+      expected: false
+    },
+    {
+      candidate: 'MIT AND (ISC',
+      licenses: ['MIT', 'Apache-2.0'],
+      expected: false
+    },
+    {
+      candidate: 'MIT OR ISC',
+      licenses: ['MiT'],
+      expected: false
+    }
+  ]
+
+  for (const unit of units) {
+    let got: boolean = spdx.satisfiesAll(unit.candidate, unit.licenses)
+    if (got != unit.expected) {
+      console.log(
+        `failing unit test inputs: candidate(${unit.candidate}) licenses(${unit.licenses})`
+      )
+    }
+    expect(got).toBe(unit.expected)
+  }
+})
+
 test('satisfies', () => {
-  expect(spdx.satisfies('MIT', 'MIT')).toBe(true)
+  const units = [
+    {
+      candidate: 'MIT',
+      constraint: 'MIT',
+      expected: true
+    },
+    {
+      candidate: 'Apache-2.0',
+      constraint: 'MIT',
+      expected: false
+    },
+    {
+      candidate: 'MIT OR Apache-2.0',
+      constraint: 'MIT',
+      expected: true
+    },
+    {
+      candidate: 'MIT OR Apache-2.0',
+      constraint: 'Apache-2.0',
+      expected: true
+    },
+    {
+      candidate: 'MIT OR Apache-2.0',
+      constraint: 'BSD-3-Clause',
+      expected: false
+    },
+    {
+      candidate: 'MIT OR Apache-2.0',
+      constraint: 'Apache-2.0 OR BSD-3-Clause',
+      expected: true
+    },
+    {
+      candidate: 'MIT AND Apache-2.0',
+      constraint: 'MIT AND Apache-2.0',
+      expected: true
+    },
+    {
+      candidate: 'MIT OR Apache-2.0',
+      constraint: 'MIT AND Apache-2.0',
+      expected: false
+    },
+    {
+      candidate: 'ISC OR (MIT AND Apache-2.0)',
+      constraint: 'MIT AND Apache-2.0',
+      expected: true
+    },
+
+    // missing params, case sensitivity, syntax problems,
+    // or unknown licenses will return 'false'
+    {
+      candidate: 'MIT',
+      constraint: 'MiT',
+      expected: false
+    },
+    {
+      candidate: 'MIT AND (ISC OR',
+      constraint: 'MIT',
+      expected: false
+    },
+    {
+      candidate: 'MIT OR ISC OR Apache-2.0',
+      constraint: '',
+      expected: false
+    },
+    {
+      candidate: '',
+      constraint: '(BSD-3-Clause AND ISC) OR MIT',
+      expected: false
+    }
+  ]
+
+  for (const unit of units) {
+    let got: boolean = spdx.satisfies(unit.candidate, unit.constraint)
+    if (got != unit.expected) {
+      console.log(
+        `failing unit test inputs: candidateExpr(${unit.candidate}) constraintExpr(${unit.constraint})`
+      )
+    }
+    expect(got).toBe(unit.expected)
+  }
 })
 
 test('isValid', () => {
-  expect(spdx.isValid('MIT')).toBe(true)
-  expect(spdx.isValid('FOOBARBAZ')).toBe(false)
+  const units = [
+    {
+      candidate: 'MIT',
+      expected: true
+    },
+    {
+      candidate: 'MIT AND BSD-3-Clause',
+      expected: true
+    },
+    {
+      candidate: '(MIT AND ISC) OR BSD-3-Clause',
+      expected: true
+    },
+    {
+      candidate: 'NOASSERTION',
+      expected: false
+    },
+    {
+      candidate: 'Foobar',
+      expected: false
+    },
+    {
+      candidate: '',
+      expected: false
+    }
+  ]
+  for (const unit of units) {
+    let got: boolean = spdx.isValid(unit.candidate)
+    if (got != unit.expected) {
+      console.log(`failing unit test inputs: candidateExpr(${unit.candidate})`)
+    }
+    expect(got).toBe(unit.expected)
+  }
 })

src/licenses.ts

@@ -87,11 +87,19 @@ export async function getInvalidLicenseChanges(
     } else if (validityCache.get(license) === undefined) {
       try {
         if (allow !== undefined) {
-          const found = spdx.satisfiesAny(license, allow)
-          validityCache.set(license, found)
+          if (spdx.isValid(license)) {
+            const found = spdx.satisfiesAny(license, allow)
+            validityCache.set(license, found)
+          } else {
+            invalidLicenseChanges.unresolved.push(change)
+          }
         } else if (deny !== undefined) {
-          const found = spdx.satisfiesAny(license, deny)
-          validityCache.set(license, !found)
+          if (spdx.isValid(license)) {
+            const found = spdx.satisfiesAny(license, deny)
+            validityCache.set(license, !found)
+          } else {
+            invalidLicenseChanges.unresolved.push(change)
+          }
         }
       } catch (err) {
         invalidLicenseChanges.unresolved.push(change)

src/spdx.ts

@@ -1,28 +1,51 @@
-import * as spdx from '@onebeyond/spdx-license-satisfies'
+import * as spdxlib from '@onebeyond/spdx-license-satisfies'
 import parse from 'spdx-expression-parse'
 
+/*
+ * NOTE: spdx-license-satisfies methods depend on spdx-expression-parse
+ * which throws errors in the presence of any syntax trouble, unknown
+ * license tokens, case sensitivity problems etc. to simplify handling
+ * you should pre-screen inputs to the satisfies* methods using isValid
+ */
+
+// accepts a pair of well-formed SPDX expressions. the
+// candidate is tested against the constraint
 export function satisfies(
   candidateExpr: string,
   constraintExpr: string
 ): boolean {
-  return spdx.satisfies(candidateExpr, constraintExpr)
+  try {
+    return spdxlib.satisfies(candidateExpr, constraintExpr)
+  } catch (_) {
+    return false
+  }
 }
 
+// accepts an SPDX expression and a non-empty list of licenses (not expressions)
 export function satisfiesAny(
   candidateExpr: string,
   licenses: string[]
 ): boolean {
-  return spdx.satisfiesAny(candidateExpr, licenses)
+  try {
+    return spdxlib.satisfiesAny(candidateExpr, licenses)
+  } catch (_) {
+    return false
+  }
 }
 
+// accepts an SPDX expression and a non-empty list of licenses (not expressions)
 export function satisfiesAll(
   candidateExpr: string,
   licenses: string[]
 ): boolean {
-  return spdx.satisfiesAll(candidateExpr, licenses)
+  try {
+    return spdxlib.satisfiesAll(candidateExpr, licenses)
+  } catch (_) {
+    return false
+  }
 }
 
-// can be a single license or an SPDX expression
+// accepts any SPDX expression
 export function isValid(spdxExpr: string): boolean {
   try {
     parse(spdxExpr)