From dc06df31b6b69a6f5d2d2fceaddf82bda7a66fc2 Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Fri, 9 Dec 2022 20:36:43 +0900 Subject: [PATCH] readline: refactor to use `validateNumber` `validateNumber` throws more proper error code and error name. PR-URL: /~https://github.com/nodejs/node/pull/45801 Reviewed-By: Luigi Pinca Reviewed-By: Minwoo Jung Reviewed-By: James M Snell Reviewed-By: Filip Skokan Reviewed-By: Antoine du Hamel Reviewed-By: Rafael Gonzaga --- lib/internal/readline/interface.js | 10 ++-------- test/parallel/test-readline-interface.js | 17 +++++++++++++++-- .../test-readline-promises-interface.js | 17 +++++++++++++++-- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/lib/internal/readline/interface.js b/lib/internal/readline/interface.js index 8822073f971ef7..37617c4c56cf54 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -19,7 +19,6 @@ const { MathMax, MathMaxApply, NumberIsFinite, - NumberIsNaN, ObjectSetPrototypeOf, RegExpPrototypeExec, StringPrototypeCodePointAt, @@ -42,6 +41,7 @@ const { const { validateAbortSignal, validateArray, + validateNumber, validateString, validateUint32, } = require('internal/validators'); @@ -196,13 +196,7 @@ function InterfaceConstructor(input, output, completer, terminal) { historySize = kHistorySize; } - if ( - typeof historySize !== 'number' || - NumberIsNaN(historySize) || - historySize < 0 - ) { - throw new ERR_INVALID_ARG_VALUE.RangeError('historySize', historySize); - } + validateNumber(historySize, 'historySize', 0); // Backwards compat; check the isTTY prop of the output stream // when `terminal` was not specified diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index b5a3b0499fcdbd..1e02c34b225481 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -126,7 +126,7 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); // Constructor throws if historySize is not a positive number - ['not a number', -1, NaN, {}, true, Symbol(), null].forEach((historySize) => { + [-1, NaN].forEach((historySize) => { assert.throws(() => { readline.createInterface({ input, @@ -134,7 +134,20 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); }, { name: 'RangeError', - code: 'ERR_INVALID_ARG_VALUE' + code: 'ERR_OUT_OF_RANGE', + }); + }); + + // Constructor throws if type of historySize is not a number + ['not a number', {}, true, Symbol(), null].forEach((historySize) => { + assert.throws(() => { + readline.createInterface({ + input, + historySize, + }); + }, { + name: 'TypeError', + code: 'ERR_INVALID_ARG_TYPE', }); }); diff --git a/test/parallel/test-readline-promises-interface.js b/test/parallel/test-readline-promises-interface.js index 2a211025cdb27b..2a8c5aae4e3949 100644 --- a/test/parallel/test-readline-promises-interface.js +++ b/test/parallel/test-readline-promises-interface.js @@ -103,7 +103,7 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); // Constructor throws if historySize is not a positive number - ['not a number', -1, NaN, {}, true, Symbol(), null].forEach((historySize) => { + [-1, NaN].forEach((historySize) => { assert.throws(() => { readline.createInterface({ input, @@ -111,7 +111,20 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); }, { name: 'RangeError', - code: 'ERR_INVALID_ARG_VALUE' + code: 'ERR_OUT_OF_RANGE', + }); + }); + + // Constructor throws if type of historySize is not a number + ['not a number', {}, true, Symbol(), null].forEach((historySize) => { + assert.throws(() => { + readline.createInterface({ + input, + historySize, + }); + }, { + name: 'TypeError', + code: 'ERR_INVALID_ARG_TYPE', }); });