Refactoring on PR feedback.

__tests__/licenses.test.ts

@@ -1,6 +1,6 @@
 import {expect, test} from '@jest/globals'
 import {Change, Changes} from '../src/schemas'
-import {hasInvalidLicenses} from '../src/licenses'
+import {getDeniedLicenseChanges} from '../src/licenses'
 
 let npmChange: Change = {
   manifest: 'package.json',
@@ -46,14 +46,14 @@ let rubyChange: Change = {
   ]
 }
 
-test('hasInvalidLicenses fails a licenses outside the allow list is found', async () => {
+test('it fails if a license outside the allow list is found', async () => {
   const changes: Changes = [npmChange, rubyChange]
-  const invalidChanges = hasInvalidLicenses(changes, {allow: ['BSD']})
+  const invalidChanges = getDeniedLicenseChanges(changes, {allow: ['BSD']})
   expect(invalidChanges[0]).toBe(npmChange)
 })
 
-test('hasInvalidLicenses fails if a denied license is found', async () => {
+test('it fails if a license inside the deny list is found', async () => {
   const changes: Changes = [npmChange, rubyChange]
-  const invalidChanges = hasInvalidLicenses(changes, {deny: ['BSD']})
+  const invalidChanges = getDeniedLicenseChanges(changes, {deny: ['BSD']})
   expect(invalidChanges[0]).toBe(rubyChange)
 })

src/licenses.ts

@@ -1,18 +1,22 @@
 import {Change, ChangeSchema} from './schemas'
 
-export function hasInvalidLicenses(
+/**
+ * Loops through a list of changes, filtering and returning the
+ * ones that don't conform to the licenses allow/deny lists.
+ * @param {Change[]} changes The list of changes to filter.
+ * @param { { allow?: string[], deny?: string[]}} licenses An object with `allow`/`deny` keys, each containing a list of licenses.
+ * @returns {Array<Change} The list of denied changes.
+ */
+export function getDeniedLicenseChanges(
   changes: Array<Change>,
-  allowLicenses: Array<string> | undefined,
-  failLicenses: Array<string> | undefined
+  licenses: {
+    allow?: Array<string>
+    deny?: Array<string>
+  }
 ): Array<Change> {
-  let disallowed: Change[] = []
+  let {allow = [], deny = []} = licenses
 
-  if (allowLicenses === undefined) {
-    allowLicenses = []
-  }
-  if (failLicenses === undefined) {
-    failLicenses = []
-  }
+  let disallowed: Change[] = []
 
   for (const change of changes) {
     let license = change.license
@@ -20,12 +24,12 @@ export function hasInvalidLicenses(
     if (license === null) {
       continue
     }
-    if (allowLicenses.length > 0) {
-      if (!allowLicenses.includes(license)) {
+    if (allow.length > 0) {
+      if (!allow.includes(license)) {
         disallowed.push(change)
       }
-    } else if (failLicenses.length > 0) {
-      if (failLicenses.includes(license)) {
+    } else if (deny.length > 0) {
+      if (deny.includes(license)) {
         disallowed.push(change)
       }
     }

src/main.ts

@@ -6,7 +6,7 @@ import {RequestError} from '@octokit/request-error'
 import {Change, PullRequestSchema, Severity} from './schemas'
 import {readConfigFile} from '../src/config'
 import {filterChangesBySeverity} from '../src/filter'
-import {hasInvalidLicenses} from './licenses'
+import {getDeniedLicenseChanges} from './licenses'
 
 async function run(): Promise<void> {
   try {
@@ -31,18 +31,15 @@ async function run(): Promise<void> {
     let minSeverity = config.fail_on_severity
     let failed = false
 
-    let licenseErrors = hasInvalidLicenses(
-      changes,
-      config.allow_licenses,
-      config.deny_licenses
-    )
+    let licenses = {
+      allow: config.allow_licenses,
+      deny: config.deny_licenses
+    }
+
+    let licenseErrors = getDeniedLicenseChanges(changes, licenses)
 
     if (licenseErrors.length > 0) {
-      printLicensesError(
-        licenseErrors,
-        config.allow_licenses,
-        config.deny_licenses
-      )
+      printLicensesError(licenseErrors, licenses)
       core.setFailed('Dependency review detected incompatible licenses.')
       return
     }
@@ -118,17 +115,25 @@ function renderSeverity(
 
 function printLicensesError(
   changes: Array<Change>,
-  allowLicenses: Array<string> | undefined,
-  denyLicenses: Array<string> | undefined
+  licenses: {
+    allow?: Array<string>
+    deny?: Array<string>
+  }
 ): void {
-  core.info('Dependency review detected incompatible licenses.')
-
-  if (allowLicenses !== undefined) {
-    core.info('\nAllowed licenses: ' + allowLicenses.join(', ') + '\n')
+  if (changes.length == 0) {
+    return
   }
 
-  if (denyLicenses !== undefined) {
-    core.info('\nDenied licenses: ' + denyLicenses.join(', ') + '\n')
+  let {allow = [], deny = []} = licenses
+
+  core.info('Dependency review detected incompatible licenses.')
+
+  if (allow.length > 0) {
+    core.info('\nAllowed licenses: ' + allow.join(', ') + '\n')
+  }
+
+  if (deny.length > 0) {
+    core.info('\nDenied licenses: ' + deny.join(', ') + '\n')
   }
 
   core.info('The following dependencies have incompatible licenses:\n')