repl,readline: refactor for simplicity

This just refactors code without changing the behavior. Especially
the REPL code is difficult to read and deeply indented. This reduces
the indentation to improve that.

PR-URL: https://github.com/nodejs/node/pull/30907
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

2 files changed, 179 insertions, 181 deletions

diff --git a/lib/readline.js b/lib/readline.js
index 9a306c654b5..0ba30cc04b1 100644
--- a/lib/readline.js
+++ b/lib/readline.js
@@ -505,41 +505,43 @@ Interface.prototype._tabComplete = function(lastKeypressWasTab) {
       return;
     }
 
-    const completions = rv[0];
-    const completeOn = rv[1];  // The text that was completed
-    if (completions && completions.length) {
-      // Apply/show completions.
-      if (lastKeypressWasTab) {
-        self._writeToOutput('\r\n');
-        const width = completions.reduce(function completionReducer(a, b) {
-          return a.length > b.length ? a : b;
-        }).length + 2;  // 2 space padding
-        let maxColumns = MathFloor(self.columns / width);
-        if (!maxColumns || maxColumns === Infinity) {
-          maxColumns = 1;
-        }
-        let group = [];
-        for (let i = 0; i < completions.length; i++) {
-          const c = completions[i];
-          if (c === '') {
-            handleGroup(self, group, width, maxColumns);
-            group = [];
-          } else {
-            group.push(c);
-          }
-        }
-        handleGroup(self, group, width, maxColumns);
-      }
+    // Result and the text that was completed.
+    const [completions, completeOn] = rv;
+
+    if (!completions || completions.length === 0) {
+      return;
+    }
 
-      // If there is a common prefix to all matches, then apply that portion.
-      const f = completions.filter((e) => e);
-      const prefix = commonPrefix(f);
-      if (prefix.length > completeOn.length) {
-        self._insertString(prefix.slice(completeOn.length));
+    // Apply/show completions.
+    if (lastKeypressWasTab) {
+      self._writeToOutput('\r\n');
+      const width = completions.reduce((a, b) => {
+        return a.length > b.length ? a : b;
+      }).length + 2;  // 2 space padding
+      let maxColumns = MathFloor(self.columns / width);
+      if (!maxColumns || maxColumns === Infinity) {
+        maxColumns = 1;
       }
+      let group = [];
+      for (const c of completions) {
+        if (c === '') {
+          handleGroup(self, group, width, maxColumns);
+          group = [];
+        } else {
+          group.push(c);
+        }
+      }
+      handleGroup(self, group, width, maxColumns);
+    }
 
-      self._refreshLine();
+    // If there is a common prefix to all matches, then apply that portion.
+    const f = completions.filter((e) => e);
+    const prefix = commonPrefix(f);
+    if (prefix.length > completeOn.length) {
+      self._insertString(prefix.slice(completeOn.length));
     }
+
+    self._refreshLine();
   });
 };
 
diff --git a/lib/repl.js b/lib/repl.js
index 221b5f1f612..4f32c9c28ce 100644
--- a/lib/repl.js
+++ b/lib/repl.js
@@ -52,7 +52,6 @@ const {
   ObjectGetOwnPropertyNames,
   ObjectGetPrototypeOf,
   ObjectKeys,
-  ObjectPrototypeHasOwnProperty,
   ObjectSetPrototypeOf,
   Symbol,
 } = primordials;
@@ -1164,7 +1163,7 @@ function complete(line, callback) {
     completeOn = match[1];
     const subdir = match[2] || '';
     filter = match[1];
-    let dir, files, name, base, ext, abs, subfiles, isDirectory;
+    let dir, files, subfiles, isDirectory;
     group = [];
     let paths = [];
 
@@ -1186,14 +1185,14 @@ function complete(line, callback) {
         continue;
       }
       for (let f = 0; f < files.length; f++) {
-        name = files[f];
-        ext = path.extname(name);
-        base = name.slice(0, -ext.length);
+        const name = files[f];
+        const ext = path.extname(name);
+        const base = name.slice(0, -ext.length);
         if (versionedFileNamesRe.test(base) || name === '.npm') {
           // Exclude versioned names that 'npm' installs.
           continue;
         }
-        abs = path.resolve(dir, name);
+        const abs = path.resolve(dir, name);
         try {
           isDirectory = fs.statSync(abs).isDirectory();
         } catch {
@@ -1261,86 +1260,87 @@ function complete(line, callback) {
   //   foo.<|>        # completions for 'foo' with filter ''
   } else if (line.length === 0 || /\w|\.|\$/.test(line[line.length - 1])) {
     match = simpleExpressionRE.exec(line);
-    if (line.length === 0 || match) {
-      let expr;
-      completeOn = (match ? match[0] : '');
-      if (line.length === 0) {
-        filter = '';
-        expr = '';
-      } else if (line[line.length - 1] === '.') {
-        filter = '';
-        expr = match[0].slice(0, match[0].length - 1);
-      } else {
-        const bits = match[0].split('.');
-        filter = bits.pop();
-        expr = bits.join('.');
+    if (line.length !== 0 && !match) {
+      completionGroupsLoaded();
+      return;
+    }
+    let expr;
+    completeOn = (match ? match[0] : '');
+    if (line.length === 0) {
+      filter = '';
+      expr = '';
+    } else if (line[line.length - 1] === '.') {
+      filter = '';
+      expr = match[0].slice(0, match[0].length - 1);
+    } else {
+      const bits = match[0].split('.');
+      filter = bits.pop();
+      expr = bits.join('.');
+    }
+
+    // Resolve expr and get its completions.
+    const memberGroups = [];
+    if (!expr) {
+      // Get global vars synchronously
+      completionGroups.push(getGlobalLexicalScopeNames(this[kContextId]));
+      let contextProto = this.context;
+      while (contextProto = ObjectGetPrototypeOf(contextProto)) {
+        completionGroups.push(filteredOwnPropertyNames(contextProto));
       }
+      const contextOwnNames = filteredOwnPropertyNames(this.context);
+      if (!this.useGlobal) {
+        // When the context is not `global`, builtins are not own
+        // properties of it.
+        contextOwnNames.push(...globalBuiltins);
+      }
+      completionGroups.push(contextOwnNames);
+      if (filter !== '') addCommonWords(completionGroups);
+      completionGroupsLoaded();
+      return;
+    }
 
-      // Resolve expr and get its completions.
-      const memberGroups = [];
-      if (!expr) {
-        // Get global vars synchronously
-        completionGroups.push(getGlobalLexicalScopeNames(this[kContextId]));
-        let contextProto = this.context;
-        while (contextProto = ObjectGetPrototypeOf(contextProto)) {
-          completionGroups.push(filteredOwnPropertyNames(contextProto));
-        }
-        const contextOwnNames = filteredOwnPropertyNames(this.context);
-        if (!this.useGlobal) {
-          // When the context is not `global`, builtins are not own
-          // properties of it.
-          contextOwnNames.push(...globalBuiltins);
+    const evalExpr = `try { ${expr} } catch {}`;
+    this.eval(evalExpr, this.context, 'repl', (e, obj) => {
+      if (obj != null) {
+        if (typeof obj === 'object' || typeof obj === 'function') {
+          try {
+            memberGroups.push(filteredOwnPropertyNames(obj));
+          } catch {
+            // Probably a Proxy object without `getOwnPropertyNames` trap.
+            // We simply ignore it here, as we don't want to break the
+            // autocompletion. Fixes the bug
+            // https://github.com/nodejs/node/issues/2119
+          }
         }
-        completionGroups.push(contextOwnNames);
-        if (filter !== '') addCommonWords(completionGroups);
-        completionGroupsLoaded();
-      } else {
-        const evalExpr = `try { ${expr} } catch {}`;
-        this.eval(evalExpr, this.context, 'repl', (e, obj) => {
-          if (obj != null) {
-            if (typeof obj === 'object' || typeof obj === 'function') {
-              try {
-                memberGroups.push(filteredOwnPropertyNames(obj));
-              } catch {
-                // Probably a Proxy object without `getOwnPropertyNames` trap.
-                // We simply ignore it here, as we don't want to break the
-                // autocompletion. Fixes the bug
-                // https://github.com/nodejs/node/issues/2119
-              }
-            }
-            // Works for non-objects
-            try {
-              let p;
-              if (typeof obj === 'object' || typeof obj === 'function') {
-                p = ObjectGetPrototypeOf(obj);
-              } else {
-                p = obj.constructor ? obj.constructor.prototype : null;
-              }
-              // Circular refs possible? Let's guard against that.
-              let sentinel = 5;
-              while (p !== null && sentinel-- !== 0) {
-                memberGroups.push(filteredOwnPropertyNames(p));
-                p = ObjectGetPrototypeOf(p);
-              }
-            } catch {}
+        // Works for non-objects
+        try {
+          let p;
+          if (typeof obj === 'object' || typeof obj === 'function') {
+            p = ObjectGetPrototypeOf(obj);
+          } else {
+            p = obj.constructor ? obj.constructor.prototype : null;
           }
-
-          if (memberGroups.length) {
-            for (let i = 0; i < memberGroups.length; i++) {
-              completionGroups.push(
-                memberGroups[i].map((member) => `${expr}.${member}`));
-            }
-            if (filter) {
-              filter = `${expr}.${filter}`;
-            }
+          // Circular refs possible? Let's guard against that.
+          let sentinel = 5;
+          while (p !== null && sentinel-- !== 0) {
+            memberGroups.push(filteredOwnPropertyNames(p));
+            p = ObjectGetPrototypeOf(p);
           }
+        } catch {}
+      }
 
-          completionGroupsLoaded();
-        });
+      if (memberGroups.length) {
+        for (let i = 0; i < memberGroups.length; i++) {
+          completionGroups.push(
+            memberGroups[i].map((member) => `${expr}.${member}`));
+        }
+        if (filter) {
+          filter = `${expr}.${filter}`;
+        }
       }
-    } else {
+
       completionGroupsLoaded();
-    }
+    });
   } else {
     completionGroupsLoaded();
   }
@@ -1361,32 +1361,32 @@ function complete(line, callback) {
       completionGroups = newCompletionGroups;
     }
 
-    let completions;
-
-    if (completionGroups.length) {
-      const uniq = {};  // Unique completions across all groups
-      completions = [];
-      // Completion group 0 is the "closest"
-      // (least far up the inheritance chain)
-      // so we put its completions last: to be closest in the REPL.
-      for (let i = 0; i < completionGroups.length; i++) {
-        group = completionGroups[i];
-        group.sort();
-        for (let j = group.length - 1; j >= 0; j--) {
-          const c = group[j];
-          if (!ObjectPrototypeHasOwnProperty(uniq, c)) {
-            completions.unshift(c);
-            uniq[c] = true;
-          }
+    const completions = [];
+    // Unique completions across all groups.
+    const uniqueSet = new Set(['']);
+    // Completion group 0 is the "closest" (least far up the inheritance
+    // chain) so we put its completions last: to be closest in the REPL.
+    for (const group of completionGroups) {
+      group.sort((a, b) => (b > a ? 1 : -1));
+      const setSize = uniqueSet.size;
+      for (const entry of group) {
+        if (!uniqueSet.has(entry)) {
+          completions.unshift(entry);
+          uniqueSet.add(entry);
         }
-        completions.unshift(''); // Separator btwn groups
       }
-      while (completions.length && completions[0] === '') {
-        completions.shift();
+      // Add a separator between groups.
+      if (uniqueSet.size !== setSize) {
+        completions.unshift('');
       }
     }
 
-    callback(null, [completions || [], completeOn]);
+    // Remove obsolete group entry, if present.
+    if (completions[0] === '') {
+      completions.shift();
+    }
+
+    callback(null, [completions, completeOn]);
   }
 }
 
@@ -1419,6 +1419,9 @@ REPLServer.prototype.memory = deprecate(
   'REPLServer.memory() is deprecated',
   'DEP0082');
 
+// TODO(BridgeAR): This should be replaced with acorn to build an AST. The
+// language became more complex and using a simple approach like this is not
+// sufficient anymore.
 function _memory(cmd) {
   const self = this;
   self.lines = self.lines || [];
@@ -1426,7 +1429,6 @@ function _memory(cmd) {
 
   // Save the line so I can do magic later
   if (cmd) {
-    // TODO should I tab the level?
     const len = self.lines.level.length ? self.lines.level.length - 1 : 0;
     self.lines.push('  '.repeat(len) + cmd);
   } else {
@@ -1434,61 +1436,55 @@ function _memory(cmd) {
     self.lines.push('');
   }
 
+  if (!cmd) {
+    self.lines.level = [];
+    return;
+  }
+
   // I need to know "depth."
   // Because I can not tell the difference between a } that
   // closes an object literal and a } that closes a function
-  if (cmd) {
-    // Going down is { and (   e.g. function() {
-    // going up is } and )
-    let dw = cmd.match(/[{(]/g);
-    let up = cmd.match(/[})]/g);
-    up = up ? up.length : 0;
-    dw = dw ? dw.length : 0;
-    let depth = dw - up;
-
-    if (depth) {
-      (function workIt() {
-        if (depth > 0) {
-          // Going... down.
-          // Push the line#, depth count, and if the line is a function.
-          // Since JS only has functional scope I only need to remove
-          // "function() {" lines, clearly this will not work for
-          // "function()
-          // {" but nothing should break, only tab completion for local
-          // scope will not work for this function.
-          self.lines.level.push({
-            line: self.lines.length - 1,
-            depth: depth,
-            isFunction: /\bfunction\b/.test(cmd)
-          });
-        } else if (depth < 0) {
-          // Going... up.
-          const curr = self.lines.level.pop();
-          if (curr) {
-            const tmp = curr.depth + depth;
-            if (tmp < 0) {
-              // More to go, recurse
-              depth += curr.depth;
-              workIt();
-            } else if (tmp > 0) {
-              // Remove and push back
-              curr.depth += depth;
-              self.lines.level.push(curr);
-            }
+
+  // Going down is { and (   e.g. function() {
+  // going up is } and )
+  let dw = cmd.match(/[{(]/g);
+  let up = cmd.match(/[})]/g);
+  up = up ? up.length : 0;
+  dw = dw ? dw.length : 0;
+  let depth = dw - up;
+
+  if (depth) {
+    (function workIt() {
+      if (depth > 0) {
+        // Going... down.
+        // Push the line#, depth count, and if the line is a function.
+        // Since JS only has functional scope I only need to remove
+        // "function() {" lines, clearly this will not work for
+        // "function()
+        // {" but nothing should break, only tab completion for local
+        // scope will not work for this function.
+        self.lines.level.push({
+          line: self.lines.length - 1,
+          depth: depth,
+          isFunction: /\bfunction\b/.test(cmd)
+        });
+      } else if (depth < 0) {
+        // Going... up.
+        const curr = self.lines.level.pop();
+        if (curr) {
+          const tmp = curr.depth + depth;
+          if (tmp < 0) {
+            // More to go, recurse
+            depth += curr.depth;
+            workIt();
+          } else if (tmp > 0) {
+            // Remove and push back
+            curr.depth += depth;
+            self.lines.level.push(curr);
           }
         }
-      }());
-    }
-
-    // It is possible to determine a syntax error at this point.
-    // if the REPL still has a bufferedCommand and
-    // self.lines.level.length === 0
-    // TODO? keep a log of level so that any syntax breaking lines can
-    // be cleared on .break and in the case of a syntax error?
-    // TODO? if a log was kept, then I could clear the bufferedCommand and
-    // eval these lines and throw the syntax error
-  } else {
-    self.lines.level = [];
+      }
+    }());
   }
 }