Handle out-of-order declarations in scopes. (#5690)

Author: loganfsmyth

src/test/mochitest/browser.ini

@@ -27,6 +27,8 @@ support-files =
   examples/babel/fixtures/modules/output.js.map
   examples/babel/fixtures/non-modules/output.js
   examples/babel/fixtures/non-modules/output.js.map
+  examples/babel/fixtures/out-of-order-declarations/output.js
+  examples/babel/fixtures/out-of-order-declarations/output.js.map
   examples/babel/fixtures/flowtype-bindings/output.js
   examples/babel/fixtures/flowtype-bindings/output.js.map
   examples/babel/fixtures/step-over-for-of/output.js

src/test/mochitest/browser_dbg-babel-scopes.js

@@ -251,6 +251,23 @@ add_task(async function() {
     "thirdModuleScoped()"
   ]);
 
+  await breakpointScopes(dbg, "out-of-order-declarations", { line: 8, column: 4 }, [
+    "callback",
+    "fn()",
+    ["val", "undefined"],
+    "root",
+    ["callback", "(optimized away)"],
+    ["fn", "(optimized away)"],
+    ["val", "(optimized away)"],
+    "Module",
+
+    // This value is currently unmapped because import declarations don't map
+    // very well and ones at the end of the file map especially badly.
+    ["aDefault", "(unmapped)"],
+    ["root", "(optimized away)"],
+    ["val", "(optimized away)"],
+  ]);
+
   await breakpointScopes(dbg, "non-modules", { line: 7, column: 2 }, []);
 
   await breakpointScopes(dbg, "flowtype-bindings", { line: 8, column: 2 }, [

src/test/mochitest/examples/babel/fixtures/out-of-order-declarations/input.js

@@ -0,0 +1,17 @@
+var val = "outer-value";
+
+export default function root() {
+  var val = "middle-value";
+  var fn = function outerFn(outer){};
+
+  function callback() {
+    console.log("pause here", val, aDefault, fn);
+
+    var val = "inner-value";
+    function fn(inner){};
+  }
+
+  callback();
+}
+
+import aDefault from "./src/mod";

src/test/mochitest/examples/babel/fixtures/out-of-order-declarations/output.js

@@ -0,0 +1 @@
+export default "a-default";

src/test/mochitest/examples/babel/fixtures/out-of-order-declarations/output.js.map

@@ -31,6 +31,8 @@
     <button onclick="modules()">Run modules</button>
     <script src="babel/fixtures/non-modules/output.js"></script>
     <button onclick="nonModules()">Run nonModules</button>
+    <script src="babel/fixtures/out-of-order-declarations/output.js"></script>
+    <button onclick="outOfOrderDeclarations()">Run outOfOrderDeclarations</button>
     <script src="babel/fixtures/shadowed-vars/output.js"></script>
     <button onclick="shadowedVars()">Run shadowedVars</button>
     <script src="babel/fixtures/step-over-for-of/output.js"></script>

src/test/mochitest/examples/babel/fixtures/out-of-order-declarations/src/mod.js

@@ -119,6 +119,8 @@ type TempScope = {
 
 type ScopeCollectionVisitorState = {
   sourceId: SourceId,
+  freeVariables: Map<string, Array<BindingLocation>>,
+  freeVariableStack: Array<Map<string, Array<BindingLocation>>>,
   scope: TempScope,
   scopeStack: Array<TempScope>
 };
@@ -133,11 +135,20 @@ export function parseSourceScopes(sourceId: SourceId): ?Array<ParsedScope> {
 
   const state = {
     sourceId,
+    freeVariables: new Map(),
+    freeVariableStack: [],
     scope: lexical,
     scopeStack: []
   };
   t.traverse(ast, scopeCollectionVisitor, state);
 
+  for (const [key, freeVariables] of state.freeVariables) {
+    const binding = global.bindings[key];
+    if (binding) {
+      binding.refs = freeVariables.concat(binding.refs);
+    }
+  }
+
   // TODO: This should probably check for ".mjs" extension on the
   // original file, and should also be skipped if the the generated
   // code is an ES6 module rather than a script.
@@ -206,6 +217,9 @@ function pushTempScope(
   const scope = createTempScope(type, displayName, state.scope, loc);
 
   state.scope = scope;
+
+  state.freeVariableStack.push(state.freeVariables);
+  state.freeVariables = new Map();
   return scope;
 }
 
@@ -305,19 +319,6 @@ function isLexicalVariable(node) {
   return isNode(node, "VariableDeclaration") && isLetOrConst(node);
 }
 
-function findIdentifierInScopes(
-  scope: TempScope,
-  name: string
-): TempScope | null {
-  // Find nearest outer scope with the specifed name and add reference.
-  for (let s = scope; s; s = s.parent) {
-    if (name in s.bindings) {
-      return s;
-    }
-  }
-  return null;
-}
-
 function createGlobalScope(
   ast: BabelNode,
   sourceId: SourceId
@@ -579,25 +580,31 @@ const scopeCollectionVisitor = {
         }
       });
     } else if (t.isIdentifier(node) && t.isReferenced(node, parentNode)) {
-      const identScope = findIdentifierInScopes(state.scope, node.name);
-      if (identScope) {
-        identScope.bindings[node.name].refs.push({
-          type: "ref",
-          start: fromBabelLocation(node.loc.start, state.sourceId),
-          end: fromBabelLocation(node.loc.end, state.sourceId),
-          meta: buildMetaBindings(state.sourceId, node, ancestors)
-        });
+      let freeVariables = state.freeVariables.get(node.name);
+      if (!freeVariables) {
+        freeVariables = [];
+        state.freeVariables.set(node.name, freeVariables);
       }
+
+      freeVariables.push({
+        type: "ref",
+        start: fromBabelLocation(node.loc.start, state.sourceId),
+        end: fromBabelLocation(node.loc.end, state.sourceId),
+        meta: buildMetaBindings(state.sourceId, node, ancestors)
+      });
     } else if (t.isThisExpression(node)) {
-      const identScope = findIdentifierInScopes(state.scope, "this");
-      if (identScope) {
-        identScope.bindings.this.refs.push({
-          type: "ref",
-          start: fromBabelLocation(node.loc.start, state.sourceId),
-          end: fromBabelLocation(node.loc.end, state.sourceId),
-          meta: buildMetaBindings(state.sourceId, node, ancestors)
-        });
+      let freeVariables = state.freeVariables.get("this");
+      if (!freeVariables) {
+        freeVariables = [];
+        state.freeVariables.set("this", freeVariables);
       }
+
+      freeVariables.push({
+        type: "ref",
+        start: fromBabelLocation(node.loc.start, state.sourceId),
+        end: fromBabelLocation(node.loc.end, state.sourceId),
+        meta: buildMetaBindings(state.sourceId, node, ancestors)
+      });
     } else if (t.isClassProperty(parentNode, { value: node })) {
       const scope = pushTempScope(state, "function", "Class Field", {
         start: fromBabelLocation(node.loc.start, state.sourceId),
@@ -628,12 +635,49 @@ const scopeCollectionVisitor = {
     ancestors: TraversalAncestors,
     state: ScopeCollectionVisitorState
   ) {
-    const scope = state.scopeStack.pop();
-    if (!scope) {
+    const currentScope = state.scope;
+    const parentScope = state.scopeStack.pop();
+    if (!parentScope) {
       throw new Error("Assertion failure - unsynchronized pop");
     }
+    state.scope = parentScope;
+
+    // It is possible, as in the case of function expressions, that a single
+    // node has added multiple scopes, so we need to traverse upward here
+    // rather than jumping stright to 'parentScope'.
+    for (
+      let scope = currentScope;
+      scope && scope !== parentScope;
+      scope = scope.parent
+    ) {
+      const freeVariables = state.freeVariables;
+      state.freeVariables = state.freeVariableStack.pop();
+      const parentFreeVariables = state.freeVariables;
+
+      // Match up any free variables that match this scope's bindings and
+      // merge then into the refs.
+      for (const key of Object.keys(scope.bindings)) {
+        const binding = scope.bindings[key];
+
+        const freeVars = freeVariables.get(key);
+        if (freeVars) {
+          binding.refs.push(...freeVars);
+          freeVariables.delete(key);
+        }
+      }
 
-    state.scope = scope;
+      // Move any undeclared references in this scope into the parent for
+      // processing in higher scopes.
+      for (const [key, value] of freeVariables) {
+        let refs = parentFreeVariables.get(key);
+        if (!refs) {
+          refs = [];
+          parentFreeVariables.set(key, refs);
+        }
+
+        refs.push(...value);
+      }
+    }
   }
 };