Merge pull request #515 from snyk/feat/suggest-usage-for-sln

Feat: Don't assume targetFile is always present & show a user message with a suggestion to help guide

src/cli/commands/protect/wizard.js

@@ -31,6 +31,8 @@ const cwd = process.cwd();
 const detect = require('../../../lib/detect');
 const plugins = require('../../../lib/plugins');
 const moduleInfo = require('../../../lib/module-info');
+const {MissingTargetFileError} = require('../../../lib/errors/missing-targetfile-error');
+
 const unsupportedPackageManagers = {
   rubygems: 'RubyGems',
   maven: 'Maven',
@@ -279,6 +281,9 @@ function processAnswers(answers, policy, options) {
   var packageFile = path.resolve(cwd, 'package.json');
   var packageManager = detect.detectPackageManager(cwd, options);
   var targetFile = options.file || detect.detectPackageFile(cwd);
+  if (!targetFile) {
+    throw MissingTargetFileError(cwd);
+  }
   const isLockFileBased = targetFile.endsWith('package-lock.json') || targetFile.endsWith('yarn.lock');
 
   var pkg = {};

src/cli/index.ts

@@ -14,6 +14,7 @@ import errors = require('../lib/errors/legacy-errors');
 import ansiEscapes = require('ansi-escapes');
 import {isPathToPackageFile} from '../lib/detect';
 import {updateCheck} from '../lib/updater';
+import { MissingTargetFileError } from '../lib/errors/missing-targetfile-error';
 
 async function runCommand(args) {
   const result = await args.method(...args.options._);
@@ -102,8 +103,7 @@ function checkRuntime() {
 function checkPaths(args) {
   for (const path of args.options._) {
     if (typeof path === 'string' && isPathToPackageFile(path)) {
-      throw new Error(`Not a recognised option did you mean --file=${path}. ` +
-        'Check other options by running snyk --help');
+      throw MissingTargetFileError(path);
     }
   }
 }

src/lib/errors/index.ts

@@ -1 +1,2 @@
 export {NoSupportedManifestsFoundError} from './no-supported-manifests-found';
+export {MissingTargetFileError} from './missing-targetfile-error';

src/lib/errors/missing-targetfile-error.ts

@@ -0,0 +1,11 @@
+import {CustomError} from './custom-error';
+
+export function MissingTargetFileError(path: string) {
+  const errorMsg = `Not a recognised option did you mean --file=${path}? ` +
+  'Check other options by running snyk --help';
+
+  const error = new CustomError(errorMsg);
+  error.code = 422;
+  error.userMessage = errorMsg;
+  return error;
+}

src/lib/plugins/nodejs-plugin/index.ts

@@ -1,9 +1,13 @@
 import * as modulesParser from './npm-modules-parser';
 import * as lockParser from './npm-lock-parser';
 import * as types from '../types';
+import { MissingTargetFileError } from '../../errors/missing-targetfile-error';
 
 export async function inspect(root: string, targetFile: string, options: types.Options = {}):
 Promise<types.InspectResult> {
+  if (!targetFile ) {
+    throw MissingTargetFileError(root);
+  }
   const isLockFileBased = (targetFile.endsWith('package-lock.json') || targetFile.endsWith('yarn.lock'));
 
   const getLockFileDeps = isLockFileBased && !options.traverseNodeModules;

src/lib/plugins/rubygems/index.ts

@@ -1,5 +1,6 @@
 import {inspectors, Spec} from './inspectors';
 import * as types from '../types';
+import {MissingTargetFileError} from '../../errors/missing-targetfile-error';
 
 interface RubyGemsInspectResult extends types.InspectResult {
   package: {
@@ -10,6 +11,9 @@ interface RubyGemsInspectResult extends types.InspectResult {
 }
 
 export async function inspect(root: string, targetFile: string): Promise<RubyGemsInspectResult> {
+  if (!targetFile ) {
+    throw MissingTargetFileError(root);
+  }
   const specs = await gatherSpecs(root, targetFile);
 
   return {

test/acceptance/sln-app.test.js → test/acceptance/sln-app.test.ts

@@ -1,33 +1,32 @@
-var test = require('tap').test;
-var path = require('path');
-var sln = require('../../src/lib/sln');
+import {test} from 'tap';
+import * as path from 'path';
+import * as sln from '../../src/lib/sln';
 
-test('parseFoldersFromSln when passed an existant filename', function (t) {
-  var slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution.sln';
-  var expected = JSON.stringify([
-    "dotnet2_new_mvc_project/new_mvc_project.csproj",
-    "WebApplication2/WebApplication2.csproj"
+test('parseFoldersFromSln when passed an existent filename', (t) => {
+  const slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution.sln';
+  const expected = JSON.stringify([
+    'dotnet2_new_mvc_project/new_mvc_project.csproj',
+    'WebApplication2/WebApplication2.csproj',
   ]);
-  var actual = JSON.stringify(sln.parsePathsFromSln(slnFile));
+  const actual = JSON.stringify(sln.parsePathsFromSln(slnFile));
   t.equal(actual, expected, 'should parse & extract csproj folders');
   t.end();
 });
 
-test('parseFoldersFromSln when non existant filename', function (t) {
-  var response;
-  var slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution1.sln';
+test('parseFoldersFromSln when non existent filename', (t) => {
+  let response;
+  const slnFile = 'test/acceptance/workspaces/sln-example-app/mySolution1.sln';
   try {
     response = sln.parsePathsFromSln(slnFile);
     t.fail('an exception should be thrown');
-  }
-  catch (e) {
+  } catch (e) {
     t.match(e.message, 'File not found: ', 'should throw exception');
     t.equal(response, undefined, 'shouldnt return');
   }
   t.end();
 });
 
-test('parseFoldersFromSln when no supported files found', function (t) {
+test('parseFoldersFromSln when no supported files found', (t) => {
   let response;
   const slnFile = 'test/acceptance/workspaces/sln-no-supported-files/mySolution1.sln';
   try {
@@ -40,32 +39,32 @@ test('parseFoldersFromSln when no supported files found', function (t) {
   t.end();
 });
 
-test('sln.updateArgs for existing sln with regular paths', function (t) {
-  var args = {options: {
+test('sln.updateArgs for existing sln with regular paths', (t) => {
+  const args = {options: {
     file: 'test/acceptance/workspaces/sln-example-app/mySolution.sln', _: []}};
 
   sln.updateArgs(args);
   t.notOk(args.options.file, '`file` option is removed');
   args.options._.pop();
-  t.same(args.options._.map(function (r) { return path.basename(r); }),
+  t.same(args.options._.map((r) => path.basename(r)),
     [ 'dotnet2_new_mvc_project', 'WebApplication2' ], 'args should be added');
   t.end();
 });
 
-test('sln.updateArgs for existing sln with relative paths', function (t) {
-  var args = {options: {
+test('sln.updateArgs for existing sln with relative paths', (t) => {
+  const args = {options: {
     file: 'test/acceptance/workspaces/slnSolution.sln', _: []}};
 
   sln.updateArgs(args);
   t.notOk(args.options.file, '`file` option is removed');
   args.options._.pop();
-  t.same(args.options._.map(function (r) { return path.basename(r); }),
+  t.same(args.options._.map((r) => path.basename(r)),
     [  'nuget-app', 'nuget-app-2.1'], 'args should be added');
   t.end();
 });
 
-test('sln.updateArgs for sln with no relevant projects', function (t) {
-  var args = {options: {
+test('sln.updateArgs for sln with no relevant projects', (t) => {
+  const args = {options: {
     file: 'test/acceptance/workspaces/emptySolution.sln', _: []}};
 
   try {
@@ -78,9 +77,22 @@ test('sln.updateArgs for sln with no relevant projects', function (t) {
   t.end();
 });
 
+test('sln.updateArgs for sln without --file', (t) => {
+  const args = {options: {
+    'test/acceptance/workspaces/emptySolution.sln': ''}};
+
+  try {
+    sln.updateArgs(args);
+    t.fail('should have exploded');
+  } catch (e) {
+    t.equal(e.message, 'No relevant projects found in Solution',
+      'Error thrown on solution with no valid projects');
+  }
+  t.end();
+});
 
-test('sln.updateArgs for non-existing sln', function (t) {
-  var args = {options: {file: 'non_existent', _: []}};
+test('sln.updateArgs for non-existing sln', (t) => {
+  const args = {options: {file: 'non_existent', _: []}};
 
   try {
     sln.updateArgs(args);