process: refactor emitWarning

Remove a couple of obsolete checks by refactoring the code with else. Also remove the possibility of an undefined warning. That is neither documented nor does it result in a usable warning. PR-URL: #20726 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
nodejs · Jul 16, 2018 · 49681e7 · 49681e7
1 parent 43ee4d6
commit 49681e7
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 24 deletions.
diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js
@@ -105,7 +105,7 @@ function setupProcessWarnings() {
   // process.emitWarning(str[, type[, code]][, ctor])
   // process.emitWarning(str[, options])
   process.emitWarning = function(warning, type, code, ctor, now) {
-    var detail;
+    let detail;
     if (type !== null && typeof type === 'object' && !Array.isArray(type)) {
       ctor = type.ctor;
       code = type.code;
@@ -117,23 +117,23 @@ function setupProcessWarnings() {
       code = undefined;
       type = 'Warning';
     }
+    if (type !== undefined && typeof type !== 'string') {
+      throw new ERR_INVALID_ARG_TYPE('type', 'string', type);
+    }
     if (typeof code === 'function') {
       ctor = code;
       code = undefined;
-    }
-    if (code !== undefined && typeof code !== 'string')
+    } else if (code !== undefined && typeof code !== 'string') {
       throw new ERR_INVALID_ARG_TYPE('code', 'string', code);
-    if (type !== undefined && typeof type !== 'string')
-      throw new ERR_INVALID_ARG_TYPE('type', 'string', type);
-    if (warning === undefined || typeof warning === 'string') {
+    }
+    if (typeof warning === 'string') {
       // eslint-disable-next-line no-restricted-syntax
       warning = new Error(warning);
       warning.name = String(type || 'Warning');
       if (code !== undefined) warning.code = code;
       if (detail !== undefined) warning.detail = detail;
       Error.captureStackTrace(warning, ctor || process.emitWarning);
-    }
-    if (!(warning instanceof Error)) {
+    } else if (!(warning instanceof Error)) {
       throw new ERR_INVALID_ARG_TYPE('warning', ['Error', 'string'], warning);
     }
     if (warning.name === 'DeprecationWarning') {

diff --git a/test/parallel/test-process-emitwarning.js b/test/parallel/test-process-emitwarning.js
@@ -43,8 +43,8 @@ class CustomWarning extends Error {
   [testMsg, { type: testType, code: testCode, detail: [] }],
   [testMsg, { type: testType, code: testCode, detail: null }],
   [testMsg, { type: testType, code: testCode, detail: 1 }]
-].forEach((i) => {
-  process.emitWarning.apply(null, i);
+].forEach((args) => {
+  process.emitWarning(...args);
 });
 
 const warningNoToString = new CustomWarning();
@@ -57,18 +57,25 @@ warningThrowToString.toString = function() {
 };
 process.emitWarning(warningThrowToString);
 
-const expectedError =
-  common.expectsError({ code: 'ERR_INVALID_ARG_TYPE', type: TypeError }, 11);
-
 // TypeError is thrown on invalid input
-assert.throws(() => process.emitWarning(1), expectedError);
-assert.throws(() => process.emitWarning({}), expectedError);
-assert.throws(() => process.emitWarning(true), expectedError);
-assert.throws(() => process.emitWarning([]), expectedError);
-assert.throws(() => process.emitWarning('', '', {}), expectedError);
-assert.throws(() => process.emitWarning('', 1), expectedError);
-assert.throws(() => process.emitWarning('', '', 1), expectedError);
-assert.throws(() => process.emitWarning('', true), expectedError);
-assert.throws(() => process.emitWarning('', '', true), expectedError);
-assert.throws(() => process.emitWarning('', []), expectedError);
-assert.throws(() => process.emitWarning('', '', []), expectedError);
+[
+  [1],
+  [{}],
+  [true],
+  [[]],
+  ['', '', {}],
+  ['', 1],
+  ['', '', 1],
+  ['', true],
+  ['', '', true],
+  ['', []],
+  ['', '', []],
+  [],
+  [undefined, 'foo', 'bar'],
+  [undefined]
+].forEach((args) => {
+  common.expectsError(
+    () => process.emitWarning(...args),
+    { code: 'ERR_INVALID_ARG_TYPE', type: TypeError }
+  );
+});