fix: sourcemap renames replacing in invalid contexts

Fixes #1201

Author: connor4312

CHANGELOG.md

@@ -4,7 +4,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 
 ## Nightly only
 
-Nothing (yet)
+- fix: sourcemap renames replacing in invalid contexts ([#1201](https://github.com/microsoft/vscode-js-debug/issues/1201))
 
 ## v1.65 (March 2022)
 

src/adapter/evaluator.test.ts

@@ -0,0 +1,105 @@
+/*---------------------------------------------------------
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ *--------------------------------------------------------*/
+
+import { expect } from 'chai';
+import Cdp from '../cdp/api';
+import { stubbedCdpApi, StubCdpApi } from '../cdp/stubbedApi';
+import { Base01Position } from '../common/positions';
+import { RenameMapping } from '../common/sourceMaps/renameProvider';
+import { Evaluator } from './evaluator';
+
+describe('Evaluator', () => {
+  let evaluator: Evaluator;
+  let stubCdp: StubCdpApi;
+  let renameMapping: RenameMapping;
+
+  const result: Cdp.Debugger.EvaluateOnCallFrameResult = {
+    result: {
+      type: 'string',
+      value: 'foo',
+    },
+  };
+
+  beforeEach(() => {
+    stubCdp = stubbedCdpApi();
+    renameMapping = RenameMapping.None;
+    evaluator = new Evaluator(stubCdp.actual, {
+      provideForSource: () => renameMapping,
+      provideOnStackframe: () => renameMapping,
+    });
+  });
+
+  it('prepares simple expressions', async () => {
+    const prep = evaluator.prepare('foo', { isInternalScript: false });
+    expect(prep.canEvaluateDirectly).to.be.true;
+    stubCdp.Debugger.evaluateOnCallFrame.resolves(result);
+    expect(await prep.invoke({ callFrameId: '' })).to.equal(result);
+    expect(stubCdp.Debugger.evaluateOnCallFrame.args[0][0]).to.deep.equal({
+      callFrameId: '',
+      expression: 'foo',
+    });
+  });
+
+  it('appends eval source url to internal', async () => {
+    const prep = evaluator.prepare('foo');
+    expect(prep.canEvaluateDirectly).to.be.true;
+    stubCdp.Debugger.evaluateOnCallFrame.resolves(result);
+    expect(await prep.invoke({ callFrameId: '' })).to.equal(result);
+    expect(stubCdp.Debugger.evaluateOnCallFrame.args[0][0].expression).to.match(
+      /^foo\n\/\/# sourceURL=eval/m,
+    );
+  });
+
+  it('replaces renamed identifiers', async () => {
+    const prep = evaluator.prepare('foo', {
+      isInternalScript: false,
+      renames: {
+        mapping: new RenameMapping([
+          { original: 'foo', compiled: 'bar', position: new Base01Position(0, 1) },
+        ]),
+        position: new Base01Position(0, 1),
+      },
+    });
+    expect(prep.canEvaluateDirectly).to.be.true;
+    stubCdp.Debugger.evaluateOnCallFrame.resolves(result);
+    expect(await prep.invoke({ callFrameId: '' })).to.equal(result);
+    expect(stubCdp.Debugger.evaluateOnCallFrame.args[0][0]).to.deep.equal({
+      callFrameId: '',
+      expression: 'typeof bar !== "undefined" ? bar : foo;\n',
+    });
+  });
+
+  it('does not replace identifiers in invalid contexts', async () => {
+    const prep = evaluator.prepare(
+      `const baz = foo;
+z.find(foo => true)
+const { foo } = z;
+for (const { foo } of z) {}
+try {} catch ({ foo }) {}`,
+      {
+        isInternalScript: false,
+        renames: {
+          mapping: new RenameMapping([
+            { original: 'foo', compiled: 'bar', position: new Base01Position(0, 1) },
+          ]),
+          position: new Base01Position(0, 1),
+        },
+      },
+    );
+    expect(prep.canEvaluateDirectly).to.be.true;
+    stubCdp.Debugger.evaluateOnCallFrame.resolves(result);
+    expect(await prep.invoke({ callFrameId: '' })).to.equal(result);
+    expect(stubCdp.Debugger.evaluateOnCallFrame.args[0][0]).to.deep.equal({
+      callFrameId: '',
+      expression: [
+        `const baz = typeof bar !== \"undefined\" ? bar : foo;`,
+        `z.find(bar => true);`,
+        `const {bar} = z;`,
+        `for (const {bar} of z) {}`,
+        `try {} catch ({bar}) {}`,
+        '',
+      ].join('\n'),
+    });
+  });
+});

src/adapter/evaluator.ts

@@ -13,6 +13,7 @@ import { ICdpApi } from '../cdp/connection';
 import { IPosition } from '../common/positions';
 import { parseProgram } from '../common/sourceCodeManipulations';
 import { IRenameProvider, RenameMapping } from '../common/sourceMaps/renameProvider';
+import { isInPatternSlot } from '../common/sourceUtils';
 import { StackFrame } from './stackTrace';
 import { getSourceSuffix } from './templates';
 
@@ -274,7 +275,7 @@ export class Evaluator implements IEvaluator {
 
     const parents: Node[] = [];
     const transformed = replace(parseProgram(expr), {
-      enter(node) {
+      enter(node, parent) {
         const asAcorn = node as AcornNode;
         if (node.type !== 'Identifier' || expr[asAcorn.start - 1] === '.') {
           return;
@@ -285,14 +286,18 @@ export class Evaluator implements IEvaluator {
           hoisted.add(node.name);
           mutated = true;
           this.skip();
-          return replacement(hoistName, undefinedExpression);
+          return isInPatternSlot(node, parent)
+            ? { type: 'Identifier', name: hoistName }
+            : replacement(hoistName, undefinedExpression);
         }
 
         const cname = renames?.mapping.getCompiledName(node.name, renames.position);
         if (cname) {
           mutated = true;
           this.skip();
-          return replacement(cname, node);
+          return isInPatternSlot(node, parent)
+            ? { type: 'Identifier', name: cname }
+            : replacement(cname, node);
         }
       },
       leave: () => {

src/common/sourceUtils.ts

@@ -5,7 +5,15 @@
 import { parseExpressionAt, Parser } from 'acorn';
 import { generate } from 'astring';
 import { replace, VisitorOption } from 'estraverse';
-import { Expression, ExpressionStatement, Identifier, Program, Statement } from 'estree';
+import {
+  Expression,
+  ExpressionStatement,
+  Identifier,
+  Node,
+  Pattern,
+  Program,
+  Statement,
+} from 'estree';
 import { promises as fsPromises } from 'fs';
 import { NullablePosition, Position, SourceMapConsumer, SourceMapGenerator } from 'source-map';
 import { LineColumn } from '../adapter/breakpoints/breakpointBase';
@@ -321,6 +329,25 @@ export function getOptimalCompiledPosition(
   return allLocations[0][0];
 }
 
+/**
+ * Returns if the given node is in a slot reserved for a Pattern where an
+ * Expression node could not go.
+ */
+export const isInPatternSlot = (node: Pattern, parent: Node | null | undefined): boolean =>
+  !!parent &&
+  (((parent.type === 'FunctionExpression' ||
+    parent.type === 'FunctionDeclaration' ||
+    parent.type === 'ArrowFunctionExpression') &&
+    parent.params.includes(node)) ||
+    ((parent.type === 'ForInStatement' || parent.type === 'ForOfStatement') &&
+      parent.left === node) ||
+    (parent.type === 'VariableDeclarator' && parent.id === node) ||
+    (parent.type === 'AssignmentPattern' && parent.left === node) ||
+    (parent.type === 'CatchClause' && parent.param === node) ||
+    ('kind' in parent && parent.kind === 'init' && parent.value === node) ||
+    (parent.type === 'RestElement' && parent.argument === node) ||
+    (parent.type === 'AssignmentPattern' && parent.left === node));
+
 /**
  * Returns the syntax error in the given code, if any.
  */