feat(@snyk/fix): show which parent manifest fixes occurred in

When an item was already fixed, mention though which targetFile
snyk · Apr 12, 2021 · 3a68874 · 3a68874
1 parent 8f399b5
commit 3a68874
Show file tree

Hide file tree

Showing 4 changed files with 49 additions and 19 deletions.
diff --git a/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts b/packages/snyk-fix/src/plugins/python/handlers/pip-requirements/index.ts
@@ -10,7 +10,7 @@ import {
   RemediationChanges,
   Workspace,
 } from '../../../../types';
-import { PluginFixResponse } from '../../../types';
+import { FixedCache, PluginFixResponse } from '../../../types';
 import { updateDependencies } from './update-dependencies';
 import { MissingRemediationDataError } from '../../../../lib/errors/missing-remediation-data';
 import { MissingFileNameError } from '../../../../lib/errors/missing-file-name';
@@ -41,16 +41,19 @@ export async function pipRequirementsTxt(
   handlerResult.skipped.push(...notFixable);
 
   const ordered = sortByDirectory(fixable);
-  const fixedFilesCache: string[] = [];
+  let fixedFilesCache: FixedCache = {};
   for (const dir of Object.keys(ordered)) {
     debug(`Fixing entities in directory ${dir}`);
     const entitiesPerDirectory = ordered[dir].map((e) => e.entity);
-    const { failed, succeeded, skipped, fixedFiles } = await fixAll(
+    const { failed, succeeded, skipped, fixedCache } = await fixAll(
       entitiesPerDirectory,
       options,
       fixedFilesCache,
     );
-    fixedFilesCache.push(...fixedFiles);
+    fixedFilesCache = {
+      ...fixedFilesCache,
+      ...fixedCache,
+    };
     handlerResult.succeeded.push(...succeeded);
     handlerResult.failed.push(...failed);
     handlerResult.skipped.push(...skipped);
@@ -83,8 +86,8 @@ export function getRequiredData(
 async function fixAll(
   entities: EntityToFix[],
   options: FixOptions,
-  fixedCache: string[],
-): Promise<PluginFixResponse & { fixedFiles: string[] }> {
+  fixedCache: FixedCache,
+): Promise<PluginFixResponse & { fixedCache: FixedCache }> {
   const handlerResult: PluginFixResponse = {
     succeeded: [],
     failed: [],
@@ -95,27 +98,42 @@ async function fixAll(
     try {
       const { dir, base } = pathLib.parse(targetFile);
       // parse & join again to support correct separator
-      if (fixedCache.includes(pathLib.normalize(pathLib.join(dir, base)))) {
+      const filePath = pathLib.normalize(pathLib.join(dir, base));
+      if (
+        Object.keys(fixedCache).includes(
+          pathLib.normalize(pathLib.join(dir, base)),
+        )
+      ) {
         handlerResult.succeeded.push({
           original: entity,
-          changes: [{ success: true, userMessage: 'Previously fixed' }],
+          changes: [
+            {
+              success: true,
+              userMessage: `Fixed through ${fixedCache[filePath].fixedIn}`,
+            },
+          ],
         });
         continue;
       }
-      const { changes, fixedFiles } = await applyAllFixes(entity, options);
+      const { changes, fixedMeta } = await applyAllFixes(entity, options);
       if (!changes.length) {
         debug('Manifest has not changed!');
         throw new NoFixesCouldBeAppliedError();
       }
-      fixedCache.push(...fixedFiles);
+      Object.keys(fixedMeta).forEach((f) => {
+        fixedCache[f] = {
+          fixedIn: targetFile,
+        };
+      });
       handlerResult.succeeded.push({ original: entity, changes });
     } catch (e) {
       debug(`Failed to fix ${targetFile}.\nERROR: ${e}`);
       handlerResult.failed.push({ original: entity, error: e });
     }
   }
-  return { ...handlerResult, fixedFiles: [] };
+  return { ...handlerResult, fixedCache };
 }
+
 // TODO: optionally verify the deps install
 export async function fixIndividualRequirementsTxt(
   workspace: Workspace,
@@ -154,11 +172,16 @@ export async function fixIndividualRequirementsTxt(
 export async function applyAllFixes(
   entity: EntityToFix,
   options: FixOptions,
-): Promise<{ changes: FixChangesSummary[]; fixedFiles: string[] }> {
+): Promise<{
+  changes: FixChangesSummary[];
+  fixedMeta: { [filePath: string]: FixChangesSummary[] };
+}> {
   const { remediation, targetFile: entryFileName, workspace } = getRequiredData(
     entity,
   );
-  const fixedFiles: string[] = [];
+  const fixedMeta: {
+    [filePath: string]: FixChangesSummary[];
+  } = {};
   const { dir, base } = pathLib.parse(entryFileName);
   const provenance = await extractProvenance(workspace, dir, base);
   const upgradeChanges: FixChangesSummary[] = [];
@@ -178,7 +201,7 @@ export async function applyAllFixes(
     );
     appliedUpgradeRemediation.push(...appliedRemediation);
     upgradeChanges.push(...changes);
-    fixedFiles.push(pathLib.normalize(pathLib.join(dir, fileName)));
+    fixedMeta[pathLib.normalize(pathLib.join(dir, fileName))] = upgradeChanges;
   }
 
   /* Apply all left over remediation as pins in the entry targetFile */
@@ -199,7 +222,7 @@ export async function applyAllFixes(
     directUpgradesOnly,
   );
 
-  return { changes: [...upgradeChanges, ...pinnedChanges], fixedFiles };
+  return { changes: [...upgradeChanges, ...pinnedChanges], fixedMeta };
 }
 
 function filterOutAppliedUpgrades(

diff --git a/packages/snyk-fix/src/plugins/types.ts b/packages/snyk-fix/src/plugins/types.ts
@@ -1,5 +1,6 @@
 import {
   EntityToFix,
+  FixChangesSummary,
   FixOptions,
   WithError,
   WithFixChangesApplied,
@@ -19,3 +20,9 @@ export interface PluginFixResponse {
 export interface FixHandlerResultByPlugin {
   [pluginId: string]: PluginFixResponse;
 }
+
+export interface FixedCache {
+  [filePath: string]: {
+    fixedIn: string;
+  };
+}
diff --git a/...eptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap b/...eptance/plugins/python/update-dependencies/__snapshots__/update-dependencies.spec.ts.snap
@@ -11,7 +11,7 @@ Successful fixes:
   ✔ Pinned transitive from 1.0.1 to 2.0.1 (pinned in app-with-constraints/constraints.txt)
 
   app-with-constraints/lib/requirements.txt
-  ✔ Previously fixed
+  ✔ Fixed through app-with-constraints/requirements.txt
 
 Summary:
 
@@ -29,10 +29,10 @@ Successful fixes:
   ✔ Upgraded Jinja2 from 2.7.2 to 2.7.3 (upgraded in app-with-already-fixed/lib/requirements.txt)
 
   app-with-already-fixed/core/requirements.txt
-  ✔ Previously fixed
+  ✔ Fixed through app-with-already-fixed/requirements.txt
 
   app-with-already-fixed/lib/requirements.txt
-  ✔ Previously fixed
+  ✔ Fixed through app-with-already-fixed/requirements.txt
 
 Summary:
 

diff --git a/...s/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts b/...s/snyk-fix/test/acceptance/plugins/python/update-dependencies/update-dependencies.spec.ts
@@ -1118,7 +1118,7 @@ describe('fix *req*.txt / *.txt Python projects', () => {
     expect(result.results.python.succeeded[1].changes).toEqual([
       {
         success: true,
-        userMessage: 'Previously fixed',
+        userMessage: 'Fixed through app-with-constraints/requirements.txt',
       },
     ]);
   });