Use original scope mappings to rewrite preview and watch expressions (#5684)

Author: loganfsmyth

src/actions/expressions.js

@@ -9,7 +9,9 @@ import {
   getExpressions,
   getSelectedFrame,
   getSelectedFrameId,
-  getSource
+  getSource,
+  getSelectedSource,
+  getSelectedScopeMappings
 } from "../selectors";
 import { PROMISE } from "./utils/middleware/promise";
 import { isGeneratedId } from "devtools-source-map";
@@ -115,9 +117,15 @@ function evaluateExpression(expression: Expression) {
       const source = getSource(getState(), location.sourceId);
       const sourceId = source.get("id");
 
-      if (!isGeneratedId(sourceId)) {
+      const selectedSource = getSelectedSource(getState());
+
+      if (
+        selectedSource &&
+        !isGeneratedId(sourceId) &&
+        !isGeneratedId(selectedSource.get("id"))
+      ) {
         input = await getMappedExpression(
-          { sourceMaps },
+          { getState, sourceMaps },
           generatedLocation,
           input
         );
@@ -139,9 +147,14 @@ function evaluateExpression(expression: Expression) {
  * and replaces all posible generated names.
  */
 export async function getMappedExpression(
-  { sourceMaps }: Object,
+  { getState, sourceMaps }: Object,
   generatedLocation: Location,
   expression: string
 ): Promise<string> {
-  return expression;
+  const mappings = getSelectedScopeMappings(getState());
+  if (!mappings) {
+    return expression;
+  }
+
+  return await parser.mapOriginalExpression(expression, mappings);
 }

src/actions/pause/fetchScopes.js

@@ -23,6 +23,6 @@ export function fetchScopes() {
       [PROMISE]: client.getFrameScopes(frame)
     });
 
-    dispatch(mapScopes(scopes, frame));
+    await dispatch(mapScopes(scopes, frame));
   };
 }

src/actions/pause/mapScopes.js

@@ -55,7 +55,7 @@ export function mapScopes(scopes: Promise<Scope>, frame: Frame) {
       !sourceRecord.get("isPrettyPrinted") &&
       !isGeneratedId(frame.location.sourceId);
 
-    dispatch({
+    await dispatch({
       type: "MAP_SCOPES",
       frame,
       [PROMISE]: (async function() {
@@ -88,7 +88,12 @@ async function buildMappedScopes(
   scopes: Scope,
   sourceMaps: any,
   client: any
-): Promise<?OriginalScope> {
+): Promise<?{
+  mappings: {
+    [string]: string
+  },
+  scope: OriginalScope
+}> {
   const originalAstScopes = await getScopes(frame.location);
   const generatedAstScopes = await getScopes(frame.generatedLocation);
 
@@ -102,42 +107,52 @@ async function buildMappedScopes(
     frame.this
   );
 
-  const mappedOriginalScopes = await Promise.all(
-    Array.from(originalAstScopes, async item => {
-      const generatedBindings = {};
+  const expressionLookup = {};
+  const mappedOriginalScopes = [];
 
-      await Promise.all(
-        Object.keys(item.bindings).map(async name => {
-          const binding = item.bindings[name];
+  for (const item of originalAstScopes) {
+    const generatedBindings = {};
 
-          const result = await findGeneratedBinding(
-            sourceMaps,
-            client,
-            source,
-            name,
-            binding,
-            generatedAstBindings
-          );
+    for (const name of Object.keys(item.bindings)) {
+      const binding = item.bindings[name];
 
-          if (result) {
-            generatedBindings[name] = result;
-          }
-        })
+      const result = await findGeneratedBinding(
+        sourceMaps,
+        client,
+        source,
+        name,
+        binding,
+        generatedAstBindings
       );
 
-      return {
-        ...item,
-        generatedBindings
-      };
-    })
-  );
+      if (result) {
+        generatedBindings[name] = result.grip;
+
+        if (
+          binding.refs.length !== 0 &&
+          // These are assigned depth-first, so we don't want shadowed
+          // bindings in parent scopes overwriting the expression.
+          !Object.prototype.hasOwnProperty.call(expressionLookup, name)
+        ) {
+          expressionLookup[name] = result.expression;
+        }
+      }
+    }
+
+    mappedOriginalScopes.push({
+      ...item,
+      generatedBindings
+    });
+  }
 
   const mappedGeneratedScopes = generateClientScope(
     scopes,
     mappedOriginalScopes
   );
 
-  return isReliableScope(mappedGeneratedScopes) ? mappedGeneratedScopes : null;
+  return isReliableScope(mappedGeneratedScopes)
+    ? { mappings: expressionLookup, scope: mappedGeneratedScopes }
+    : null;
 }
 
 /**
@@ -244,7 +259,10 @@ async function findGeneratedBinding(
   name: string,
   originalBinding: BindingData,
   generatedAstBindings: Array<GeneratedBindingLocation>
-): Promise<?BindingContents> {
+): Promise<?{
+  grip: BindingContents,
+  expression: string | null
+}> {
   // If there are no references to the implicits, then we have no way to
   // even attempt to map it back to the original since there is no location
   // data to use. Bail out instead of just showing it as unmapped.
@@ -275,7 +293,10 @@ async function findGeneratedBinding(
   }, null);
 
   if (genContent && genContent.desc) {
-    return genContent.desc;
+    return {
+      grip: genContent.desc,
+      expression: genContent.expression
+    };
   } else if (genContent) {
     // If there is no descriptor for 'this', then this is not the top-level
     // 'this' that the server gave us a binding for, and we can just ignore it.
@@ -287,17 +308,20 @@ async function findGeneratedBinding(
     // means that the server scope information didn't match the scope
     // information from the DevTools parsed scopes.
     return {
-      configurable: false,
-      enumerable: true,
-      writable: false,
-      value: {
-        type: "unscoped",
-        unscoped: true,
-
-        // HACK: Until support for "unscoped" lands in devtools-reps,
-        // this will make these show as (unavailable).
-        missingArguments: true
-      }
+      grip: {
+        configurable: false,
+        enumerable: true,
+        writable: false,
+        value: {
+          type: "unscoped",
+          unscoped: true,
+
+          // HACK: Until support for "unscoped" lands in devtools-reps,
+          // this will make these show as (unavailable).
+          missingArguments: true
+        }
+      },
+      expression: null
     };
   }
 
@@ -306,17 +330,20 @@ async function findGeneratedBinding(
   // of some scope, but the generated location is outside, leading
   // us to search for bindings that don't technically exist.
   return {
-    configurable: false,
-    enumerable: true,
-    writable: false,
-    value: {
-      type: "unmapped",
-      unmapped: true,
-
-      // HACK: Until support for "unmapped" lands in devtools-reps,
-      // this will make these show as (unavailable).
-      missingArguments: true
-    }
+    grip: {
+      configurable: false,
+      enumerable: true,
+      writable: false,
+      value: {
+        type: "unmapped",
+        unmapped: true,
+
+        // HACK: Until support for "unmapped" lands in devtools-reps,
+        // this will make these show as (unavailable).
+        missingArguments: true
+      }
+    },
+    expression: null
   };
 }
 

src/actions/pause/paused.js

@@ -70,10 +70,6 @@ export function paused(pauseInfo: Pause) {
       dispatch(removeBreakpoint(hiddenBreakpointLocation));
     }
 
-    if (!isEvaluatingExpression(getState())) {
-      dispatch(evaluateExpressions());
-    }
-
     await dispatch(mapFrames());
     const selectedFrame = getSelectedFrame(getState());
 
@@ -86,6 +82,12 @@ export function paused(pauseInfo: Pause) {
     }
 
     dispatch(togglePaneCollapse("end", false));
-    dispatch(fetchScopes());
+    await dispatch(fetchScopes());
+
+    // Run after fetching scoping data so that it may make use of the sourcemap
+    // expression mappings for local variables.
+    if (!isEvaluatingExpression(getState())) {
+      dispatch(evaluateExpressions());
+    }
   };
 }

src/actions/preview.js

@@ -158,7 +158,7 @@ export function setPreview(
           );
 
           expression = await getMappedExpression(
-            { sourceMaps },
+            { sourceMaps, getState },
             generatedLocation,
             expression
           );

src/actions/types.js

@@ -300,7 +300,12 @@ type PauseAction =
       type: "MAP_SCOPES",
       frame: Frame,
       status: AsyncStatus,
-      value: Scope
+      value: {
+        scope: Scope,
+        mappings: {
+          [string]: string | null
+        }
+      }
     }
   | {
       type: "MAP_FRAMES",

src/reducers/pause.js

@@ -35,6 +35,11 @@ export type PauseState = {
         pending: boolean,
         scope: OriginalScope
       }
+    },
+    mappings: {
+      [FrameId]: {
+        [string]: string | null
+      }
     }
   },
   selectedFrameId: ?string,
@@ -57,7 +62,8 @@ export const createPauseState = (): PauseState => ({
   selectedFrameId: undefined,
   frameScopes: {
     generated: {},
-    original: {}
+    original: {},
+    mappings: {}
   },
   loadedObjects: {},
   shouldPauseOnExceptions: prefs.pauseOnExceptions,
@@ -73,7 +79,8 @@ const emptyPauseState = {
   frames: null,
   frameScopes: {
     generated: {},
-    original: {}
+    original: {},
+    mappings: {}
   },
   selectedFrameId: null,
   loadedObjects: {},
@@ -143,14 +150,20 @@ function update(
         ...state.frameScopes.original,
         [selectedFrameId]: {
           pending: status !== "done",
-          scope: value
+          scope: value && value.scope
         }
       };
+
+      const mappings = {
+        ...state.frameScopes.mappings,
+        [selectedFrameId]: value && value.mappings
+      };
       return {
         ...state,
         frameScopes: {
           ...state.frameScopes,
-          original
+          original,
+          mappings
         }
       };
     }
@@ -347,6 +360,19 @@ export function getSelectedScope(state: OuterState) {
   return scope || null;
 }
 
+export function getSelectedScopeMappings(
+  state: OuterState
+): {
+  [string]: string | null
+} | null {
+  const frameId = getSelectedFrameId(state);
+  if (!frameId) {
+    return null;
+  }
+
+  return getFrameScopes(state).mappings[frameId];
+}
+
 export function getSelectedFrameId(state: OuterState) {
   return state.pause.selectedFrameId;
 }

src/test/mochitest/browser.ini

@@ -128,6 +128,7 @@ support-files =
 [browser_dbg-async-stepping.js]
 [browser_dbg-babel-scopes.js]
 [browser_dbg-babel-stepping.js]
+[browser_dbg-babel-preview.js]
 [browser_dbg-breaking.js]
 [browser_dbg-breaking-from-console.js]
 [browser_dbg-breakpoints.js]