Fix tests and respond to review feedback

__tests__/spdx.test.ts

@@ -83,7 +83,6 @@ describe('satisfiesAll', () => {
       licenses: ['MIT'],
       expected: true
     },
-    // TODO(dangoor): this does not seem correct to me: the only license is Apache-2.0 and it's on the list
     {
       candidate: 'Apache-2.0',
       licenses: ['MIT', 'ISC', 'Apache-2.0'],
@@ -234,7 +233,12 @@ describe('satisfies', () => {
     },
     {
       candidate: 'MIT OR OTHER',
-      constraint: 'MIT OR LicenseRef-clearlydefined-OTHER',
+      allowList: ['MIT', 'LicenseRef-clearlydefined-OTHER'],
+      expected: true
+    },
+    {
+      candidate: '(Apache-2.0 AND OTHER) OR (MIT AND OTHER)',
+      allowList: ['Apache-2.0', 'LicenseRef-clearlydefined-OTHER'],
       expected: true
     }
   ]
@@ -286,7 +290,7 @@ describe('isValid', () => {
   }
 })
 
-describe('removeInvalidSPDX', () => {
+describe('cleanInvalidSPDX', () => {
   const units = [
     {
       candidate: 'MIT',
@@ -314,7 +318,7 @@ describe('removeInvalidSPDX', () => {
     }
   ]
   for (const unit of units) {
-    const got: string = spdx.removeInvalidSPDX(unit.candidate)
+    const got: string = spdx.cleanInvalidSPDX(unit.candidate)
     test(`should return ${unit.expected} for ("${unit.candidate}")`, () => {
       expect(got).toBe(unit.expected)
     })

src/spdx.ts

@@ -12,7 +12,7 @@ import parse from 'spdx-expression-parse'
 // accepts a pair of well-formed SPDX expressions. the
 // candidate is tested against the constraint
 export function satisfies(candidateExpr: string, allowList: string[]): boolean {
-  candidateExpr = removeInvalidSPDX(candidateExpr)
+  candidateExpr = cleanInvalidSPDX(candidateExpr)
   try {
     return spdxSatisfies(candidateExpr, allowList)
   } catch (_) {
@@ -25,7 +25,7 @@ export function satisfiesAny(
   candidateExpr: string,
   licenses: string[]
 ): boolean {
-  candidateExpr = removeInvalidSPDX(candidateExpr)
+  candidateExpr = cleanInvalidSPDX(candidateExpr)
   try {
     return spdxlib.satisfiesAny(candidateExpr, licenses)
   } catch (_) {
@@ -38,7 +38,7 @@ export function satisfiesAll(
   candidateExpr: string,
   licenses: string[]
 ): boolean {
-  candidateExpr = removeInvalidSPDX(candidateExpr)
+  candidateExpr = cleanInvalidSPDX(candidateExpr)
   try {
     return spdxlib.satisfiesAll(candidateExpr, licenses)
   } catch (_) {
@@ -48,7 +48,7 @@ export function satisfiesAll(
 
 // accepts any SPDX expression
 export function isValid(spdxExpr: string): boolean {
-  spdxExpr = removeInvalidSPDX(spdxExpr)
+  spdxExpr = cleanInvalidSPDX(spdxExpr)
   try {
     parse(spdxExpr)
     return true
@@ -57,10 +57,10 @@ export function isValid(spdxExpr: string): boolean {
   }
 }
 
-const replaceOtherRegex = /(?<![\w-])OTHER(?![\w-])/
+const replaceOtherRegex = /(?<![\w-])OTHER(?![\w-])/g
 
 // adjusts license expressions to not include the invalid `OTHER`
 // which ClearlyDefined adds to license strings
-export function removeInvalidSPDX(spdxExpr: string): string {
+export function cleanInvalidSPDX(spdxExpr: string): string {
   return spdxExpr.replace(replaceOtherRegex, 'LicenseRef-clearlydefined-OTHER')
 }