chore: Refactor and document CodePath (#17558)

nzakas · web-flow · commit 24e1f140ec68 · 2023-09-25T22:18:35.000+02:00
* chore: Refactor and document CodePath

* Clarify docs
diff --git a/lib/linter/code-path-analysis/code-path.js b/lib/linter/code-path-analysis/code-path.js
@@ -80,42 +80,58 @@ class CodePath {
     }
 
     /**
-     * The initial code path segment.
+     * The initial code path segment. This is the segment that is at the head
+     * of the code path.
+     * This is a passthrough to the underlying `CodePathState`.
      * @type {CodePathSegment}
      */
     get initialSegment() {
         return this.internal.initialSegment;
     }
 
     /**
-     * Final code path segments.
-     * This array is a mix of `returnedSegments` and `thrownSegments`.
+     * Final code path segments. These are the terminal (tail) segments in the
+     * code path, which is the combination of `returnedSegments` and `thrownSegments`.
+     * All segments in this array are reachable.
+     * This is a passthrough to the underlying `CodePathState`.
      * @type {CodePathSegment[]}
      */
     get finalSegments() {
         return this.internal.finalSegments;
     }
 
     /**
-     * Final code path segments which is with `return` statements.
-     * This array contains the last path segment if it's reachable.
-     * Since the reachable last path returns `undefined`.
+     * Final code path segments that represent normal completion of the code path.
+     * For functions, this means both explicit `return` statements and implicit returns,
+     * such as the last reachable segment in a function that does not have an
+     * explicit `return` as this implicitly returns `undefined`. For scripts,
+     * modules, class field initializers, and class static blocks, this means
+     * all lines of code have been executed.
+     * These segments are also present in `finalSegments`.
+     * This is a passthrough to the underlying `CodePathState`.
      * @type {CodePathSegment[]}
      */
     get returnedSegments() {
         return this.internal.returnedForkContext;
     }
 
     /**
-     * Final code path segments which is with `throw` statements.
+     * Final code path segments that represent `throw` statements.
+     * This is a passthrough to the underlying `CodePathState`.
+     * These segments are also present in `finalSegments`.
      * @type {CodePathSegment[]}
      */
     get thrownSegments() {
         return this.internal.thrownForkContext;
     }
 
     /**
-     * Current code path segments.
+     * Tracks the traversal of the code path through each segment. This array
+     * starts empty and segments are added or removed as the code path is
+     * traversed. This array always ends up empty at the end of a code path
+     * traversal. The `CodePathState` uses this to track its progress through
+     * the code path.
+     * This is a passthrough to the underlying `CodePathState`.
      * @type {CodePathSegment[]}
      * @deprecated
      */
@@ -126,79 +142,123 @@ class CodePath {
     /**
      * Traverses all segments in this code path.
      *
-     *     codePath.traverseSegments(function(segment, controller) {
+     *     codePath.traverseSegments((segment, controller) => {
      *         // do something.
      *     });
      *
      * This method enumerates segments in order from the head.
      *
-     * The `controller` object has two methods.
+     * The `controller` argument has two methods:
      *
-     * - `controller.skip()` - Skip the following segments in this branch.
-     * - `controller.break()` - Skip all following segments.
-     * @param {Object} [options] Omittable.
-     * @param {CodePathSegment} [options.first] The first segment to traverse.
-     * @param {CodePathSegment} [options.last] The last segment to traverse.
+     * - `skip()` - skips the following segments in this branch
+     * - `break()` - skips all following segments in the traversal
+     *
+     * A note on the parameters: the `options` argument is optional. This means
+     * the first argument might be an options object or the callback function.
+     * @param {Object} [optionsOrCallback] Optional first and last segments to traverse.
+     * @param {CodePathSegment} [optionsOrCallback.first] The first segment to traverse.
+     * @param {CodePathSegment} [optionsOrCallback.last] The last segment to traverse.
      * @param {Function} callback A callback function.
      * @returns {void}
      */
-    traverseSegments(options, callback) {
+    traverseSegments(optionsOrCallback, callback) {
+
+        // normalize the arguments into a callback and options
         let resolvedOptions;
         let resolvedCallback;
 
-        if (typeof options === "function") {
-            resolvedCallback = options;
+        if (typeof optionsOrCallback === "function") {
+            resolvedCallback = optionsOrCallback;
             resolvedOptions = {};
         } else {
-            resolvedOptions = options || {};
+            resolvedOptions = optionsOrCallback || {};
             resolvedCallback = callback;
         }
 
+        // determine where to start traversing from based on the options
         const startSegment = resolvedOptions.first || this.internal.initialSegment;
         const lastSegment = resolvedOptions.last;
 
-        let item = null;
+        // set up initial location information
+        let record = null;
         let index = 0;
         let end = 0;
         let segment = null;
-        const visited = Object.create(null);
+
+        // segments that have already been visited during traversal
+        const visited = new Set();
+
+        // tracks the traversal steps
         const stack = [[startSegment, 0]];
+
+        // tracks the last skipped segment during traversal
         let skippedSegment = null;
+
+        // indicates if we exited early from the traversal
         let broken = false;
+
+        /**
+         * Maintains traversal state.
+         */
         const controller = {
+
+            /**
+             * Skip the following segments in this branch.
+             * @returns {void}
+             */
             skip() {
                 if (stack.length <= 1) {
                     broken = true;
                 } else {
                     skippedSegment = stack[stack.length - 2][0];
                 }
             },
+
+            /**
+             * Stop traversal completely - do not traverse to any
+             * other segments.
+             * @returns {void}
+             */
             break() {
                 broken = true;
             }
         };
 
         /**
-         * Checks a given previous segment has been visited.
+         * Checks if a given previous segment has been visited.
          * @param {CodePathSegment} prevSegment A previous segment to check.
          * @returns {boolean} `true` if the segment has been visited.
          */
         function isVisited(prevSegment) {
             return (
-                visited[prevSegment.id] ||
+                visited.has(prevSegment) ||
                 segment.isLoopedPrevSegment(prevSegment)
             );
         }
 
+        // the traversal
         while (stack.length > 0) {
-            item = stack[stack.length - 1];
-            segment = item[0];
-            index = item[1];
+
+            /*
+             * This isn't a pure stack. We use the top record all the time
+             * but don't always pop it off. The record is popped only if
+             * one of the following is true:
+             *
+             * 1) We have already visited the segment.
+             * 2) We have not visited *all* of the previous segments.
+             * 3) We have traversed past the available next segments.
+             *
+             * Otherwise, we just read the value and sometimes modify the
+             * record as we traverse.
+             */
+            record = stack[stack.length - 1];
+            segment = record[0];
+            index = record[1];
 
             if (index === 0) {
 
                 // Skip if this segment has been visited already.
-                if (visited[segment.id]) {
+                if (visited.has(segment)) {
                     stack.pop();
                     continue;
                 }
@@ -212,18 +272,29 @@ class CodePath {
                     continue;
                 }
 
-                // Reset the flag of skipping if all branches have been skipped.
+                // Reset the skipping flag if all branches have been skipped.
                 if (skippedSegment && segment.prevSegments.includes(skippedSegment)) {
                     skippedSegment = null;
                 }
-                visited[segment.id] = true;
+                visited.add(segment);
 
-                // Call the callback when the first time.
+                /*
+                 * If the most recent segment hasn't been skipped, then we call
+                 * the callback, passing in the segment and the controller.
+                 */
                 if (!skippedSegment) {
                     resolvedCallback.call(this, segment, controller);
+
+                    // exit if we're at the last segment
                     if (segment === lastSegment) {
                         controller.skip();
                     }
+
+                    /*
+                     * If the previous statement was executed, or if the callback
+                     * called a method on the controller, we might need to exit the
+                     * loop, so check for that and break accordingly.
+                     */
                     if (broken) {
                         break;
                     }
@@ -233,12 +304,35 @@ class CodePath {
             // Update the stack.
             end = segment.nextSegments.length - 1;
             if (index < end) {
-                item[1] += 1;
+
+                /*
+                 * If we haven't yet visited all of the next segments, update
+                 * the current top record on the stack to the next index to visit
+                 * and then push a record for the current segment on top.
+                 *
+                 * Setting the current top record's index lets us know how many
+                 * times we've been here and ensures that the segment won't be
+                 * reprocessed (because we only process segments with an index
+                 * of 0).
+                 */
+                record[1] += 1;
                 stack.push([segment.nextSegments[index], 0]);
             } else if (index === end) {
-                item[0] = segment.nextSegments[index];
-                item[1] = 0;
+
+                /*
+                 * If we are at the last next segment, then reset the top record
+                 * in the stack to next segment and set its index to 0 so it will
+                 * be processed next.
+                 */
+                record[0] = segment.nextSegments[index];
+                record[1] = 0;
             } else {
+
+                /*
+                 * If index > end, that means we have no more segments that need
+                 * processing. So, we pop that record off of the stack in order to
+                 * continue traversing at the next level up.
+                 */
                 stack.pop();
             }
         }
