feat: [OSM-1040] refactor feature flags usage

snyk · Apr 29, 2024 · 7dc15c9 · 7dc15c9
1 parent b0af501
commit 7dc15c9
Show file tree

Hide file tree

Showing 13 changed files with 100 additions and 233 deletions.
diff --git a/src/lib/detect.ts b/src/lib/detect.ts
@@ -5,14 +5,13 @@ import { NoSupportedManifestsFoundError } from './errors';
 import {
   SupportedPackageManagers,
   SUPPORTED_MANIFEST_FILES,
-  SUPPORTED_MANIFEST_FILES_UNDER_FF,
-  SupportedPackageManagersUnderFeatureFlag,
-  PACKAGE_MANAGERS_FEATURE_FLAGS_MAP,
+  PNPM_FEATURE_FLAG,
 } from './package-managers';
 
 const debug = debugLib('snyk-detect');
 
 const DETECTABLE_FILES: string[] = [
+  'pnpm-lock.yaml',
   'yarn.lock',
   'package-lock.json',
   'package.json',
@@ -41,6 +40,7 @@ const DETECTABLE_FILES: string[] = [
 ];
 
 export const AUTO_DETECTABLE_FILES: string[] = [
+  'pnpm-lock.yaml',
   'package-lock.json',
   'yarn.lock',
   'package.json',
@@ -68,10 +68,6 @@ export const AUTO_DETECTABLE_FILES: string[] = [
   'Package.swift',
 ];
 
-export const AUTO_DETECTABLE_FILES_UNDER_FF = {
-  pnpm: 'pnpm-lock.yaml',
-};
-
 // when file is specified with --file, we look it up here
 // this is also used when --all-projects flag is enabled and auto detection plugin is triggered
 const DETECTABLE_PACKAGE_MANAGERS: {
@@ -88,6 +84,7 @@ const DETECTABLE_PACKAGE_MANAGERS: {
   [SUPPORTED_MANIFEST_FILES.BUILD_GRADLE_KTS]: 'gradle',
   [SUPPORTED_MANIFEST_FILES.BUILD_SBT]: 'sbt',
   [SUPPORTED_MANIFEST_FILES.YARN_LOCK]: 'yarn',
+  [SUPPORTED_MANIFEST_FILES.PNPM_LOCK]: 'pnpm',
   [SUPPORTED_MANIFEST_FILES.PACKAGE_JSON]: 'npm',
   [SUPPORTED_MANIFEST_FILES.PIPFILE]: 'pip',
   [SUPPORTED_MANIFEST_FILES.SETUP_PY]: 'pip',
@@ -109,31 +106,15 @@ const DETECTABLE_PACKAGE_MANAGERS: {
   [SUPPORTED_MANIFEST_FILES.PACKAGE_SWIFT]: 'swift',
 };
 
-export const DETECTABLE_PACKAGE_MANAGERS_UNDER_FF: {
-  [key in SUPPORTED_MANIFEST_FILES_UNDER_FF]: SupportedPackageManagersUnderFeatureFlag;
-} = {
-  [SUPPORTED_MANIFEST_FILES_UNDER_FF.PNPM_LOCK]: 'pnpm',
-};
-
 export function isPathToPackageFile(
   path: string,
   featureFlags: Set<string> = new Set<string>(),
 ) {
-  for (const fileName of Object.keys(DETECTABLE_PACKAGE_MANAGERS)) {
-    if (
-      path.endsWith(fileName) &&
-      featureFlags.has(
-        PACKAGE_MANAGERS_FEATURE_FLAGS_MAP[
-          DETECTABLE_PACKAGE_MANAGERS_UNDER_FF[fileName]
-        ],
-      )
-    ) {
-      return true;
-    }
-  }
-
   for (const fileName of DETECTABLE_FILES) {
     if (path.endsWith(fileName)) {
+      if (!isFileCompatible(fileName, featureFlags)) {
+        continue;
+      }
       return true;
     }
   }
@@ -202,25 +183,31 @@ export function isLocalFolder(root: string) {
   }
 }
 
-export function detectPackageFile(
-  root: string,
+function isFileCompatible(
+  file: string,
   featureFlags: Set<string> = new Set<string>(),
 ) {
-  for (const file of Object.keys(DETECTABLE_PACKAGE_MANAGERS_UNDER_FF)) {
-    if (
-      fs.existsSync(pathLib.resolve(root, file)) &&
-      featureFlags.has(
-        PACKAGE_MANAGERS_FEATURE_FLAGS_MAP[
-          DETECTABLE_PACKAGE_MANAGERS_UNDER_FF[file]
-        ],
-      )
-    ) {
-      return file;
-    }
+  if (
+    file === SUPPORTED_MANIFEST_FILES.PNPM_LOCK &&
+    !featureFlags.has(PNPM_FEATURE_FLAG)
+  ) {
+    return false;
   }
+  return true;
+}
 
+export function detectPackageFile(
+  root: string,
+  featureFlags: Set<string> = new Set<string>(),
+) {
   for (const file of DETECTABLE_FILES) {
     if (fs.existsSync(pathLib.resolve(root, file))) {
+      if (!isFileCompatible(file, featureFlags)) {
+        debug(
+          `found pnpm lockfile ${file} in ${root}, but ${PNPM_FEATURE_FLAG} not enabled`,
+        );
+        continue;
+      }
       debug('found package file ' + file + ' in ' + root);
       return file;
     }
@@ -231,7 +218,7 @@ export function detectPackageFile(
 export function detectPackageManagerFromFile(
   file: string,
   featureFlags: Set<string> = new Set<string>(),
-): SupportedPackageManagers | SupportedPackageManagersUnderFeatureFlag {
+): SupportedPackageManagers {
   let key = pathLib.basename(file);
 
   // TODO: fix this to use glob matching instead
@@ -248,26 +235,13 @@ export function detectPackageManagerFromFile(
     key = '.war';
   }
 
-  if (
-    !(key in DETECTABLE_PACKAGE_MANAGERS) &&
-    !(key in DETECTABLE_PACKAGE_MANAGERS_UNDER_FF)
-  ) {
+  if (!(key in DETECTABLE_PACKAGE_MANAGERS)) {
     // we throw and error here because the file was specified by the user
     throw new Error('Could not detect package manager for file: ' + file);
   }
 
-  if (key in DETECTABLE_PACKAGE_MANAGERS_UNDER_FF) {
-    if (
-      featureFlags.has(
-        PACKAGE_MANAGERS_FEATURE_FLAGS_MAP[
-          DETECTABLE_PACKAGE_MANAGERS_UNDER_FF[key]
-        ],
-      )
-    ) {
-      return DETECTABLE_PACKAGE_MANAGERS_UNDER_FF[key];
-    } else {
-      throw new Error('Could not detect package manager for file: ' + file);
-    }
+  if (!isFileCompatible(key, featureFlags)) {
+    throw new Error('Could not detect package manager for file: ' + file);
   }
 
   return DETECTABLE_PACKAGE_MANAGERS[key];

diff --git a/src/lib/find-files.ts b/src/lib/find-files.ts
@@ -5,7 +5,10 @@ const sortBy = require('lodash.sortby');
 const groupBy = require('lodash.groupby');
 import { detectPackageManagerFromFile } from './detect';
 import * as debugModule from 'debug';
-import { PNPM_FEATURE_FLAG } from './package-managers';
+import {
+  PNPM_FEATURE_FLAG,
+  SUPPORTED_MANIFEST_FILES,
+} from './package-managers';
 
 const debug = debugModule('snyk:find-files');
 
@@ -277,14 +280,15 @@ function chooseBestManifest(
 ): string | null {
   switch (projectType) {
     case 'node': {
-      const nodeLockfiles = ['package-lock.json', 'yarn.lock'];
+      const nodeLockfiles = [
+        SUPPORTED_MANIFEST_FILES.PACKAGE_LOCK_JSON as string,
+        SUPPORTED_MANIFEST_FILES.YARN_LOCK as string,
+      ];
       if (featureFlags.has(PNPM_FEATURE_FLAG)) {
-        nodeLockfiles.push('pnpm-lock.yaml');
+        nodeLockfiles.push(SUPPORTED_MANIFEST_FILES.PNPM_LOCK as string);
       }
       const lockFile = files.filter((path) =>
-        ['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml'].includes(
-          path.base,
-        ),
+        nodeLockfiles.includes(path.base),
       )[0];
       debug(
         `Encountered multiple node lockfiles files, defaulting to ${lockFile.path}`,

diff --git a/src/lib/package-managers.ts b/src/lib/package-managers.ts
@@ -1,13 +1,10 @@
 export const PNPM_FEATURE_FLAG = 'enablePnpmCli';
 
-export const PACKAGE_MANAGERS_FEATURE_FLAGS_MAP = {
-  pnpm: PNPM_FEATURE_FLAG,
-};
-
 export type SupportedPackageManagers =
   | 'rubygems'
   | 'npm'
   | 'yarn'
+  | 'pnpm'
   | 'maven'
   | 'pip'
   | 'sbt'
@@ -24,8 +21,6 @@ export type SupportedPackageManagers =
   | 'Unmanaged (C/C++)'
   | 'swift';
 
-export type SupportedPackageManagersUnderFeatureFlag = 'pnpm';
-
 export enum SUPPORTED_MANIFEST_FILES {
   GEMFILE = 'Gemfile',
   GEMFILE_LOCK = 'Gemfile.lock',
@@ -38,6 +33,7 @@ export enum SUPPORTED_MANIFEST_FILES {
   BUILD_GRADLE_KTS = 'build.gradle.kts',
   BUILD_SBT = 'build.sbt',
   YARN_LOCK = 'yarn.lock',
+  PNPM_LOCK = 'pnpm-lock.yaml',
   PACKAGE_JSON = 'package.json',
   PIPFILE = 'Pipfile',
   SETUP_PY = 'setup.py',
@@ -59,16 +55,13 @@ export enum SUPPORTED_MANIFEST_FILES {
   PACKAGE_SWIFT = 'Package.swift',
 }
 
-export enum SUPPORTED_MANIFEST_FILES_UNDER_FF {
-  PNPM_LOCK = 'pnpm-lock.yaml',
-}
-
 export const SUPPORTED_PACKAGE_MANAGER_NAME: {
   readonly [packageManager in SupportedPackageManagers]: string;
 } = {
   rubygems: 'RubyGems',
   npm: 'npm',
   yarn: 'Yarn',
+  pnpm: 'pnpm',
   maven: 'Maven',
   pip: 'pip',
   sbt: 'SBT',
@@ -86,12 +79,6 @@ export const SUPPORTED_PACKAGE_MANAGER_NAME: {
   swift: 'Swift',
 };
 
-export const SUPPORTED_PACKAGE_MANAGER_UNDER_FF_NAME: {
-  readonly [packageManager in SupportedPackageManagersUnderFeatureFlag]: string;
-} = {
-  pnpm: 'pnpm',
-};
-
 export const GRAPH_SUPPORTED_PACKAGE_MANAGERS: SupportedPackageManagers[] = [
   'npm',
   'sbt',

diff --git a/src/lib/plugins/extract-package-manager.ts b/src/lib/plugins/extract-package-manager.ts
@@ -1,20 +1,14 @@
 import * as cliInterface from '@snyk/cli-interface';
 import { ScannedProjectCustom } from './get-multi-plugin-result';
-import {
-  SupportedPackageManagers,
-  SupportedPackageManagersUnderFeatureFlag,
-} from '../package-managers';
+import { SupportedPackageManagers } from '../package-managers';
 
 export function extractPackageManager(
   scannedProject: ScannedProjectCustom,
   pluginRes: cliInterface.legacyPlugin.MultiProjectResult,
   options: {
     packageManager?: SupportedPackageManagers;
   },
-):
-  | SupportedPackageManagers
-  | SupportedPackageManagersUnderFeatureFlag
-  | undefined {
+): SupportedPackageManagers | undefined {
   // try and use the package Manager from the plugin
   // result if present
   const packageManager =

diff --git a/src/lib/plugins/get-deps-from-plugin.ts b/src/lib/plugins/get-deps-from-plugin.ts
@@ -15,14 +15,12 @@ import {
   detectPackageFile,
   AUTO_DETECTABLE_FILES,
   detectPackageManagerFromFile,
-  AUTO_DETECTABLE_FILES_UNDER_FF,
 } from '../detect';
 import analytics = require('../analytics');
 import { convertSingleResultToMultiCustom } from './convert-single-splugin-res-to-multi-custom';
 import { convertMultiResultToMultiCustom } from './convert-multi-plugin-res-to-multi-custom';
 import { processYarnWorkspaces } from './nodejs-plugin/yarn-workspaces-parser';
 import { ScannedProject } from '@snyk/cli-interface/legacy/common';
-import { PACKAGE_MANAGERS_FEATURE_FLAGS_MAP } from '../package-managers';
 
 const debug = debugModule('snyk-test');
 
@@ -48,17 +46,10 @@ export async function getDepsFromPlugin(
     const levelsDeep = options.detectionDepth;
     const ignore = options.exclude ? options.exclude.split(',') : [];
 
-    const files = multiProjectProcessors[scanType].files;
-    Object.keys(AUTO_DETECTABLE_FILES_UNDER_FF).forEach((pkgManager) => {
-      if (featureFlags.has(PACKAGE_MANAGERS_FEATURE_FLAGS_MAP[pkgManager])) {
-        files.push(AUTO_DETECTABLE_FILES_UNDER_FF[pkgManager]);
-      }
-    });
-
     const { files: targetFiles, allFilesFound } = await find(
       root,
       ignore,
-      files,
+      multiProjectProcessors[scanType].files,
       featureFlags,
       levelsDeep,
     );
@@ -111,7 +102,7 @@ export async function getDepsFromPlugin(
   if (!options.docker && !(options.file || options.packageManager)) {
     throw NoSupportedManifestsFoundError([...root]);
   }
-  const inspectRes = await getSinglePluginResult(root, options, featureFlags);
+  const inspectRes = await getSinglePluginResult(root, options);
 
   if (!pluginApi.isMultiResult(inspectRes)) {
     if (!inspectRes.package && !inspectRes.dependencyGraph) {