chore: implement apostrophising, JSON lines, improve readability

Author: Tlacenka

packages/plugin-js-packages/src/lib/runner/audit/constants.ts

@@ -17,18 +17,20 @@ export const auditScoreModifiers: Record<PackageAuditLevel, number> = {
 };
 /* eslint-enable no-magic-numbers */
 
-/* eslint-disable @typescript-eslint/consistent-type-assertions */
 export const normalizeAuditMapper: Record<
   PackageManager,
-  (_: string) => AuditResult
+  (output: string) => AuditResult
 > = {
   npm: npmToAuditResult,
   'yarn-classic': yarnv1ToAuditResult,
   // eslint-disable-next-line @typescript-eslint/naming-convention
-  'yarn-modern': _ => ({} as AuditResult),
-  pnpm: _ => ({} as AuditResult),
+  'yarn-modern': () => {
+    throw new Error('Yarn v2+ audit is not supported yet.');
+  },
+  pnpm: () => {
+    throw new Error('PNPM audit is not supported yet.');
+  },
 };
-/* eslint-enable @typescript-eslint/consistent-type-assertions */
 
 const npmDependencyOptions: Record<DependencyGroup, string[]> = {
   prod: ['--omit=dev', '--omit=optional'],
@@ -41,6 +43,7 @@ export const auditArgs = (
 ): Record<PackageManager, string[]> => ({
   npm: [...npmDependencyOptions[groupDep], '--json', '--audit-level=none'],
   'yarn-classic': ['--json', `--groups ${dependencyGroupToLong[groupDep]}`],
+  // TODO: Add once the package managers are supported.
   'yarn-modern': [],
   pnpm: [],
 });

packages/plugin-js-packages/src/lib/runner/audit/transform.ts

@@ -1,19 +1,19 @@
-import type { AuditOutput, Issue, IssueSeverity } from '@code-pushup/models';
-import { objectToEntries } from '@code-pushup/utils';
+import type { AuditOutput, Issue } from '@code-pushup/models';
+import { apostrophize, objectToEntries } from '@code-pushup/utils';
 import {
+  AuditSeverity,
   DependencyGroup,
-  PackageAuditLevel,
   PackageManager,
   packageAuditLevels,
 } from '../../config';
 import { auditScoreModifiers } from './constants';
-import { AuditResult } from './types';
+import { AuditResult, AuditSummary, Vulnerability } from './types';
 
 export function auditResultToAuditOutput(
   result: AuditResult,
   packageManager: PackageManager,
   dependenciesType: DependencyGroup,
-  auditLevelMapping: Record<PackageAuditLevel, IssueSeverity>,
+  auditLevelMapping: AuditSeverity,
 ): AuditOutput {
   const issues = vulnerabilitiesToIssues(
     result.vulnerabilities,
@@ -24,14 +24,12 @@ export function auditResultToAuditOutput(
     slug: `${packageManager}-audit-${dependenciesType}`,
     score: calculateAuditScore(result.summary),
     value: result.summary.total,
-    displayValue: vulnerabilitiesToDisplayValue(result.summary),
+    displayValue: summaryToDisplayValue(result.summary),
     ...(issues.length > 0 && { details: { issues } }),
   };
 }
 
-export function calculateAuditScore(
-  stats: Record<PackageAuditLevel | 'total', number>,
-) {
+export function calculateAuditScore(stats: AuditSummary) {
   if (stats.total === 0) {
     return 1;
   }
@@ -49,43 +47,41 @@ export function calculateAuditScore(
   );
 }
 
-export function vulnerabilitiesToDisplayValue(
-  vulnerabilities: Record<PackageAuditLevel | 'total', number>,
-): string {
-  if (vulnerabilities.total === 0) {
+export function summaryToDisplayValue(summary: AuditSummary): string {
+  if (summary.total === 0) {
     return '0 vulnerabilities';
   }
 
   const vulnerabilityStats = packageAuditLevels
-    .map(level =>
-      vulnerabilities[level] > 0 ? `${vulnerabilities[level]} ${level}` : '',
-    )
+    .map(level => (summary[level] > 0 ? `${summary[level]} ${level}` : ''))
     .filter(text => text !== '')
     .join(', ');
-  return `${vulnerabilities.total} ${
-    vulnerabilities.total === 1 ? 'vulnerability' : 'vulnerabilities'
+  return `${summary.total} ${
+    summary.total === 1 ? 'vulnerability' : 'vulnerabilities'
   } (${vulnerabilityStats})`;
 }
 
 export function vulnerabilitiesToIssues(
-  vulnerabilities: AuditResult['vulnerabilities'],
-  auditLevelMapping: Record<PackageAuditLevel, IssueSeverity>,
+  vulnerabilities: Vulnerability[],
+  auditLevelMapping: AuditSeverity,
 ): Issue[] {
   if (vulnerabilities.length === 0) {
     return [];
   }
 
-  return Object.values(vulnerabilities).map<Issue>(detail => {
+  return Object.values(vulnerabilities).map((detail): Issue => {
     const versionRange =
       detail.versionRange === '*'
         ? '**all** versions'
         : `versions **${detail.versionRange}**`;
-    const depHierarchy =
+    const directDependency =
       typeof detail.directDependency === 'string'
-        ? `\`${detail.directDependency}\`'${
-            detail.directDependency.endsWith('s') ? '' : 's'
-          } dependency \`${detail.name}\``
-        : `\`${detail.name}\` dependency`;
+        ? `\`${detail.directDependency}\``
+        : '';
+    const depHierarchy =
+      directDependency === ''
+        ? `\`${detail.name}\` dependency`
+        : `${apostrophize(directDependency)} dependency \`${detail.name}\``;
 
     const vulnerabilitySummary = `has a **${detail.severity}** vulnerability in ${versionRange}.`;
     const fixInfo = detail.fixInformation ? ` ${detail.fixInformation}` : '';

packages/plugin-js-packages/src/lib/runner/audit/transform.unit.test.ts

@@ -4,10 +4,10 @@ import { defaultAuditLevelMapping } from '../../constants';
 import {
   auditResultToAuditOutput,
   calculateAuditScore,
-  vulnerabilitiesToDisplayValue,
+  summaryToDisplayValue,
   vulnerabilitiesToIssues,
 } from './transform';
-import { AuditResult } from './types';
+import { Vulnerability } from './types';
 
 describe('auditResultToAuditOutput', () => {
   it('should return audit output with no vulnerabilities', () => {
@@ -194,7 +194,7 @@ describe('calculateAuditScore', () => {
 describe('vulnerabilitiesToDisplayValue', () => {
   it('should return passed for no vulnerabilities', () => {
     expect(
-      vulnerabilitiesToDisplayValue({
+      summaryToDisplayValue({
         critical: 0,
         high: 0,
         moderate: 0,
@@ -207,7 +207,7 @@ describe('vulnerabilitiesToDisplayValue', () => {
 
   it('should return a summary of vulnerabilities', () => {
     expect(
-      vulnerabilitiesToDisplayValue({
+      summaryToDisplayValue({
         critical: 1,
         high: 0,
         moderate: 2,
@@ -253,7 +253,7 @@ describe('vulnerabilitiesToIssues', () => {
             title: 'tough-cookie Prototype Pollution vulnerability',
             url: 'https://github.com/advisories/GHSA-72xf-g2v4-qvf3',
           },
-        ] as AuditResult['vulnerabilities'],
+        ] as Vulnerability[],
         defaultAuditLevelMapping,
       ),
     ).toEqual<Issue[]>([
@@ -273,7 +273,7 @@ describe('vulnerabilitiesToIssues', () => {
             name: '@cypress/request',
             directDependency: 'cypress',
           },
-        ] as AuditResult['vulnerabilities'],
+        ] as Vulnerability[],
         defaultAuditLevelMapping,
       ),
     ).toEqual<Issue[]>([
@@ -294,7 +294,7 @@ describe('vulnerabilitiesToIssues', () => {
             severity: 'high',
             directDependency: true,
           },
-        ] as AuditResult['vulnerabilities'],
+        ] as Vulnerability[],
         { ...defaultAuditLevelMapping, high: 'info' },
       ),
     ).toEqual<Issue[]>([
@@ -314,7 +314,7 @@ describe('vulnerabilitiesToIssues', () => {
             versionRange: '*',
             directDependency: true,
           },
-        ] as AuditResult['vulnerabilities'],
+        ] as Vulnerability[],
         defaultAuditLevelMapping,
       ),
     ).toEqual<Issue[]>([

packages/plugin-js-packages/src/lib/runner/audit/unify-type.ts

@@ -1,4 +1,4 @@
-import { objectToEntries, toUnixNewlines } from '@code-pushup/utils';
+import { fromJsonLines, objectToEntries } from '@code-pushup/utils';
 import { filterAuditResult } from '../utils';
 import {
   AuditResult,
@@ -86,15 +86,13 @@ export function npmToAdvisory(
     while (newReferences.length > 0 && !advisoryInfoFound) {
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       const ref = newReferences.pop()!;
+      prevNodes.add(ref);
       const result = npmToAdvisory(ref, vulnerabilities, prevNodes);
 
-      if (result == null) {
-        prevNodes.add(ref);
-        continue;
+      if (result != null) {
+        advisoryInfo = { title: result.title, url: result.url };
+        advisoryInfoFound = true;
       }
-
-      advisoryInfo = { title: result.title, url: result.url };
-      advisoryInfoFound = true;
     }
     /* eslint-enable functional/immutable-data, functional/no-loop-statements */
 
@@ -105,9 +103,7 @@ export function npmToAdvisory(
 }
 
 export function yarnv1ToAuditResult(output: string): AuditResult {
-  const yarnv1Json = jsonLinesToJson(output);
-  const yarnv1Result = JSON.parse(yarnv1Json) as Yarnv1AuditResultJson;
-
+  const yarnv1Result = fromJsonLines<Yarnv1AuditResultJson>(output);
   const [yarnv1Advisory, yarnv1Summary] = validateYarnv1Result(yarnv1Result);
 
   const vulnerabilities = yarnv1Advisory.map(
@@ -143,11 +139,6 @@ export function yarnv1ToAuditResult(output: string): AuditResult {
   return filterAuditResult({ vulnerabilities, summary }, 'id');
 }
 
-function jsonLinesToJson(text: string) {
-  const unifiedNewLines = toUnixNewlines(text).trim();
-  return `[${unifiedNewLines.split('\n').join(',')}]`;
-}
-
 function validateYarnv1Result(
   result: Yarnv1AuditResultJson,
 ): [Yarnv1AuditAdvisory[], Yarnv1AuditSummary] {

packages/plugin-js-packages/src/lib/runner/audit/unify-type.unit.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it } from 'vitest';
+import { toJsonLines } from '@code-pushup/utils';
 import {
   AuditResult,
   NpmAdvisory,
@@ -249,11 +250,9 @@ describe('yarnv1ToAuditResult', () => {
       },
     } satisfies Yarnv1AuditSummary;
 
-    const jsonLines = [advisory, summary]
-      .map(item => JSON.stringify(item))
-      .join('\n');
-
-    expect(yarnv1ToAuditResult(jsonLines)).toEqual<AuditResult>({
+    expect(
+      yarnv1ToAuditResult(toJsonLines([advisory, summary])),
+    ).toEqual<AuditResult>({
       vulnerabilities: [
         {
           name: 'semver',
@@ -278,13 +277,12 @@ describe('yarnv1ToAuditResult', () => {
   });
 
   it('should throw for no audit summary', () => {
-    expect(() =>
-      yarnv1ToAuditResult(
-        JSON.stringify({
-          data: {},
-          type: 'auditAdvisory',
-        } as Yarnv1AuditAdvisory),
-      ),
-    ).toThrow('no summary found');
+    const advisory = {
+      data: {},
+      type: 'auditAdvisory',
+    };
+    expect(() => yarnv1ToAuditResult(toJsonLines([advisory]))).toThrow(
+      'no summary found',
+    );
   });
 });

packages/plugin-js-packages/src/lib/runner/outdated/constants.ts

@@ -11,7 +11,7 @@ export const outdatedSeverity: Record<VersionType, IssueSeverity> = {
 
 export const outdatedArgs: Record<PackageManager, string[]> = {
   npm: ['--json', '--long'],
-  'yarn-classic': ['--json', '|', 'jq', '-s'],
+  'yarn-classic': ['--json'],
   'yarn-modern': [],
   pnpm: [],
 };

packages/plugin-js-packages/src/lib/runner/outdated/transform.ts

@@ -79,13 +79,13 @@ export function outdatedToDisplayValue(stats: Record<VersionType, number>) {
 
 export function outdatedToIssues(dependencies: OutdatedResult): Issue[] {
   return dependencies.map<Issue>(dep => {
-    const { name, current, latest, url, project } = dep;
+    const { name, current, latest, url } = dep;
     const outdatedLevel = getOutdatedLevel(current, latest);
     const packageReference =
       url == null ? `\`${name}\`` : `[\`${name}\`](${url})`;
 
     return {
-      message: `${project}'s dependency ${packageReference} requires a **${outdatedLevel}** update from **${current}** to **${latest}**.`,
+      message: `Package ${packageReference} requires a **${outdatedLevel}** update from **${current}** to **${latest}**.`,
       severity: outdatedSeverity[outdatedLevel],
     };
   });