feat(options): initial support for long options

This adds initial support for long options. This integration tests them
with `shell.touch()` and unit tests them in `test/common.js`. This
documents the new syntax.

This also refactors some of the common internals:

 * Creates a new CommandError type to replace the `'earlyExit'` hack
 * Clearer Error types for `parseOptions()` exceptions
 * Removes side effects from a test which modifies `common.config`
 * Fixes appveyor and travis config to run lint (regressed in #920)

Issue #924
Test: touch.js, common.js

.travis.yml

@@ -13,6 +13,7 @@ os:
   - osx
 script:
   - npm run test-with-coverage
+  - npm run lint
   # make sure when the docs are generated nothing changes (a.k.a. the docs have already been generated)
   - npm run gendocs
   - npm run check-node-support

README.md

@@ -647,16 +647,18 @@ Available options:
 + `-a`: Change only the access time
 + `-c`: Do not create any files
 + `-m`: Change only the modification time
-+ `{'-d': date}`: Use `date` (instance of `Date`) instead of current time
-+ `{'-r': file}`: Use `file`'s times instead of current time
++ `{'-d': someDate}`, `{date: someDate}`: Use `someDate` (instance of
+  `Date`) instead of current time
++ `{'-r': file}`, `{reference: file}`: Use `file`'s times instead of current
+   time
 
 Examples:
 
 ```javascript
 touch('source.js');
 touch('-c', 'path/to/file.js');
 touch({ '-r': 'referenceFile.txt' }, 'path/to/file.js');
-touch({ '-d': new Date('December 17, 1995 03:24:00') }, 'path/to/file.js');
+touch({ date: new Date('December 17, 1995 03:24:00') }, 'path/to/file.js');
 ```
 
 Update the access and modification times of each file to the current time.

appveyor.yml

@@ -25,6 +25,7 @@ build: off
 
 test_script:
   - npm run test-with-coverage
+  - npm run lint
 
 on_success:
   - npm run codecov -- -f coverage/lcov.info

src/common.js

@@ -77,6 +77,15 @@ function convertErrorOutput(msg) {
 }
 exports.convertErrorOutput = convertErrorOutput;
 
+// An exception class to help propagate command errors (e.g., non-zero exit
+// status) up to the top-level. {@param value} should be a ShellString.
+function CommandError(value) {
+  this.returnValue = value;
+}
+CommandError.prototype = Object.create(Error.prototype);
+CommandError.prototype.constructor = CommandError;
+exports.CommandError = CommandError; // visible for testing
+
 // Shows error message. Throws if config.fatal is true
 function error(msg, _code, options) {
   // Validate input
@@ -111,10 +120,7 @@ function error(msg, _code, options) {
   if (msg.length > 0 && !options.silent) log(logEntry);
 
   if (!options.continue) {
-    throw {
-      msg: 'earlyExit',
-      retValue: (new ShellString('', state.error, state.errorCode)),
-    };
+    throw new CommandError(new ShellString('', state.error, state.errorCode));
   }
 }
 exports.error = error;
@@ -161,11 +167,12 @@ exports.ShellString = ShellString;
 function parseOptions(opt, map, errorOptions) {
   // Validate input
   if (typeof opt !== 'string' && !isObject(opt)) {
-    throw new Error('options must be strings or key-value pairs');
+    throw new TypeError('options must be strings or key-value pairs');
   } else if (!isObject(map)) {
-    throw new Error('parseOptions() internal error: map must be an object');
+    throw new TypeError('parseOptions() internal error: map must be an object');
   } else if (errorOptions && !isObject(errorOptions)) {
-    throw new Error('parseOptions() internal error: errorOptions must be object');
+    throw new TypeError(
+        'parseOptions() internal error: errorOptions must be object');
   }
 
   if (opt === '--') {
@@ -206,13 +213,23 @@ function parseOptions(opt, map, errorOptions) {
     });
   } else { // opt is an Object
     Object.keys(opt).forEach(function (key) {
-      // key is a string of the form '-r', '-d', etc.
-      var c = key[1];
-      if (c in map) {
-        var optionName = map[c];
-        options[optionName] = opt[key]; // assign the given value
+      if (key[0] === '-') {
+        // key is a string of the form '-r', '-d', etc.
+        var c = key[1];
+        if (c in map) {
+          var optionName = map[c];
+          options[optionName] = opt[key]; // assign the given value
+        } else {
+          error('option not recognized: ' + c, errorOptions || {});
+        }
       } else {
-        error('option not recognized: ' + c, errorOptions || {});
+        if (key in options) {
+          // key is a "long option", so it should be the same
+          options[key] = opt[key];
+        } else {
+          error('option not recognized: {' + key + ':...}',
+              errorOptions || {});
+        }
       }
     });
   }
@@ -384,8 +401,8 @@ function wrap(cmd, fn, options) {
           retValue = fn.apply(this, args);
         } catch (e) {
           /* istanbul ignore else */
-          if (e.msg === 'earlyExit') {
-            retValue = e.retValue;
+          if (e instanceof CommandError) {
+            retValue = e.returnValue;
           } else {
             throw e; // this is probably a bug that should be thrown up the call stack
           }

src/touch.js

@@ -20,16 +20,18 @@ common.register('touch', _touch, {
 //@ + `-a`: Change only the access time
 //@ + `-c`: Do not create any files
 //@ + `-m`: Change only the modification time
-//@ + `{'-d': date}`: Use `date` (instance of `Date`) instead of current time
-//@ + `{'-r': file}`: Use `file`'s times instead of current time
+//@ + `{'-d': someDate}`, `{date: someDate}`: Use `someDate` (instance of
+//@   `Date`) instead of current time
+//@ + `{'-r': file}`, `{reference: file}`: Use `file`'s times instead of current
+//@    time
 //@
 //@ Examples:
 //@
 //@ ```javascript
 //@ touch('source.js');
 //@ touch('-c', 'path/to/file.js');
 //@ touch({ '-r': 'referenceFile.txt' }, 'path/to/file.js');
-//@ touch({ '-d': new Date('December 17, 1995 03:24:00') }, 'path/to/file.js');
+//@ touch({ date: new Date('December 17, 1995 03:24:00') }, 'path/to/file.js');
 //@ ```
 //@
 //@ Update the access and modification times of each file to the current time.

test/common.js

@@ -4,13 +4,16 @@ import shell from '..';
 import common from '../src/common';
 import utils from './utils/utils';
 
-shell.config.silent = true;
-
 test.beforeEach(() => {
+  shell.config.silent = true;
   common.state.error = null;
   common.state.errorCode = 0;
 });
 
+test.afterEach(() => {
+  common.config.resetForTesting();
+});
+
 //
 // Invalids
 //
@@ -34,45 +37,45 @@ test('parseOptions (invalid option in options object)', t => {
       f: 'force',
       r: 'reverse',
     });
-  });
+  }, common.CommandError);
 });
 
 test('parseOptions (without a hyphen in the string)', t => {
   t.throws(() => {
     common.parseOptions('f', {
       f: 'force',
     });
-  });
+  }, Error);
 });
 
 test('parseOptions (opt is not a string/object)', t => {
   t.throws(() => {
     common.parseOptions(1, {
       f: 'force',
     });
-  });
+  }, TypeError);
 });
 
 test('parseOptions (map is not an object)', t => {
   t.throws(() => {
     common.parseOptions('-f', 27);
-  });
+  }, TypeError);
 });
 
 test('parseOptions (errorOptions is not an object)', t => {
   t.throws(() => {
     common.parseOptions('-f', {
       f: 'force',
     }, 'not a valid errorOptions');
-  });
+  }, TypeError);
 });
 
 test('parseOptions (unrecognized string option)', t => {
   t.throws(() => {
     common.parseOptions('-z', {
       f: 'force',
     });
-  });
+  }, common.CommandError);
 });
 
 test('parseOptions (unrecognized option in Object)', t => {
@@ -200,7 +203,6 @@ test('common.buffer with different config.bufLength', t => {
   const buf = common.buffer();
   t.truthy(buf instanceof Buffer);
   t.is(buf.length, 20);
-  common.config.reset();
 });
 
 test('common.parseOptions (normal case)', t => {
@@ -264,6 +266,29 @@ test('common.parseOptions throws when passed a string not starting with "-"', t
   }, Error, "Options string must start with a '-'");
 });
 
+test('common.parseOptions allows long options', t => {
+  const result = common.parseOptions({ value: true }, {
+    v: 'value',
+  });
+  t.truthy(result.value);
+});
+
+test('common.parseOptions allows long options with values', t => {
+  const someObject = {};
+  const result = common.parseOptions({ value: someObject }, {
+    v: 'value',
+  });
+  t.is(result.value, someObject);
+});
+
+test('common.parseOptions throws for unknown long option', t => {
+  t.throws(() => {
+    common.parseOptions({ throws: true }, {
+      v: 'value',
+    });
+  }, common.CommandError);
+});
+
 test('common.parseOptions with -- argument', t => {
   const result = common.parseOptions('--', {
     R: 'recursive',
@@ -307,3 +332,10 @@ test('Changing shell.config.execPath does not modify process', t => {
   shell.config.execPath = 'foo';
   t.not(shell.config.execPath, process.execPath);
 });
+
+test('CommandError is a subclass of Error', t => {
+  const e = new common.CommandError(new common.ShellString('some value'));
+  t.truthy(e instanceof common.CommandError);
+  t.truthy(e instanceof Error);
+  t.is(e.constructor, common.CommandError);
+});

test/touch.js

@@ -121,9 +121,18 @@ test('uses a reference file for mtime', t => {
 
 test('accepts -d flag', t => {
   const testFile = tmpFile(t);
-  const oldStat = resetUtimes(testFile);
   const date = new Date('December 17, 1995 03:24:00');
-  const result = shell.touch({'-d': date}, testFile);
+  const result = shell.touch({ '-d': date }, testFile);
+  t.is(result.code, 0);
+  // Compare getTime(), because Date can't be compared with triple-equals.
+  t.is(common.statFollowLinks(testFile).mtime.getTime(), date.getTime());
+  t.is(common.statFollowLinks(testFile).atime.getTime(), date.getTime());
+});
+
+test('accepts long option (date)', t => {
+  const testFile = tmpFile(t);
+  const date = new Date('December 17, 1995 03:24:00');
+  const result = shell.touch({ date }, testFile);
   t.is(result.code, 0);
   // Compare getTime(), because Date can't be compared with triple-equals.
   t.is(common.statFollowLinks(testFile).mtime.getTime(), date.getTime());