chore: update per PR comments; squash before merging

src/lib/snyk-test/index.js

@@ -3,6 +3,7 @@ module.exports = test;
 var detect = require('../detect');
 var runTest = require('./run-test');
 var chalk = require('chalk');
+var npm = require('./npm');
 
 function test(root, options, callback) {
   if (typeof options === 'function') {
@@ -48,7 +49,7 @@ function executeTest(root, options) {
 function run(root, options) {
   var packageManager = options.packageManager;
   if (['npm', 'yarn'].indexOf(packageManager) >= 0) {
-    return require('./npm')(packageManager, root, options).then((res) => [res]);
+    return npm(packageManager, root, options).then((res) => [res]);
   }
   if (!options.docker && [
     'rubygems',

src/lib/snyk-test/npm-plugin/index.ts

@@ -4,16 +4,9 @@ import {PkgTree} from 'snyk-nodejs-lockfile-parser';
 
 export async function inspect(root: string, targetFile: string, options: any = {}): Promise<PkgTree> {
   const isLockFileBased = (targetFile.endsWith('package-lock.json') || targetFile.endsWith('yarn.lock'));
-  if (targetFile.endsWith('yarn.lock') && getRuntimeVersion() < 6) {
-    options.traverseNodeModules = true;
-  }
 
   const getLockFileDeps = isLockFileBased && !options.traverseNodeModules;
   return getLockFileDeps ?
     await lockParser.parse(root, targetFile, options) :
     await modulesParser.parse(root, targetFile, options);
 }
-
-function getRuntimeVersion() {
-  return parseInt(process.version.slice(1).split('.')[0], 10);
-}

src/lib/snyk-test/npm-plugin/npm-lock-parser.ts

@@ -7,8 +7,9 @@ import * as _ from 'lodash';
 import * as analytics from '../../analytics';
 import * as fs from 'fs';
 import * as lockFileParser from 'snyk-nodejs-lockfile-parser';
+import {PkgTree} from 'snyk-nodejs-lockfile-parser';
 
-export async function parse(root, targetFile, options) {
+export async function parse(root, targetFile, options): Promise<PkgTree> {
   const lockFileFullPath = path.resolve(root, targetFile);
   if (!fs.existsSync(lockFileFullPath)) {
     throw new Error('Lockfile ' + targetFile + ' not found at location: ' +
@@ -20,21 +21,17 @@ export async function parse(root, targetFile, options) {
   const shrinkwrapFullPath = path.resolve(fullPath.dir, 'npm-shrinkwrap.json');
 
   if (!fs.existsSync(manifestFileFullPath)) {
-    throw new Error('Manifest file package.json not found at location: ' +
-      manifestFileFullPath);
-  }
-
-  if (!manifestFileFullPath && lockFileFullPath) {
     throw new Error('Detected a lockfile at location: '
-      + lockFileFullPath + '\n However the package.json is missing!');
+      + lockFileFullPath + '\n However the package.json from location: ' +
+      manifestFileFullPath);
   }
 
   if (fs.existsSync(shrinkwrapFullPath)) {
     throw new Error('`npm-shrinkwrap.json` was found while using lockfile.\n'
       + 'Please run your command again without `--file=' + targetFile + '` flag.');
   }
 
-  const manifestFile = fs.readFileSync(manifestFileFullPath);
+  const manifestFile = fs.readFileSync(manifestFileFullPath, 'utf-8');
   const lockFile = fs.readFileSync(lockFileFullPath, 'utf-8');
 
   analytics.add('local', true);
@@ -52,7 +49,7 @@ export async function parse(root, targetFile, options) {
     await spinner(resolveModuleSpinnerLabel);
     const strictOutOfSync = _.get(options, 'strictOutOfSync') !== 'false';
     return lockFileParser
-      .buildDepTree(manifestFile.toString(), lockFile, options.dev, lockFileType, strictOutOfSync);
+      .buildDepTree(manifestFile, lockFile, options.dev, lockFileType, strictOutOfSync);
   } finally {
     await spinner.clear(resolveModuleSpinnerLabel);
   }

src/lib/snyk-test/npm-plugin/npm-modules-parser.ts

@@ -11,8 +11,7 @@ export async function parse(root, targetFile, options): Promise<PkgTree> {
     'node_modules',
   );
 
-  const nodeModulesExist = await fs.exists(nodeModulesPath);
-  if (!nodeModulesExist) {
+  if (!fs.existsSync(nodeModulesPath)) {
     // throw a custom error
     throw new Error('Missing node_modules folder: we can\'t test ' +
       `without dependencies.\nPlease run '${options.packageManager} install' first.`);
@@ -27,7 +26,6 @@ export async function parse(root, targetFile, options): Promise<PkgTree> {
     path.dirname(path.resolve(root, targetFile));
   try {
     await spinner(resolveModuleSpinnerLabel);
-    // yarn projects fall back to node_module traversal if node < 6
     if (targetFile.endsWith('yarn.lock')) {
       options.file = options.file.replace('yarn.lock', 'package.json');
     }
@@ -36,9 +34,8 @@ export async function parse(root, targetFile, options): Promise<PkgTree> {
     if (targetFile.endsWith('package-lock.json')) {
       options.file = options.file.replace('package-lock.json', 'package.json');
     }
-    const modules = snyk.modules(
+    return snyk.modules(
       root, Object.assign({}, options, {noFromArrays: true}));
-    return modules;
   } finally {
     spinner.clear(resolveModuleSpinnerLabel)();
   }

src/lib/snyk-test/npm/index.ts

@@ -15,7 +15,7 @@ import * as depGraphLib from '@snyk/dep-graph';
 import {AnnotatedIssue, convertTestDepGraphResultToLegacy} from '../legacy';
 
 // important: this is different from ./config (which is the *user's* config)
-import * as config from '../../config';
+import * as snykConfig from '../../config';
 
 export = runTest;
 
@@ -37,7 +37,7 @@ interface Payload {
   qs?: object | null;
   modules?: {
     numDependencies: number;
-    pluck: any;
+    pluck: any; // function asiting in traversal of node_modules (snyk-resolve-deps/lib/pluck.js)
   };
 }
 
@@ -93,7 +93,7 @@ async function assemblePayload(root: string, options): Promise<Payload> {
 
 function assembleRemotePayload(root: string, options): Payload {
   // options.vulnEndpoint is only used for file system tests
-  const url = `${config.API}${(options.vulnEndpoint || `/vuln/${options.packageManager}`)}`;
+  const url = `${snykConfig.API}${(options.vulnEndpoint || `/vuln/${options.packageManager}`)}`;
   const module = moduleToObject(root);
   debug('testing remote: %s', module.name + '@' + module.version);
 
@@ -141,7 +141,7 @@ async function assembleLocalPayload(root: string, options): Promise<Payload> {
     return {
       method: 'POST',
       // options.vulnEndpoint is only used for file system tests
-      url: config.API + options.vulnEndpoint,
+      url: snykConfig.API + options.vulnEndpoint,
       qs: common.assembleQueryString(options),
       json: true,
       headers: {
@@ -166,7 +166,7 @@ async function assembleLocalPayload(root: string, options): Promise<Payload> {
   return {
     method: 'POST',
     // options.vulnEndpoint is only used for file system tests
-    url: config.API + '/test-dep-graph',
+    url: snykConfig.API + '/test-dep-graph',
     qs: common.assembleQueryString(options),
     json: true,
     headers: {
@@ -200,6 +200,7 @@ async function sendPayload(payload: Payload): Promise<any> {
   const hasDevDependencies = payload && payload.body && payload.body.hasDevDependencies;
   const filesystemPolicy = payload.body && !!payload.body.policy;
 
+  //TODO switch to request-native-promise (orsagie)
   return await new Promise((resolve, reject) => {
     request(payload, (error, result, body) => {
       if (error) {