From f0f272b9951927182e68bb05848f1d419174ff8b Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Fri, 9 Dec 2022 20:36:43 +0900 Subject: [PATCH 1/2] readline: refactor to use `validateNumber` `validateNumber` throws more proper error code and error name. --- 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 5de6a8fe03da13..9ca53e9f6d337d 100644 --- a/lib/internal/readline/interface.js +++ b/lib/internal/readline/interface.js @@ -19,7 +19,6 @@ const { MathMax, MathMaxApply, NumberIsFinite, - NumberIsNaN, ObjectSetPrototypeOf, RegExpPrototypeExec, RegExpPrototypeSymbolReplace, @@ -45,6 +44,7 @@ const { const { validateAbortSignal, validateArray, + validateNumber, validateString, validateUint32, } = require('internal/validators'); @@ -199,13 +199,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..707c651a244d04 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..99e02462312b9a 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' }); }); From 6dc2c145b182cdd9c11aed000071e36d13aa2356 Mon Sep 17 00:00:00 2001 From: Deokjin Kim Date: Wed, 18 Jan 2023 19:34:25 +0900 Subject: [PATCH 2/2] add trailing commas Co-authored-by: Antoine du Hamel --- test/parallel/test-readline-interface.js | 4 ++-- test/parallel/test-readline-promises-interface.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-readline-interface.js b/test/parallel/test-readline-interface.js index 707c651a244d04..1e02c34b225481 100644 --- a/test/parallel/test-readline-interface.js +++ b/test/parallel/test-readline-interface.js @@ -134,7 +134,7 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); }, { name: 'RangeError', - code: 'ERR_OUT_OF_RANGE' + code: 'ERR_OUT_OF_RANGE', }); }); @@ -147,7 +147,7 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); }, { name: 'TypeError', - code: 'ERR_INVALID_ARG_TYPE' + code: 'ERR_INVALID_ARG_TYPE', }); }); diff --git a/test/parallel/test-readline-promises-interface.js b/test/parallel/test-readline-promises-interface.js index 99e02462312b9a..2a8c5aae4e3949 100644 --- a/test/parallel/test-readline-promises-interface.js +++ b/test/parallel/test-readline-promises-interface.js @@ -111,7 +111,7 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); }, { name: 'RangeError', - code: 'ERR_OUT_OF_RANGE' + code: 'ERR_OUT_OF_RANGE', }); }); @@ -124,7 +124,7 @@ function assertCursorRowsAndCols(rli, rows, cols) { }); }, { name: 'TypeError', - code: 'ERR_INVALID_ARG_TYPE' + code: 'ERR_INVALID_ARG_TYPE', }); });