From b089c5b0022d8994cb98459b12d7b538ad27e6f7 Mon Sep 17 00:00:00 2001
From: David Losert <davelosert@github.com>
Date: Tue, 28 Feb 2023 11:08:39 +0000
Subject: [PATCH] Adds conditional license summary

---
 __tests__/summary.test.ts | 73 +++++++++++++++++++++++++++++++++++++++
 src/summary.ts            | 65 +++++++++++-----------------------
 2 files changed, 94 insertions(+), 44 deletions(-)

diff --git a/__tests__/summary.test.ts b/__tests__/summary.test.ts
index 000d33f..6ea8149 100644
--- a/__tests__/summary.test.ts
+++ b/__tests__/summary.test.ts
@@ -212,3 +212,76 @@ test('addChangeVulnerabilitiesToSummary() - does not print severity statment if
   const text = core.summary.stringify()
   expect(text).not.toContain('Only included vulnerabilities')
 })
+
+test('addLicensesToSummary() - does not include entire section if no license issues found', () => {
+  summary.addLicensesToSummary(emptyInvalidLicenseChanges, defaultConfig)
+  const text = core.summary.stringify()
+  expect(text).toEqual('')
+})
+
+test('addLicensesToSummary() - includes all license issues', () => {
+  const licenseIssues = {
+    forbidden: [createTestChange()],
+    unresolved: [createTestChange(), createTestChange()],
+    unlicensed: [createTestChange(), createTestChange(), createTestChange()]
+  }
+
+  summary.addLicensesToSummary(licenseIssues, defaultConfig)
+
+  const text = core.summary.stringify()
+  expect(text).toContain('<h3>License Issues</h3>')
+  expect(text).toContain('<h4>Incompatible Licenses</h4>')
+  expect(text).toContain('<h4>Unknown Licenses</h4>')
+  expect(text).toContain('<h4>Invalid SPDX License Definitions</h4>')
+})
+
+test('addLicensesToSummary() - does not include specific license type sub-section if nothing is found', () => {
+  const licenseIssues = {
+    forbidden: [],
+    unlicensed: [],
+    unresolved: [createTestChange()]
+  }
+
+  summary.addLicensesToSummary(licenseIssues, defaultConfig)
+
+  const text = core.summary.stringify()
+  expect(text).not.toContain('<h4>Incompatible Licenses</h4>')
+  expect(text).not.toContain('<h4>Unknown Licenses</h4>')
+  expect(text).toContain('<h4>Invalid SPDX License Definitions</h4>')
+})
+
+test('addLicensesToSummary() - includes list of configured allowed licenses', () => {
+  const licenseIssues = {
+    forbidden: [createTestChange()],
+    unresolved: [],
+    unlicensed: []
+  }
+
+  const config: ConfigurationOptions = {
+    ...defaultConfig,
+    allow_licenses: ['MIT', 'Apache-2.0']
+  }
+
+  summary.addLicensesToSummary(licenseIssues, config)
+
+  const text = core.summary.stringify()
+  expect(text).toContain('<strong>Allowed Licenses</strong>: MIT, Apache-2.0')
+})
+
+test('addLicensesToSummary() - includes configured denied license', () => {
+  const licenseIssues = {
+    forbidden: [createTestChange()],
+    unresolved: [],
+    unlicensed: []
+  }
+
+  const config: ConfigurationOptions = {
+    ...defaultConfig,
+    deny_licenses: ['MIT']
+  }
+
+  summary.addLicensesToSummary(licenseIssues, config)
+
+  const text = core.summary.stringify()
+  expect(text).toContain('<strong>Denied Licenses</strong>: MIT')
+})
diff --git a/src/summary.ts b/src/summary.ts
index 6c85798..1ce5bc1 100644
--- a/src/summary.ts
+++ b/src/summary.ts
@@ -10,28 +10,6 @@ const icons = {
   warning: '⚠️'
 }
 
-export function createSummary(
-  addedChanges: Changes,
-  invalidLicenseChanges: InvalidLicenseChanges,
-  config: ConfigurationOptions
-): void {
-  addSummaryToSummary(
-    config.vulnerability_check ? addedChanges : [],
-    config.license_check
-      ? invalidLicenseChanges
-      : {unresolved: [], forbidden: [], unlicensed: []},
-    config
-  )
-
-  if (config.vulnerability_check && addedChanges.length > 0) {
-    addChangeVulnerabilitiesToSummary(addedChanges, config.fail_on_severity)
-  }
-
-  if (config.license_check && invalidLicenseChanges.unresolved.length > 0) {
-    addLicensesToSummary(invalidLicenseChanges, config)
-  }
-}
-
 export function addSummaryToSummary(
   addedPackages: Changes,
   invalidLicenseChanges: InvalidLicenseChanges,
@@ -144,9 +122,13 @@ export function addChangeVulnerabilitiesToSummary(
 }
 
 export function addLicensesToSummary(
-  invalidLicenseChanges: Record<string, Changes>,
+  invalidLicenseChanges: InvalidLicenseChanges,
   config: ConfigurationOptions
 ): void {
+  if (countLicenseIssues(invalidLicenseChanges) === 0) {
+    return
+  }
+
   core.summary.addHeading('License Issues', 3)
 
   if (config.allow_licenses && config.allow_licenses.length > 0) {
@@ -160,11 +142,6 @@ export function addLicensesToSummary(
     )
   }
 
-  if (Object.values(invalidLicenseChanges).every(item => item.length === 0)) {
-    core.summary.addQuote('No license violations detected.')
-    return
-  }
-
   core.debug(
     `found ${invalidLicenseChanges.unlicensed.length} unknown licenses`
   )
@@ -184,27 +161,27 @@ export function addLicensesToSummary(
   )
 }
 function printLicenseViolation(heading: string, changes: Changes): void {
-  core.summary.addHeading(heading, 5).addSeparator()
+  if (changes.length === 0) {
+    return
+  }
 
-  if (changes.length > 0) {
-    const rows: SummaryTableRow[] = []
-    const manifests = getManifestsSet(changes)
+  core.summary.addHeading(heading, 4).addSeparator()
 
-    for (const manifest of manifests) {
-      core.summary.addHeading(`<em>${manifest}</em>`, 4)
+  const rows: SummaryTableRow[] = []
+  const manifests = getManifestsSet(changes)
 
-      for (const change of changes.filter(pkg => pkg.manifest === manifest)) {
-        rows.push([
-          renderUrl(change.source_repository_url, change.name),
-          change.version,
-          formatLicense(change.license)
-        ])
-      }
+  for (const manifest of manifests) {
+    core.summary.addHeading(`<em>${manifest}</em>`, 4)
 
-      core.summary.addTable([['Package', 'Version', 'License'], ...rows])
+    for (const change of changes.filter(pkg => pkg.manifest === manifest)) {
+      rows.push([
+        renderUrl(change.source_repository_url, change.name),
+        change.version,
+        formatLicense(change.license)
+      ])
     }
-  } else {
-    core.summary.addQuote(`No ${heading.toLowerCase()} detected.`)
+
+    core.summary.addTable([['Package', 'Version', 'License'], ...rows])
   }
 }