process: doc-only deprecate non-string env value

doc/api/deprecations.md

@@ -915,7 +915,6 @@ Type: Runtime
 
 This was never a documented feature.
 
-
 <a id="DEP0101"></a>
 ### DEP0101: --with-lttng
 
@@ -932,6 +931,17 @@ Using the `noAssert` argument has no functionality anymore. All input is going
 to be verified, no matter if it is set to true or not. Skipping the verification
 could lead to hard to find errors and crashes.
 
+<a id="DEP00XX"></a>
+### DEP00XX: process.env string coercion
+
+Type: Documentation-only (supports [`--pending-deprecation`][])
+
+Currently when assigning a property to [`process.env`][], the assigned value is
+implicitly converted to a string if it is not a string. This behavior is
+deprecated if the assigned value is not a string, boolean, or number. In the
+future, such assignment may result in a thrown error. Please convert the
+property to a string before assigning it to `process.env`.
+
 [`--pending-deprecation`]: cli.html#cli_pending_deprecation
 [`Buffer.allocUnsafeSlow(size)`]: buffer.html#buffer_class_method_buffer_allocunsafeslow_size
 [`Buffer.from(array)`]: buffer.html#buffer_class_method_buffer_from_array
@@ -968,6 +978,7 @@ could lead to hard to find errors and crashes.
 [`fs.stat()`]: fs.html#fs_fs_stat_path_callback
 [`os.networkInterfaces`]: os.html#os_os_networkinterfaces
 [`os.tmpdir()`]: os.html#os_os_tmpdir
+[`process.env`]: process.html#process_process_env
 [`punycode`]: punycode.html
 [`require.extensions`]: modules.html#modules_require_extensions
 [`setInterval()`]: timers.html#timers_setinterval_callback_delay_args

doc/api/process.md

@@ -836,6 +836,10 @@ emitMyWarning();
 ## process.env
 <!-- YAML
 added: v0.1.27
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/18990
+    description: Implicit conversion of variable value to string is deprecated.
 -->
 
 * {Object}
@@ -877,7 +881,8 @@ console.log(process.env.foo);
 ```
 
 Assigning a property on `process.env` will implicitly convert the value
-to a string.
+to a string. **This behavior is deprecated.** Future versions of Node.js may
+throw an error when the value is not a string, number, or boolean.
 
 Example:
 

src/env-inl.h

@@ -327,6 +327,7 @@ inline Environment::Environment(IsolateData* isolate_data,
       trace_sync_io_(false),
       abort_on_uncaught_exception_(false),
       emit_napi_warning_(true),
+      emit_env_nonstring_warning_(true),
       makecallback_cntr_(0),
       should_abort_on_uncaught_toggle_(isolate_, 1),
 #if HAVE_INSPECTOR

src/env.h

@@ -735,6 +735,11 @@ class Environment {
   void AddPromiseHook(promise_hook_func fn, void* arg);
   bool RemovePromiseHook(promise_hook_func fn, void* arg);
   bool EmitNapiWarning();
+  inline bool EmitProcessEnvWarning() {
+    bool current_value = emit_env_nonstring_warning_;
+    emit_env_nonstring_warning_ = false;
+    return current_value;
+  }
 
   typedef void (*native_immediate_callback)(Environment* env, void* data);
   // cb will be called as cb(env, data) on the next event loop iteration.
@@ -789,6 +794,7 @@ class Environment {
   bool trace_sync_io_;
   bool abort_on_uncaught_exception_;
   bool emit_napi_warning_;
+  bool emit_env_nonstring_warning_;
   size_t makecallback_cntr_;
   std::vector<double> destroy_async_id_list_;
 

src/node.cc

@@ -2659,6 +2659,17 @@ static void EnvGetter(Local<Name> property,
 static void EnvSetter(Local<Name> property,
                       Local<Value> value,
                       const PropertyCallbackInfo<Value>& info) {
+  Environment* env = Environment::GetCurrent(info);
+  if (config_pending_deprecation && env->EmitProcessEnvWarning() &&
+      !value->IsString() && !value->IsNumber() && !value->IsBoolean()) {
+    if (ProcessEmitDeprecationWarning(
+          env,
+          "Assigning any value other than a string, number, or boolean value "
+          "to a process.env property is deprecated. Please make sure to "
+          "convert the value to a string before setting process.env with it.",
+          "DEP00XX").IsNothing())
+      return;
+  }
 #ifdef __POSIX__
   node::Utf8Value key(info.GetIsolate(), property);
   node::Utf8Value val(info.GetIsolate(), value);

test/parallel/test-process-env-deprecation.js

@@ -0,0 +1,16 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+// Flags: --pending-deprecation
+
+common.expectWarning(
+  'DeprecationWarning',
+  'Assigning any value other than a string, number, or boolean value to a ' +
+  'process.env property is deprecated. Please make sure to convert the value ' +
+  'to a string before setting process.env with it.',
+  'DEP00XX'
+);
+
+process.env.ABC = undefined;
+assert.strictEqual(process.env.ABC, 'undefined');