From 90d35adccd763dff70ae86cef0319d16cce8c40a Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Thu, 12 Mar 2020 23:24:01 -0400
Subject: [PATCH] doc,lib,src,test: make --experimental-report a nop

This commit makes the --experimental-report CLI flag a no-op.

PR-URL: https://github.com/nodejs/node/pull/32242
Fixes: https://github.com/nodejs/node/issues/26293
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/cli.md                                | 24 ++++------
 doc/api/report.md                             | 11 ++---
 doc/node.1                                    | 22 ++++-----
 lib/internal/bootstrap/pre_execution.js       |  9 ----
 src/node_options.cc                           | 46 +------------------
 src/node_options.h                            |  3 +-
 test/addons/worker-addon/test.js              |  1 -
 test/parallel/test-bootstrap-modules.js       |  2 +
 ...rocess-env-allowed-flags-are-documented.js |  1 +
 test/report/test-report-config.js             |  6 +--
 test/report/test-report-fatal-error.js        |  3 +-
 test/report/test-report-getreport.js          |  7 +--
 test/report/test-report-signal.js             |  5 +-
 test/report/test-report-uncaught-exception.js |  5 +-
 test/report/test-report-uv-handles.js         |  8 +---
 test/report/test-report-worker.js             |  1 -
 test/report/test-report-writereport.js        | 15 ++----
 17 files changed, 35 insertions(+), 134 deletions(-)

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 207723dabc4d34..0f1e24222dc666 100644
--- a/doc/api/cli.md
+++ b/doc/api/cli.md
@@ -199,13 +199,6 @@ added: v10.0.0
 
 Enable experimental top-level `await` keyword support in REPL.
 
-### `--experimental-report`
-<!-- YAML
-added: v11.8.0
--->
-
-Enable experimental diagnostic report feature.
-
 ### `--experimental-specifier-resolution=mode`
 <!-- YAML
 added: v12.16.0
@@ -658,9 +651,9 @@ changes:
 
 Enables the report to be triggered on fatal errors (internal errors within
 the Node.js runtime such as out of memory) that lead to termination of the
-application, if `--experimental-report` is enabled. Useful to inspect various
-diagnostic data elements such as heap, stack, event loop state, resource
-consumption etc. to reason about the fatal error.
+application. Useful to inspect various diagnostic data elements such as heap,
+stack, event loop state, resource consumption etc. to reason about the fatal
+error.
 
 ### `--report-on-signal`
 <!-- YAML
@@ -673,8 +666,8 @@ changes:
 -->
 
 Enables report to be generated upon receiving the specified (or predefined)
-signal to the running Node.js process, if `--experimental-report` is enabled.
-The signal to trigger the report is specified through `--report-signal`.
+signal to the running Node.js process. The signal to trigger the report is
+specified through `--report-signal`.
 
 ### `--report-signal=signal`
 <!-- YAML
@@ -699,9 +692,9 @@ changes:
                  `--report-uncaught-exception`
 -->
 
-Enables report to be generated on un-caught exceptions, if
-`--experimental-report` is enabled. Useful when inspecting JavaScript stack in
-conjunction with native stack and other runtime environment data.
+Enables report to be generated on uncaught exceptions. Useful when inspecting
+the JavaScript stack in conjunction with native stack and other runtime
+environment data.
 
 ### `--throw-deprecation`
 <!-- YAML
@@ -1121,7 +1114,6 @@ Node.js options that are allowed are:
 * `--experimental-modules`
 * `--experimental-policy`
 * `--experimental-repl-await`
-* `--experimental-report`
 * `--experimental-specifier-resolution`
 * `--experimental-vm-modules`
 * `--experimental-wasi-unstable-preview1`
diff --git a/doc/api/report.md b/doc/api/report.md
index c2640c90849ad7..711c1412687f08 100644
--- a/doc/api/report.md
+++ b/doc/api/report.md
@@ -32,7 +32,6 @@ is provided below for reference.
     "cwd": "/home/nodeuser/project/node",
     "commandLine": [
       "/home/nodeuser/project/node/out/Release/node",
-      "--experimental-report",
       "--report-uncaught-exception",
       "/home/nodeuser/project/node/test/report/test-exception.js",
       "child"
@@ -393,14 +392,10 @@ is provided below for reference.
 ## Usage
 
 ```bash
-node --experimental-report --report-uncaught-exception \
-  --report-on-signal --report-on-fatalerror app.js
+node --report-uncaught-exception --report-on-signal \
+--report-on-fatalerror app.js
 ```
 
-* `--experimental-report` Enables the diagnostic report feature.
- In the absence of this flag, use of all other related options will result in
- an error.
-
 * `--report-uncaught-exception` Enables report to be generated on
 un-caught exceptions. Useful when inspecting JavaScript stack in conjunction
 with native stack and other runtime environment data.
@@ -570,7 +565,7 @@ Configuration on module initialization is also available via
 environment variables:
 
 ```bash
-NODE_OPTIONS="--experimental-report --report-uncaught-exception \
+NODE_OPTIONS="--report-uncaught-exception \
   --report-on-fatalerror --report-on-signal \
   --report-signal=SIGUSR2  --report-filename=./report.json \
   --report-directory=/home/nodeuser"
diff --git a/doc/node.1 b/doc/node.1
index 82f06d372c9639..ca07cdef1c5a57 100644
--- a/doc/node.1
+++ b/doc/node.1
@@ -138,11 +138,6 @@ keyword support in REPL.
 .It Fl -experimental-specifier-resolution
 Select extension resolution algorithm for ES Modules; either 'explicit' (default) or 'node'
 .
-.It Fl -experimental-report
-Enable experimental
-.Sy diagnostic report
-feature.
-.
 .It Fl -experimental-vm-modules
 Enable experimental ES module support in VM module.
 .
@@ -308,16 +303,16 @@ will be written.
 .It Fl -report-on-fatalerror
 Enables the
 .Sy diagnostic report
-to be triggered on fatal errors (internal errors within the Node.js runtime such as out of memory) that leads to termination of the application, if
-.Sy --experimental-report
-is enabled. Useful to inspect various diagnostic data elements such as heap, stack, event loop state, resource consumption etc. to reason about the fatal error.
+to be triggered on fatal errors (internal errors within the Node.js runtime such
+as out of memory) that leads to termination of the application. Useful to
+inspect various diagnostic data elements such as heap, stack, event loop state,
+resource consumption etc. to reason about the fatal error.
 .
 .It Fl -report-on-signal
 Enables
 .Sy diagnostic report
-to be generated upon receiving the specified (or predefined) signal to the running Node.js process, if
-.Sy --experimental-report
-is enabled. Default signal is SIGUSR2.
+to be generated upon receiving the specified (or predefined) signal to the
+running Node.js process. Default signal is SIGUSR2.
 .
 .It Fl -report-signal
 Sets or resets the signal for
@@ -327,9 +322,8 @@ generation (not supported on Windows). Default signal is SIGUSR2.
 .It Fl -report-uncaught-exception
 Enables
 .Sy diagnostic report
-to be generated on un-caught exceptions, if
-.Sy --experimental-report
-is enabled. Useful when inspecting JavaScript stack in conjunction with native stack and other runtime environment data.
+to be generated on un-caught exceptions. Useful when inspecting JavaScript
+stack in conjunction with native stack and other runtime environment data.
 .
 .It Fl -throw-deprecation
 Throw errors for deprecations.
diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
index 22ef1b2c30511f..460c29c7584a46 100644
--- a/lib/internal/bootstrap/pre_execution.js
+++ b/lib/internal/bootstrap/pre_execution.js
@@ -163,16 +163,11 @@ function setupStacktracePrinterOnSigint() {
 }
 
 function initializeReport() {
-  if (!getOptionValue('--experimental-report')) {
-    return;
-  }
   const { report } = require('internal/process/report');
-  const { emitExperimentalWarning } = require('internal/util');
   ObjectDefineProperty(process, 'report', {
     enumerable: false,
     configurable: true,
     get() {
-      emitExperimentalWarning('report');
       return report;
     }
   });
@@ -187,10 +182,6 @@ function setupDebugEnv() {
 
 // This has to be called after initializeReport() is called
 function initializeReportSignalHandlers() {
-  if (!getOptionValue('--experimental-report')) {
-    return;
-  }
-
   const { addSignalHandler } = require('internal/process/report');
 
   addSignalHandler();
diff --git a/src/node_options.cc b/src/node_options.cc
index c2a28c0719fc55..f2c0024abc6670 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -76,47 +76,6 @@ void PerProcessOptions::CheckOptions(std::vector<std::string>* errors) {
 
 void PerIsolateOptions::CheckOptions(std::vector<std::string>* errors) {
   per_env->CheckOptions(errors);
-
-  if (per_env->experimental_report) {
-    // Assign the report_signal default value here. Once the
-    // --experimental-report flag is dropped, move this initialization to
-    // node_options.h, where report_signal is declared.
-    if (report_signal.empty())
-      report_signal = "SIGUSR2";
-    return;
-  }
-
-  if (!report_directory.empty()) {
-    errors->push_back("--report-directory option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (!report_filename.empty()) {
-    errors->push_back("--report-filename option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (!report_signal.empty()) {
-    errors->push_back("--report-signal option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (report_on_fatalerror) {
-    errors->push_back(
-        "--report-on-fatalerror option is valid only when "
-        "--experimental-report is set");
-  }
-
-  if (report_on_signal) {
-    errors->push_back("--report-on-signal option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (report_uncaught_exception) {
-    errors->push_back(
-        "--report-uncaught-exception option is valid only when "
-        "--experimental-report is set");
-  }
 }
 
 void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
@@ -395,10 +354,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::experimental_vm_modules,
             kAllowedInEnvironment);
   AddOption("--experimental-worker", "", NoOp{}, kAllowedInEnvironment);
-  AddOption("--experimental-report",
-            "enable report generation",
-            &EnvironmentOptions::experimental_report,
-            kAllowedInEnvironment);
+  AddOption("--experimental-report", "", NoOp{}, kAllowedInEnvironment);
   AddOption("--experimental-wasi-unstable-preview1",
             "experimental WASI support",
             &EnvironmentOptions::experimental_wasi,
diff --git a/src/node_options.h b/src/node_options.h
index 05e9fb25dd57ce..6cb5245492d733 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -152,7 +152,6 @@ class EnvironmentOptions : public Options {
 
   bool syntax_check_only = false;
   bool has_eval_string = false;
-  bool experimental_report = false;
   bool experimental_wasi = false;
   std::string eval_string;
   bool print_eval = false;
@@ -189,7 +188,7 @@ class PerIsolateOptions : public Options {
   bool report_uncaught_exception = false;
   bool report_on_signal = false;
   bool report_on_fatalerror = false;
-  std::string report_signal;
+  std::string report_signal = "SIGUSR2";
   std::string report_filename;
   std::string report_directory;
   inline EnvironmentOptions* get_per_env_options();
diff --git a/test/addons/worker-addon/test.js b/test/addons/worker-addon/test.js
index 85a5e2701609fd..f2df4665f53068 100644
--- a/test/addons/worker-addon/test.js
+++ b/test/addons/worker-addon/test.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-report
 'use strict';
 const common = require('../../common');
 const assert = require('assert');
diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js
index 9b075d981a7e5c..391fdad4d35cf4 100644
--- a/test/parallel/test-bootstrap-modules.js
+++ b/test/parallel/test-bootstrap-modules.js
@@ -22,6 +22,7 @@ const expectedModules = new Set([
   'Internal Binding native_module',
   'Internal Binding options',
   'Internal Binding process_methods',
+  'Internal Binding report',
   'Internal Binding string_decoder',
   'Internal Binding task_queue',
   'Internal Binding timers',
@@ -54,6 +55,7 @@ const expectedModules = new Set([
   'NativeModule internal/process/execution',
   'NativeModule internal/process/per_thread',
   'NativeModule internal/process/promises',
+  'NativeModule internal/process/report',
   'NativeModule internal/process/signal',
   'NativeModule internal/process/task_queues',
   'NativeModule internal/process/warning',
diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js
index 01999cc46241c6..0e0af9471ce84e 100644
--- a/test/parallel/test-process-env-allowed-flags-are-documented.js
+++ b/test/parallel/test-process-env-allowed-flags-are-documented.js
@@ -84,6 +84,7 @@ const undocumented = difference(process.allowedNodeEnvironmentFlags,
 // Remove intentionally undocumented options.
 assert(undocumented.delete('--debug-arraybuffer-allocations'));
 assert(undocumented.delete('--es-module-specifier-resolution'));
+assert(undocumented.delete('--experimental-report'));
 assert(undocumented.delete('--experimental-worker'));
 assert(undocumented.delete('--no-node-snapshot'));
 assert(undocumented.delete('--loader'));
diff --git a/test/report/test-report-config.js b/test/report/test-report-config.js
index 49781ac9582adc..f179925440ab7b 100644
--- a/test/report/test-report-config.js
+++ b/test/report/test-report-config.js
@@ -1,12 +1,8 @@
-// Flags: --experimental-report --report-on-fatalerror --report-on-signal --report-uncaught-exception
+// Flags: --report-on-fatalerror --report-on-signal --report-uncaught-exception
 'use strict';
 const common = require('../common');
 const assert = require('assert');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
-
 // Verify that process.report.directory behaves properly.
 assert.strictEqual(process.report.directory, '');
 process.report.directory = __dirname;
diff --git a/test/report/test-report-fatal-error.js b/test/report/test-report-fatal-error.js
index 60eeef6c454ba7..99a2da71c203e1 100644
--- a/test/report/test-report-fatal-error.js
+++ b/test/report/test-report-fatal-error.js
@@ -21,8 +21,7 @@ if (process.argv[2] === 'child') {
   const tmpdir = require('../common/tmpdir');
   tmpdir.refresh();
   const spawn = require('child_process').spawn;
-  const args = ['--experimental-report',
-                '--report-on-fatalerror',
+  const args = ['--report-on-fatalerror',
                 '--max-old-space-size=20',
                 __filename,
                 'child'];
diff --git a/test/report/test-report-getreport.js b/test/report/test-report-getreport.js
index efadaa73f10cb9..4c68b4ecbb5e48 100644
--- a/test/report/test-report-getreport.js
+++ b/test/report/test-report-getreport.js
@@ -1,13 +1,8 @@
-// Flags: --experimental-report
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const helper = require('../common/report');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
-
 {
   // Test with no arguments.
   helper.validateContent(process.report.getReport());
diff --git a/test/report/test-report-signal.js b/test/report/test-report-signal.js
index c1c98d2588aba5..cb5efd9fc39fe2 100644
--- a/test/report/test-report-signal.js
+++ b/test/report/test-report-signal.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-report --report-on-signal
+// Flags: --report-on-signal
 'use strict';
 // Test producing a report via signal.
 const common = require('../common');
@@ -13,9 +13,6 @@ const assert = require('assert');
 const helper = require('../common/report');
 const tmpdir = require('../common/tmpdir');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
 tmpdir.refresh();
 process.report.directory = tmpdir.path;
 
diff --git a/test/report/test-report-uncaught-exception.js b/test/report/test-report-uncaught-exception.js
index d35b5f276f9f4e..10dcccb090019c 100644
--- a/test/report/test-report-uncaught-exception.js
+++ b/test/report/test-report-uncaught-exception.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-report --report-uncaught-exception
+// Flags: --report-uncaught-exception
 'use strict';
 // Test producing a report on uncaught exception.
 const common = require('../common');
@@ -7,9 +7,6 @@ const helper = require('../common/report');
 const tmpdir = require('../common/tmpdir');
 const error = new Error('test error');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
 tmpdir.refresh();
 process.report.directory = tmpdir.path;
 
diff --git a/test/report/test-report-uv-handles.js b/test/report/test-report-uv-handles.js
index bf5e53d154b32d..3a6a34a8573fe7 100644
--- a/test/report/test-report-uv-handles.js
+++ b/test/report/test-report-uv-handles.js
@@ -77,13 +77,12 @@ if (process.argv[2] === 'child') {
   const tmpdir = require('../common/tmpdir');
   tmpdir.refresh();
   const options = { encoding: 'utf8', silent: true, cwd: tmpdir.path };
-  const child = fork('--experimental-report', [__filename, 'child'], options);
+  const child = fork(__filename, ['child'], options);
   let child_data;
   child.on('message', (data) => { child_data = data; });
   let stderr = '';
   child.stderr.on('data', (chunk) => { stderr += chunk; });
   let stdout = '';
-  const std_msg = 'Found messages in stderr unexpectedly: ';
   const report_msg = 'Report files were written: unexpectedly';
   child.stdout.on('data', (chunk) => { stdout += chunk; });
   child.on('exit', common.mustCall((code, signal) => {
@@ -91,10 +90,7 @@ if (process.argv[2] === 'child') {
                            `${code}`);
     assert.deepStrictEqual(signal, null, 'Process should have exited cleanly,' +
                             ` but did not: ${signal}`);
-    assert.ok(stderr.match(
-      '(node:.*) ExperimentalWarning: report is an experimental' +
-      ' feature. This feature could change at any time'),
-              std_msg);
+    assert.strictEqual(stderr.trim(), '');
 
     const reports = helper.findReports(child.pid, tmpdir.path);
     assert.deepStrictEqual(reports, [], report_msg, reports);
diff --git a/test/report/test-report-worker.js b/test/report/test-report-worker.js
index 77e9fe92c411b3..feb83a50643b0f 100644
--- a/test/report/test-report-worker.js
+++ b/test/report/test-report-worker.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-report
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/report/test-report-writereport.js b/test/report/test-report-writereport.js
index f04a19f3e01bc0..50514c68373d15 100644
--- a/test/report/test-report-writereport.js
+++ b/test/report/test-report-writereport.js
@@ -1,8 +1,7 @@
-// Flags: --experimental-report
 'use strict';
 
 // Test producing a report via API call, using the no-hooks/no-signal interface.
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const { spawnSync } = require('child_process');
 const fs = require('fs');
@@ -10,9 +9,6 @@ const path = require('path');
 const helper = require('../common/report');
 const tmpdir = require('../common/tmpdir');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
 tmpdir.refresh();
 process.report.directory = tmpdir.path;
 
@@ -92,8 +88,7 @@ function validate() {
 
 {
   // Test the special "stdout" filename.
-  const args = ['--experimental-report', '-e',
-                'process.report.writeReport("stdout")'];
+  const args = ['-e', 'process.report.writeReport("stdout")'];
   const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
   assert.strictEqual(child.status, 0);
   assert.strictEqual(child.signal, null);
@@ -103,8 +98,7 @@ function validate() {
 
 {
   // Test the special "stderr" filename.
-  const args = ['--experimental-report', '-e',
-                'process.report.writeReport("stderr")'];
+  const args = ['-e', 'process.report.writeReport("stderr")'];
   const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });
   assert.strictEqual(child.status, 0);
   assert.strictEqual(child.signal, null);
@@ -117,8 +111,7 @@ function validate() {
 {
   // Test the case where the report file cannot be opened.
   const reportDir = path.join(tmpdir.path, 'does', 'not', 'exist');
-  const args = ['--experimental-report',
-                `--report-directory=${reportDir}`,
+  const args = [`--report-directory=${reportDir}`,
                 '-e',
                 'process.report.writeReport()'];
   const child = spawnSync(process.execPath, args, { cwd: tmpdir.path });