From 1f0453a9d9bb1560eab422b20f6595c82767e3e2 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 22 May 2020 14:17:52 +0200 Subject: [PATCH 1/5] console: remove dead code This check is always truthy. Thus, it's removed. Signed-off-by: Ruben Bridgewater --- lib/internal/console/constructor.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js index c5195c77c6e169..a6308c20369bb7 100644 --- a/lib/internal/console/constructor.js +++ b/lib/internal/console/constructor.js @@ -492,7 +492,7 @@ const consoleMethods = { if (setIter) tabularData = previewEntries(tabularData); - const setlike = setIter || (mapIter && !isKeyValue) || isSet(tabularData); + const setlike = setIter || mapIter || isSet(tabularData); if (setlike) { const values = []; let length = 0; From 60b8282518cf6220cae2ae7f16351d6cabf21b89 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 22 May 2020 14:04:51 +0200 Subject: [PATCH 2/5] console: mark special console properties as non-enumerable This makes sure internal console properties are not visible during default inspection. They are still visible when inspecting the console with `showHidden` set to `true`. These properties are confusing while working with the REPL and easily show up. Signed-off-by: Ruben Bridgewater --- lib/internal/console/constructor.js | 276 +++++++++++++++------------- 1 file changed, 147 insertions(+), 129 deletions(-) diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js index a6308c20369bb7..bdb8a84b94c4af 100644 --- a/lib/internal/console/constructor.js +++ b/lib/internal/console/constructor.js @@ -155,68 +155,156 @@ ObjectDefineProperty(Console, SymbolHasInstance, { } }); -// Eager version for the Console constructor -Console.prototype[kBindStreamsEager] = function(stdout, stderr) { - ObjectDefineProperties(this, { - '_stdout': { ...consolePropAttributes, value: stdout }, - '_stderr': { ...consolePropAttributes, value: stderr } - }); -}; +const kColorInspectOptions = { colors: true }; +const kNoColorInspectOptions = {}; -// Lazily load the stdout and stderr from an object so we don't -// create the stdio streams when they are not even accessed -Console.prototype[kBindStreamsLazy] = function(object) { - let stdout; - let stderr; - ObjectDefineProperties(this, { - '_stdout': { - enumerable: false, - configurable: true, - get() { - if (!stdout) stdout = object.stdout; - return stdout; - }, - set(value) { stdout = value; } - }, - '_stderr': { - enumerable: false, - configurable: true, - get() { - if (!stderr) { stderr = object.stderr; } - return stderr; - }, - set(value) { stderr = value; } +ObjectDefineProperties(Console.prototype, { + [kBindStreamsEager]: { + ...consolePropAttributes, + // Eager version for the Console constructor + value: function(stdout, stderr) { + ObjectDefineProperties(this, { + '_stdout': { ...consolePropAttributes, value: stdout }, + '_stderr': { ...consolePropAttributes, value: stderr } + }); } - }); -}; + }, + [kBindStreamsLazy]: { + ...consolePropAttributes, + // Lazily load the stdout and stderr from an object so we don't + // create the stdio streams when they are not even accessed + value: function(object) { + let stdout; + let stderr; + ObjectDefineProperties(this, { + '_stdout': { + enumerable: false, + configurable: true, + get() { + if (!stdout) stdout = object.stdout; + return stdout; + }, + set(value) { stdout = value; } + }, + '_stderr': { + enumerable: false, + configurable: true, + get() { + if (!stderr) { stderr = object.stderr; } + return stderr; + }, + set(value) { stderr = value; } + } + }); + } + }, + [kBindProperties]: { + ...consolePropAttributes, + value: function(ignoreErrors, colorMode, groupIndentation = 2) { + ObjectDefineProperties(this, { + '_stdoutErrorHandler': { + ...consolePropAttributes, + value: createWriteErrorHandler(this, kUseStdout) + }, + '_stderrErrorHandler': { + ...consolePropAttributes, + value: createWriteErrorHandler(this, kUseStderr) + }, + '_ignoreErrors': { + ...consolePropAttributes, + value: Boolean(ignoreErrors) + }, + '_times': { ...consolePropAttributes, value: new Map() }, + // Corresponds to https://console.spec.whatwg.org/#count-map + [kCounts]: { ...consolePropAttributes, value: new Map() }, + [kColorMode]: { ...consolePropAttributes, value: colorMode }, + [kIsConsole]: { ...consolePropAttributes, value: true }, + [kGroupIndent]: { ...consolePropAttributes, value: '' }, + [kGroupIndentationWidth]: { + ...consolePropAttributes, + value: groupIndentation + }, + }); + } + }, + [kWriteToConsole]: { + ...consolePropAttributes, + value: function(streamSymbol, string) { + const ignoreErrors = this._ignoreErrors; + const groupIndent = this[kGroupIndent]; + + const useStdout = streamSymbol === kUseStdout; + const stream = useStdout ? this._stdout : this._stderr; + const errorHandler = useStdout ? + this._stdoutErrorHandler : this._stderrErrorHandler; + + if (groupIndent.length !== 0) { + if (string.includes('\n')) { + string = string.replace(/\n/g, `\n${groupIndent}`); + } + string = groupIndent + string; + } + string += '\n'; + + if (ignoreErrors === false) return stream.write(string); + + // There may be an error occurring synchronously (e.g. for files or TTYs + // on POSIX systems) or asynchronously (e.g. pipes on POSIX systems), so + // handle both situations. + try { + // Add and later remove a noop error handler to catch synchronous + // errors. + if (stream.listenerCount('error') === 0) + stream.once('error', noop); + + stream.write(string, errorHandler); + } catch (e) { + // Console is a debugging utility, so it swallowing errors is not + // desirable even in edge cases such as low stack space. + if (isStackOverflowError(e)) + throw e; + // Sorry, there's no proper way to pass along the error here. + } finally { + stream.removeListener('error', noop); + } + } + }, + [kGetInspectOptions]: { + ...consolePropAttributes, + value: function(stream) { + let color = this[kColorMode]; + if (color === 'auto') { + color = stream.isTTY && ( + typeof stream.getColorDepth === 'function' ? + stream.getColorDepth() > 2 : true); + } -Console.prototype[kBindProperties] = function(ignoreErrors, colorMode, - groupIndentation = 2) { - ObjectDefineProperties(this, { - '_stdoutErrorHandler': { - ...consolePropAttributes, - value: createWriteErrorHandler(this, kUseStdout) - }, - '_stderrErrorHandler': { - ...consolePropAttributes, - value: createWriteErrorHandler(this, kUseStderr) - }, - '_ignoreErrors': { - ...consolePropAttributes, - value: Boolean(ignoreErrors) - }, - '_times': { ...consolePropAttributes, value: new Map() }, - // Corresponds to https://console.spec.whatwg.org/#count-map - [kCounts]: { ...consolePropAttributes, value: new Map() }, - [kColorMode]: { ...consolePropAttributes, value: colorMode }, - [kIsConsole]: { ...consolePropAttributes, value: true }, - [kGroupIndent]: { ...consolePropAttributes, value: '' }, - [kGroupIndentationWidth]: { - ...consolePropAttributes, - value: groupIndentation - }, - }); -}; + const options = optionsMap.get(this); + if (options) { + if (options.colors === undefined) { + options.colors = color; + } + return options; + } + + return color ? kColorInspectOptions : kNoColorInspectOptions; + } + }, + [kFormatForStdout]: { + ...consolePropAttributes, + value: function(args) { + const opts = this[kGetInspectOptions](this._stdout); + return formatWithOptions(opts, ...args); + } + }, + [kFormatForStderr]: { + ...consolePropAttributes, + value: function(args) { + const opts = this[kGetInspectOptions](this._stderr); + return formatWithOptions(opts, ...args); + } + }, +}); // Make a function that can serve as the callback passed to `stream.write()`. function createWriteErrorHandler(instance, streamSymbol) { @@ -239,76 +327,6 @@ function createWriteErrorHandler(instance, streamSymbol) { }; } -Console.prototype[kWriteToConsole] = function(streamSymbol, string) { - const ignoreErrors = this._ignoreErrors; - const groupIndent = this[kGroupIndent]; - - const useStdout = streamSymbol === kUseStdout; - const stream = useStdout ? this._stdout : this._stderr; - const errorHandler = useStdout ? - this._stdoutErrorHandler : this._stderrErrorHandler; - - if (groupIndent.length !== 0) { - if (string.includes('\n')) { - string = string.replace(/\n/g, `\n${groupIndent}`); - } - string = groupIndent + string; - } - string += '\n'; - - if (ignoreErrors === false) return stream.write(string); - - // There may be an error occurring synchronously (e.g. for files or TTYs - // on POSIX systems) or asynchronously (e.g. pipes on POSIX systems), so - // handle both situations. - try { - // Add and later remove a noop error handler to catch synchronous errors. - if (stream.listenerCount('error') === 0) - stream.once('error', noop); - - stream.write(string, errorHandler); - } catch (e) { - // Console is a debugging utility, so it swallowing errors is not desirable - // even in edge cases such as low stack space. - if (isStackOverflowError(e)) - throw e; - // Sorry, there's no proper way to pass along the error here. - } finally { - stream.removeListener('error', noop); - } -}; - -const kColorInspectOptions = { colors: true }; -const kNoColorInspectOptions = {}; -Console.prototype[kGetInspectOptions] = function(stream) { - let color = this[kColorMode]; - if (color === 'auto') { - color = stream.isTTY && ( - typeof stream.getColorDepth === 'function' ? - stream.getColorDepth() > 2 : true); - } - - const options = optionsMap.get(this); - if (options) { - if (options.colors === undefined) { - options.colors = color; - } - return options; - } - - return color ? kColorInspectOptions : kNoColorInspectOptions; -}; - -Console.prototype[kFormatForStdout] = function(args) { - const opts = this[kGetInspectOptions](this._stdout); - return formatWithOptions(opts, ...args); -}; - -Console.prototype[kFormatForStderr] = function(args) { - const opts = this[kGetInspectOptions](this._stderr); - return formatWithOptions(opts, ...args); -}; - const consoleMethods = { log(...args) { this[kWriteToConsole](kUseStdout, this[kFormatForStdout](args)); From ac845736ccaa050e29c0789dbb992c85e83e2dc9 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Fri, 22 May 2020 14:02:04 +0200 Subject: [PATCH 3/5] console: name console functions appropriately The current name of most of the global console functions is "bound consoleCall". This is changed to the actual functions name e.g., "log" or "error". Signed-off-by: Ruben Bridgewater --- lib/internal/console/constructor.js | 3 +++ lib/internal/console/global.js | 2 ++ lib/internal/util/inspector.js | 4 ++++ test/parallel/test-console-methods.js | 7 ++++-- test/parallel/test-repl.js | 31 +++++++++++++++++++++++++++ 5 files changed, 45 insertions(+), 2 deletions(-) diff --git a/lib/internal/console/constructor.js b/lib/internal/console/constructor.js index bdb8a84b94c4af..458a5cd2738eea 100644 --- a/lib/internal/console/constructor.js +++ b/lib/internal/console/constructor.js @@ -136,6 +136,9 @@ function Console(options /* or: stdout, stderr, ignoreErrors = true */) { // the prototype so that users extending the Console can override them // from the prototype chain of the subclass. this[key] = this[key].bind(this); + ObjectDefineProperty(this[key], 'name', { + value: key + }); } this[kBindStreamsEager](stdout, stderr); diff --git a/lib/internal/console/global.js b/lib/internal/console/global.js index 6a1dc3806fdb0b..1c615c78451510 100644 --- a/lib/internal/console/global.js +++ b/lib/internal/console/global.js @@ -36,7 +36,9 @@ for (const prop of ReflectOwnKeys(Console.prototype)) { if (prop === 'constructor') { continue; } const desc = ReflectGetOwnPropertyDescriptor(Console.prototype, prop); if (typeof desc.value === 'function') { // fix the receiver + const name = desc.value.name; desc.value = desc.value.bind(globalConsole); + ReflectDefineProperty(desc.value, 'name', { value: name }); } ReflectDefineProperty(globalConsole, prop, desc); } diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js index 5a95bcf8ea852a..8d413b116fd0f2 100644 --- a/lib/internal/util/inspector.js +++ b/lib/internal/util/inspector.js @@ -1,6 +1,7 @@ 'use strict'; const { + ObjectDefineProperty, ObjectKeys, } = primordials; @@ -42,6 +43,9 @@ function wrapConsole(consoleFromNode, consoleFromVM) { consoleFromNode[key] = consoleCall.bind(consoleFromNode, consoleFromVM[key], consoleFromNode[key]); + ObjectDefineProperty(consoleFromNode[key], 'name', { + value: key + }); } else { // Add additional console APIs from the inspector consoleFromNode[key] = consoleFromVM[key]; diff --git a/test/parallel/test-console-methods.js b/test/parallel/test-console-methods.js index 00dc144761cb57..ae580e79ad3d60 100644 --- a/test/parallel/test-console-methods.js +++ b/test/parallel/test-console-methods.js @@ -1,8 +1,8 @@ 'use strict'; require('../common'); -// This test ensures that console methods -// cannot be invoked as constructors +// This test ensures that console methods cannot be invoked as constructors and +// that their name is always correct. const assert = require('assert'); @@ -33,6 +33,9 @@ const methods = [ ]; for (const method of methods) { + assert.strictEqual(console[method].name, method); + assert.strictEqual(newInstance[method].name, method); + assert.throws(() => new console[method](), err); assert.throws(() => new newInstance[method](), err); assert.throws(() => Reflect.construct({}, [], console[method]), err); diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index cb66c22457fdc2..e78407c718239e 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -754,6 +754,37 @@ const errorTests = [ /^Uncaught SyntaxError: / ] }, + { + send: 'console', + expect: [ + '{', + ' log: [Function: log],', + ' warn: [Function: warn],', + ' dir: [Function: dir],', + ' time: [Function: time],', + ' timeEnd: [Function: timeEnd],', + ' timeLog: [Function: timeLog],', + ' trace: [Function: trace],', + ' assert: [Function: assert],', + ' clear: [Function: clear],', + ' count: [Function: count],', + ' countReset: [Function: countReset],', + ' group: [Function: group],', + ' groupEnd: [Function: groupEnd],', + ' table: [Function: table],', + ' debug: [Function: debug],', + ' info: [Function: info],', + ' dirxml: [Function: dirxml],', + ' error: [Function: error],', + ' groupCollapsed: [Function: groupCollapsed],', + ' Console: [Function: Console],', + ' profile: [Function: profile],', + ' profileEnd: [Function: profileEnd],', + ' timeStamp: [Function: timeStamp],', + ' context: [Function: context]', + '}', + ] + }, ]; const tcpTests = [ From 06defb00c84ac300f4acdef91e177f9554d06bac Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 23 May 2020 15:32:44 +0200 Subject: [PATCH 4/5] fixup! console: name console functions appropriately Robuster tests (e.g, no inspector) Signed-off-by: Ruben Bridgewater --- test/parallel/test-console-methods.js | 24 ++++++++++++++++++++++-- test/parallel/test-repl.js | 20 +++++++++++--------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/test/parallel/test-console-methods.js b/test/parallel/test-console-methods.js index ae580e79ad3d60..d338cc1f807f2c 100644 --- a/test/parallel/test-console-methods.js +++ b/test/parallel/test-console-methods.js @@ -32,9 +32,29 @@ const methods = [ 'groupCollapsed', ]; +const alternateNames = { + debug: 'log', + info: 'log', + dirxml: 'log', + error: 'warn', + groupCollapsed: 'group' +}; + +function assertEqualName(method) { + try { + assert.strictEqual(console[method].name, method); + } catch { + assert.strictEqual(console[method].name, alternateNames[method]); + } + try { + assert.strictEqual(newInstance[method].name, method); + } catch { + assert.strictEqual(newInstance[method].name, alternateNames[method]); + } +} + for (const method of methods) { - assert.strictEqual(console[method].name, method); - assert.strictEqual(newInstance[method].name, method); + assertEqualName(method); assert.throws(() => new console[method](), err); assert.throws(() => new newInstance[method](), err); diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index e78407c718239e..1119f881610170 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -772,16 +772,18 @@ const errorTests = [ ' group: [Function: group],', ' groupEnd: [Function: groupEnd],', ' table: [Function: table],', - ' debug: [Function: debug],', - ' info: [Function: info],', - ' dirxml: [Function: dirxml],', - ' error: [Function: error],', - ' groupCollapsed: [Function: groupCollapsed],', + / debug: \[Function: (debug|log)],/, + / info: \[Function: (info|log)],/, + / dirxml: \[Function: (dirxml|log)],/, + / error: \[Function: (error|warn)],/, + / groupCollapsed: \[Function: (groupCollapsed|group)],/, ' Console: [Function: Console],', - ' profile: [Function: profile],', - ' profileEnd: [Function: profileEnd],', - ' timeStamp: [Function: timeStamp],', - ' context: [Function: context]', + ...process.features.inspector ? [ + ' profile: [Function: profile],', + ' profileEnd: [Function: profileEnd],', + ' timeStamp: [Function: timeStamp],', + ' context: [Function: context]', + ] : [], '}', ] }, From dab9b92d053fe68639f328e91e0f8c423a0e4fdc Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Sat, 23 May 2020 22:49:20 +0200 Subject: [PATCH 5/5] fixup! console: name console functions appropriately Signed-off-by: Ruben Bridgewater --- test/parallel/test-repl.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 1119f881610170..b653e27750b27d 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -777,7 +777,7 @@ const errorTests = [ / dirxml: \[Function: (dirxml|log)],/, / error: \[Function: (error|warn)],/, / groupCollapsed: \[Function: (groupCollapsed|group)],/, - ' Console: [Function: Console],', + / Console: \[Function: Console],?/, ...process.features.inspector ? [ ' profile: [Function: profile],', ' profileEnd: [Function: profileEnd],',