From bce37c60cec3258d26c497fadc9a192158d39305 Mon Sep 17 00:00:00 2001
From: Eungyu Lee <35999704+Dailyscat@users.noreply.github.com>
Date: Mon, 13 Mar 2023 13:29:53 +0900
Subject: [PATCH] debugger: improve validations and documents for watch and
 unwatch

- debugger: add string validation for watch(expr)
- debugger: add help document for watch(index)
- test: add test for watch(index) command
- doc: add information on unwatch(index) option

PR-URL: https://github.com/nodejs/node/pull/46947
Reviewed-By: Kohei Ueno <kohei.ueno119@gmail.com>
---
 doc/api/debugger.md                           |  1 +
 lib/internal/debugger/inspect_repl.js         |  2 ++
 .../test-debugger-watch-validation.js         | 20 +++++++++++++++++++
 test/sequential/test-debugger-watchers.mjs    | 15 ++++++++++----
 4 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 test/sequential/test-debugger-watch-validation.js

diff --git a/doc/api/debugger.md b/doc/api/debugger.md
index 84422a9e6f77cc..b280fdef68b8c8 100644
--- a/doc/api/debugger.md
+++ b/doc/api/debugger.md
@@ -197,6 +197,7 @@ debug>
   after)
 * `watch(expr)`: Add expression to watch list
 * `unwatch(expr)`: Remove expression from watch list
+* `unwatch(index)`: Remove expression at specific index from watch list
 * `watchers`: List all watchers and their values (automatically listed on each
   breakpoint)
 * `repl`: Open debugger's repl for evaluation in debugging script's context
diff --git a/lib/internal/debugger/inspect_repl.js b/lib/internal/debugger/inspect_repl.js
index 91c0da67d7fd99..7fba613f09c632 100644
--- a/lib/internal/debugger/inspect_repl.js
+++ b/lib/internal/debugger/inspect_repl.js
@@ -91,6 +91,7 @@ breakOnNone           Don't pause on exceptions (this is the default)
 
 watch(expr)           Start watching the given expression
 unwatch(expr)         Stop watching an expression
+unwatch(index)        Stop watching an expression at specific index from watch list
 watchers              Print all watched expressions and their current values
 
 exec(expr), p(expr), exec expr, p expr
@@ -1078,6 +1079,7 @@ function createRepl(inspector) {
       },
 
       watch(expr) {
+        validateString(expr, 'expression');
         ArrayPrototypePush(watchedExpressions, expr);
       },
 
diff --git a/test/sequential/test-debugger-watch-validation.js b/test/sequential/test-debugger-watch-validation.js
new file mode 100644
index 00000000000000..46307c18d55526
--- /dev/null
+++ b/test/sequential/test-debugger-watch-validation.js
@@ -0,0 +1,20 @@
+'use strict';
+const common = require('../common');
+
+common.skipIfInspectorDisabled();
+
+const fixtures = require('../common/fixtures');
+const startCLI = require('../common/debugger');
+
+const assert = require('assert');
+
+const cli = startCLI([fixtures.path('debugger/break.js')]);
+
+(async () => {
+  await cli.waitForInitialBreak();
+  await cli.command('watch()');
+  await cli.waitFor(/ERR_INVALID_ARG_TYPE/);
+  assert.match(cli.output, /TypeError \[ERR_INVALID_ARG_TYPE\]: The "expression" argument must be of type string\. Received undefined/);
+})()
+.finally(() => cli.quit())
+.then(common.mustCall());
diff --git a/test/sequential/test-debugger-watchers.mjs b/test/sequential/test-debugger-watchers.mjs
index 2c4fd11545f2f5..4ff7ea00a22258 100644
--- a/test/sequential/test-debugger-watchers.mjs
+++ b/test/sequential/test-debugger-watchers.mjs
@@ -28,18 +28,25 @@ try {
   await cli.command('watchers');
 
   assert.match(cli.output, /x is not defined/);
-
+  assert.match(cli.output, /1: "Hello" = 'Hello'/);
+  assert.match(cli.output, /2: 42 = 42/);
+  assert.match(cli.output, /3: NaN = NaN/);
+  assert.match(cli.output, /4: true = true/);
+  assert.match(cli.output, /5: \[1, 2\] = \[ 1, 2 \]/);
+  assert.match(cli.output, /6: process\.env =\n\s+\{[\s\S]+,\n\s+\.\.\. \}/,
+               'shows "..." for process.env');
+
+  await cli.command('unwatch(4)');
   await cli.command('unwatch("42")');
   await cli.stepCommand('n');
 
   assert.match(cli.output, /0: x = 10/);
   assert.match(cli.output, /1: "Hello" = 'Hello'/);
   assert.match(cli.output, /2: NaN = NaN/);
-  assert.match(cli.output, /3: true = true/);
-  assert.match(cli.output, /4: \[1, 2\] = \[ 1, 2 \]/);
+  assert.match(cli.output, /3: \[1, 2\] = \[ 1, 2 \]/);
   assert.match(
     cli.output,
-    /5: process\.env =\n\s+\{[\s\S]+,\n\s+\.\.\. \}/,
+    /4: process\.env =\n\s+\{[\s\S]+,\n\s+\.\.\. \}/,
     'shows "..." for process.env'
   );