util: runtime deprecation for custom .inspect()

Change documentation-only deprecation for custom inspection using
`object.inspect` property to a runtime deprecation.

This is a breaking change. Custom inspection via `object.inspect` is
deprecated because there is a more robust Symbol-based alternative to
`.inspect` and the custom inspection via `object.inspect` feature means
that people can accidentally break `console.log()` simply by attaching a
`.inspect` property to their objects. Note that since this is a
deprecation, the custom inspection will still work. The breaking change
is simply the printing of a warning which could alarm users, break tests
or other things that might be dependent on specific output, etc.

PR-URL: https://github.com/nodejs/node/pull/16393
Ref: https://github.com/nodejs/node/issues/15549
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

doc/api/deprecations.md

@@ -703,7 +703,7 @@ Type: Runtime
 <a id="DEP0079"></a>
 ### 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`][]

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.
 -->
 

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 (typeof maybeCustomInspect === 'function' &&
+    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 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.

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
+}

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' }");
+}