From 0b09032ddd6aa62271bc9589752436efc684fb6c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 22 Oct 2017 11:17:59 -0700 Subject: [PATCH 1/4] test: apply eslint exceptions narrowly In test-util-inspect, apply ESLint exception for accessor-pairs rule narrowly. It had been applied to nearly the whole file, but is only needed for two lines. --- test/parallel/test-util-inspect.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index ecdaf0f1a4d262..5e5e0b89bc868f 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -26,8 +26,6 @@ const JSStream = process.binding('js_stream').JSStream; const util = require('util'); const vm = require('vm'); -/* eslint-disable accessor-pairs */ - assert.strictEqual(util.inspect(1), '1'); assert.strictEqual(util.inspect(false), 'false'); assert.strictEqual(util.inspect(''), "''"); @@ -280,6 +278,7 @@ assert.strictEqual( '{ readwrite: [Getter/Setter] }'); assert.strictEqual( + // eslint-disable-next-line accessor-pairs util.inspect({ set writeonly(val) {} }), '{ writeonly: [Setter] }'); @@ -476,7 +475,7 @@ assert.strictEqual(util.inspect(-0), '-0'); } }); const setter = Object.create(null, { - b: { + b: { // eslint-disable-line accessor-pairs set: function() {} } }); @@ -1151,4 +1150,3 @@ if (typeof Symbol !== 'undefined') { } assert.doesNotThrow(() => util.inspect(process)); -/* eslint-enable accessor-pairs */ From 77038aa6e62ec5c172f8769d9716b3ca90bd54dd Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 22 Oct 2017 14:23:57 -0700 Subject: [PATCH 2/4] doc: improve documentation for util.deprecate() Improve documentation for `util.deprecate()`. In particular, provide complete function signature, document arguments, and document return value. --- doc/api/util.md | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/doc/api/util.md b/doc/api/util.md index 3ba8cade5439e0..db9a31590a6962 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -107,13 +107,20 @@ environment variable set, then it will not print anything. Multiple comma-separated `section` names may be specified in the `NODE_DEBUG` environment variable. For example: `NODE_DEBUG=fs,net,tls`. -## util.deprecate(function, string) +## util.deprecate(fn, msg[, code]) -The `util.deprecate()` method wraps the given `function` or class in such a way that -it is marked as deprecated. +* `fn` {Function} The function that is being deprecated. +* `msg` {string} A warning message to display when the deprecated function is + invoked. +* `code` {string} A deprecation code. See the [list of deprecated APIs][] for a + list of codes. +* Returns: {Function} The deprecated function wrapped to emit a warning. + +The `util.deprecate()` method wraps `fn` (which may be a function or class) in +such a way that it is marked as deprecated. ```js @@ -127,10 +134,9 @@ exports.puts = util.deprecate(function() { ``` When called, `util.deprecate()` will return a function that will emit a -`DeprecationWarning` using the `process.on('warning')` event. By default, -this warning will be emitted and printed to `stderr` exactly once, the first -time it is called. After the warning is emitted, the wrapped `function` -is called. +`DeprecationWarning` using the `process.on('warning')` event. The warning will +be emitted and printed to `stderr` exactly once, the first time it is called. +After the warning is emitted, the wrapped function is called. If either the `--no-deprecation` or `--no-warnings` command line flags are used, or if the `process.noDeprecation` property is set to `true` *prior* to @@ -1213,4 +1219,5 @@ Deprecated predecessor of `console.log`. [Internationalization]: intl.html [WHATWG Encoding Standard]: https://encoding.spec.whatwg.org/ [constructor]: https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Object/constructor +[list of deprecated APIS]: deprecations.html#deprecations_list_of_deprecated_apis [semantically incompatible]: https://github.com/nodejs/node/issues/4179 From 04ffd99ac25176ade9503b73e7bd5c30212f6d37 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 22 Oct 2017 12:26:32 -0700 Subject: [PATCH 3/4] util: emit deprecation code only once If another function has already emitted the deprecation warning with the same code as the warning that is about to be emitted, do not emit the warning. --- doc/api/util.md | 29 ++++++++++---- lib/internal/util.js | 9 ++++- test/parallel/test-util-deprecate.js | 57 ++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-) create mode 100644 test/parallel/test-util-deprecate.js diff --git a/doc/api/util.md b/doc/api/util.md index db9a31590a6962..e29d13bf402ed5 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -110,6 +110,9 @@ environment variable. For example: `NODE_DEBUG=fs,net,tls`. ## util.deprecate(fn, msg[, code]) * `fn` {Function} The function that is being deprecated. @@ -122,21 +125,31 @@ added: v0.8.0 The `util.deprecate()` method wraps `fn` (which may be a function or class) in such a way that it is marked as deprecated. - ```js const util = require('util'); -exports.puts = util.deprecate(function() { - for (let i = 0, len = arguments.length; i < len; ++i) { - process.stdout.write(arguments[i] + '\n'); - } -}, 'util.puts: Use console.log instead'); +exports.obsoleteFunction = util.deprecate(function() { + // Do something here. +}, 'obsoleteFunction() is deprecated. Use newShinyFunction() instead.'); ``` When called, `util.deprecate()` will return a function that will emit a `DeprecationWarning` using the `process.on('warning')` event. The warning will -be emitted and printed to `stderr` exactly once, the first time it is called. -After the warning is emitted, the wrapped function is called. +be emitted and printed to `stderr` the first time the returned function is +called. After the warning is emitted, the wrapped function is called without +emitting a warning. + +If the same optional `code` is supplied in multiple calls to `util.deprecate()`, +the warning will be emitted only once for that `code`. + +```js +const util = require('util'); + +const fn1 = util.deprecate(someFunction, someMessage, 'DEP0001'); +const fn2 = util.deprecate(someOtherFunction, someOtherMessage, 'DEP0001'); +fn1(); // emits a deprecation warning with code DEP0001 +fn2(); // does not emit a deprecation warning because it has the same code +``` If either the `--no-deprecation` or `--no-warnings` command line flags are used, or if the `process.noDeprecation` property is set to `true` *prior* to diff --git a/lib/internal/util.js b/lib/internal/util.js index bcee867a8aba6e..5fdae42877da8f 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -23,6 +23,10 @@ function objectToString(o) { return Object.prototype.toString.call(o); } +// Keep a list of deprecation codes that have been warned on so we only warn on +// each one once. +const codesWarned = {}; + // Mark that a method should not be used. // Returns a modified function which warns once by default. // If --no-deprecation is set, then it is a no-op. @@ -46,7 +50,10 @@ function deprecate(fn, msg, code) { if (!warned) { warned = true; if (code !== undefined) { - process.emitWarning(msg, 'DeprecationWarning', code, deprecated); + if (!codesWarned[code]) { + process.emitWarning(msg, 'DeprecationWarning', code, deprecated); + codesWarned[code] = true; + } } else { process.emitWarning(msg, 'DeprecationWarning', deprecated); } diff --git a/test/parallel/test-util-deprecate.js b/test/parallel/test-util-deprecate.js new file mode 100644 index 00000000000000..1b4a5e76623743 --- /dev/null +++ b/test/parallel/test-util-deprecate.js @@ -0,0 +1,57 @@ +'use strict'; + +require('../common'); + +// Tests basic functionality of util.deprecate(). + +const assert = require('assert'); +const util = require('util'); + +const expectedWarnings = new Map(); + +// Emits deprecation only once if same function is called. +{ + const msg = 'fhqwhgads'; + const fn = util.deprecate(() => {}, msg); + expectedWarnings.set(msg, { code: undefined, count: 1 }); + fn(); + fn(); +} + +// Emits deprecation twice for different functions. +{ + const msg = 'sterrance'; + const fn1 = util.deprecate(() => {}, msg); + const fn2 = util.deprecate(() => {}, msg); + expectedWarnings.set(msg, { code: undefined, count: 2 }); + fn1(); + fn2(); +} + +// Emits deprecation only once if optional code is the same, even for different +// functions. +{ + const msg = 'cannonmouth'; + const code = 'deprecatesque'; + const fn1 = util.deprecate(() => {}, msg, code); + const fn2 = util.deprecate(() => {}, msg, code); + expectedWarnings.set(msg, { code, count: 1 }); + fn1(); + fn2(); + fn1(); + fn2(); +} + +process.on('warning', (warning) => { + assert.strictEqual(warning.name, 'DeprecationWarning'); + assert.ok(expectedWarnings.has(warning.message)); + const expected = expectedWarnings.get(warning.message); + assert.strictEqual(warning.code, expected.code); + expected.count = expected.count - 1; + if (expected.count === 0) + expectedWarnings.delete(warning.message); +}); + +process.on('exit', () => { + assert.deepStrictEqual(expectedWarnings, new Map()); +}); From 90cccaeeaf1df05f6d1cfbf28d2d3d23e03aa96c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 22 Oct 2017 14:37:34 -0700 Subject: [PATCH 4/4] util: runtime deprecation for custom .inspect() Change documentation-only deprecation for custom inspection using `object.inspect` property to a runtime deprecation. Refs: https://github.com/nodejs/node/issues/15549 --- doc/api/deprecations.md | 2 +- doc/api/util.md | 1 + lib/util.js | 17 +++++++++++++---- test/parallel/test-util-inspect-deprecated.js | 18 ++++++++++++++++++ test/parallel/test-util-inspect.js | 6 ++++++ 5 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-util-inspect-deprecated.js diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index b8170f1e440309..bf7ff58f075447 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -703,7 +703,7 @@ Type: Runtime ### DEP0079: Custom inspection function on Objects via .inspect() -Type: Documentation-only +Type: Runtime Using a property named `inspect` on an object to specify a custom inspection function for [`util.inspect()`][] is deprecated. Use [`util.inspect.custom`][] diff --git a/doc/api/util.md b/doc/api/util.md index e29d13bf402ed5..738a413dc8176c 100644 --- a/doc/api/util.md +++ b/doc/api/util.md @@ -112,6 +112,7 @@ environment variable. For example: `NODE_DEBUG=fs,net,tls`. added: v0.8.0 changes: - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/16393 description: Deprecation warnings are only emitted once for each code. --> diff --git a/lib/util.js b/lib/util.js index 00da489b88989d..a765d91b53a5db 100644 --- a/lib/util.js +++ b/lib/util.js @@ -428,14 +428,23 @@ function formatValue(ctx, value, recurseTimes, ln) { // Provide a hook for user-specified inspect functions. // Check that value is an object with an inspect function on it if (ctx.customInspect) { - const maybeCustomInspect = value[customInspectSymbol] || value.inspect; + let maybeCustom = value[customInspectSymbol]; + + if (!maybeCustom && value.inspect !== exports.inspect && + typeof value.inspect === 'function') { + maybeCustom = deprecate( + value.inspect, + 'Custom inspection function on Objects via .inspect() is deprecated', + 'DEP0079' + ); + } - if (typeof maybeCustomInspect === 'function' && + if (typeof maybeCustom === 'function' && // Filter out the util module, its inspect function is special - maybeCustomInspect !== exports.inspect && + maybeCustom !== exports.inspect && // Also filter out any prototype objects using the circular check. !(value.constructor && value.constructor.prototype === value)) { - const ret = maybeCustomInspect.call(value, recurseTimes, ctx); + const ret = maybeCustom.call(value, recurseTimes, ctx); // If the custom inspection method returned `this`, don't go into // infinite recursion. diff --git a/test/parallel/test-util-inspect-deprecated.js b/test/parallel/test-util-inspect-deprecated.js new file mode 100644 index 00000000000000..adabb0669705b2 --- /dev/null +++ b/test/parallel/test-util-inspect-deprecated.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../common'); + +// Test that deprecation warning for custom inspection via the `.inspect()` +// property (on the target object) is emitted once and only once. + +const util = require('util'); + +{ + const target = { inspect: () => 'Fhqwhgads' }; + // `common.expectWarning` will expect the warning exactly one time only + common.expectWarning( + 'DeprecationWarning', + 'Custom inspection function on Objects via .inspect() is deprecated' + ); + util.inspect(target); // should emit deprecation warning + util.inspect(target); // should not emit deprecation warning +} diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js index 5e5e0b89bc868f..027d223bd09cb8 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -1150,3 +1150,9 @@ if (typeof Symbol !== 'undefined') { } assert.doesNotThrow(() => util.inspect(process)); + +// Setting custom inspect property to a non-function should do nothing. +{ + const obj = { inspect: 'fhqwhgads' }; + assert.strictEqual(util.inspect(obj), "{ inspect: 'fhqwhgads' }"); +}