fix: null is no longer confused for an object

nfischer · ariporad · commit 2fa739e3f171 · 2016-04-20T15:20:05.000-07:00
This is an issue in the case of `echo(which('fakecmd'))`. This used to succeed
in v0.6 but was broken during the ShellString refactor.
diff --git a/src/common.js b/src/common.js
@@ -268,7 +268,7 @@ function wrap(cmd, fn, options) {
       if (options && options.notUnix) {
         retValue = fn.apply(this, args);
       } else {
-        if (typeof args[0] === 'object' && args[0].constructor.name === 'Object') {
+        if (args[0] instanceof Object && args[0].constructor.name === 'Object') {
           args = args; // object count as options
         } else if (args.length === 0 || typeof args[0] !== 'string' || args[0].length <= 1 || args[0][0] !== '-') {
           args.unshift(''); // only add dummy option if '-option' not already present
@@ -284,7 +284,7 @@ function wrap(cmd, fn, options) {
         }, []);
         // Convert ShellStrings to regular strings
         args = args.map(function(arg) {
-          if (arg.constructor.name === 'String') {
+          if (arg instanceof Object && arg.constructor.name === 'String') {
             return arg.toString();
           } else
             return arg;
diff --git a/test/echo.js b/test/echo.js
@@ -22,17 +22,26 @@ var file = 'tmp/tempscript'+Math.random()+'.js',
     script = 'require(\'../../global.js\'); echo("-asdf", "111");'; // test '-' bug (see issue #20)
 shell.ShellString(script).to(file);
 child.exec(JSON.stringify(process.execPath)+' '+file, function(err, stdout) {
-  assert.ok(stdout === '-asdf 111\n' || stdout === '-asdf 111\nundefined\n'); // 'undefined' for v0.4
+  assert.equal(stdout, '-asdf 111\n');
 
-  // simple test with silent(true)
-  shell.mkdir('-p', 'tmp');
-  var file = 'tmp/tempscript'+Math.random()+'.js',
-      script = 'require(\'../../global.js\'); config.silent=true; echo(555);';
+  // using null as an explicit argument doesn't crash the function
+  file = 'tmp/tempscript'+Math.random()+'.js';
+  script = 'require(\'../../global.js\'); echo(null);';
   shell.ShellString(script).to(file);
-  child.exec(JSON.stringify(process.execPath)+' '+file, function(err, stdout) {
-    assert.ok(stdout === '555\n' || stdout === '555\nundefined\n'); // 'undefined' for v0.4
-
-    theEnd();
+  child.exec(JSON.stringify(process.execPath)+' '+file, function(err, stdout, stderr) {
+    assert.equal(stdout, 'null\n');
+    assert.equal(stderr, '');
+
+    // simple test with silent(true)
+    shell.mkdir('-p', 'tmp');
+    var file = 'tmp/tempscript'+Math.random()+'.js',
+        script = 'require(\'../../global.js\'); config.silent=true; echo(555);';
+    shell.ShellString(script).to(file);
+    child.exec(JSON.stringify(process.execPath)+' '+file, function(err, stdout) {
+      assert.equal(stdout, '555\n');
+
+      theEnd();
+    });
   });
 });
 
