feat: show correct values of shadowed variables in hovers

CHANGELOG.md

@@ -4,6 +4,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he
 
 ## Nightly (only)
 
+- feat: show correct values of shadowed variables in hovers ([#2022](https://github.com/microsoft/vscode-js-debug/issues/2022))
 - fix: hanging on certain Linux environments ([vscode#214872](https://github.com/microsoft/vscode/issues/214872))
 
 ## v1.90 (May 2024)

package.json

@@ -157,7 +157,7 @@
     "stream-buffers": "^3.0.2",
     "ts-node": "^10.9.2",
     "tsx": "^4.7.0",
-    "typescript": "^5.3.3",
+    "typescript": "^5.5.2",
     "vsce": "^2.7.0"
   },
   "main": "./src/extension.js",

src/adapter/evaluator.ts

@@ -9,12 +9,15 @@ import { ConditionalExpression, Expression } from 'estree';
 import { inject, injectable } from 'inversify';
 import Cdp from '../cdp/api';
 import { ICdpApi } from '../cdp/connection';
-import { IPosition } from '../common/positions';
+import { Base1Position, IPosition, Range } from '../common/positions';
+import { findIndexAsync } from '../common/promiseUtil';
 import { parseProgram, replace } from '../common/sourceCodeManipulations';
 import { IRenameProvider, RenameMapping } from '../common/sourceMaps/renameProvider';
 import { isInPatternSlot } from '../common/sourceUtils';
+import { Source } from './source';
 import { StackFrame } from './stackTrace';
 import { getSourceSuffix } from './templates';
+import { VariableStore } from './variableStore';
 
 export const returnValueStr = '$returnValue';
 
@@ -24,12 +27,16 @@ const makeHoistedName = () => hoistedPrefix + randomBytes(8).toString('hex');
 
 export const IEvaluator = Symbol('IEvaluator');
 
+export type PreparedHoistFn = (
+  variable: string,
+) => Promise<Cdp.Runtime.RemoteObject | undefined> | Cdp.Runtime.RemoteObject | undefined;
+
 /**
  * Prepared call that can be invoked later on a callframe..
  */
 export type PreparedCallFrameExpr = (
   params: Omit<Cdp.Debugger.EvaluateOnCallFrameParams, 'expression'>,
-  hoisted?: { [key: string]: Cdp.Runtime.RemoteObject },
+  hoist?: PreparedHoistFn,
 ) => Promise<Cdp.Debugger.EvaluateOnCallFrameResult | undefined>;
 
 /**
@@ -109,18 +116,24 @@ export interface IPrepareOptions extends IEvaluatorBaseOptions {
 
 export type RenamePrepareOptions = { position: IPosition; mapping: RenameMapping };
 
-export interface IEvaluateOptions extends IEvaluatorBaseOptions {
-  /**
-   * Replaces the identifiers in the associated script with references to the
-   * given remote objects.
-   */
-  hoist?: ReadonlyArray<string>;
+export type LocationEvaluateOptions = {
+  source: Source;
+  position: IPosition;
+  variables: VariableStore;
+};
 
+export interface IEvaluateOptions extends IEvaluatorBaseOptions {
   /**
    * Stack frame object on which the evaluation is being run. This is
    * necessary to allow for renamed properties.
    */
   stackFrame?: StackFrame;
+
+  /**
+   * A manually-provided source location for the evaluation, as an alternative
+   * to {@link stackFrame}
+   */
+  location?: LocationEvaluateOptions;
 }
 
 /**
@@ -181,10 +194,13 @@ export class Evaluator implements IEvaluator {
 
     return {
       canEvaluateDirectly: false,
-      invoke: (params, hoistMap = {}) =>
+      invoke: (params, doHoist) =>
         Promise.all(
-          [...toHoist].map(([ident, hoisted]) =>
-            this.hoistValue(ident === returnValueStr ? this.returnValue : hoistMap[ident], hoisted),
+          [...toHoist].map(async ([ident, hoisted]) =>
+            this.hoistValue(
+              ident === returnValueStr ? this.returnValue : await doHoist?.(ident),
+              hoisted,
+            ),
           ),
         ).then(() => this.cdp.Debugger.evaluateOnCallFrame({ ...params, expression: transformed })),
     };
@@ -202,29 +218,49 @@ export class Evaluator implements IEvaluator {
   ): Promise<Cdp.Runtime.EvaluateResult>;
   public async evaluate(
     params: Cdp.Debugger.EvaluateOnCallFrameParams | Cdp.Runtime.EvaluateParams,
-    options?: IEvaluateOptions,
+    options: IEvaluateOptions = {},
   ) {
     // no call frame means there will not be any relevant $returnValue to reference
     if (!('callFrameId' in params)) {
       return this.cdp.Runtime.evaluate(params);
     }
 
-    let prepareOptions: IPrepareOptions | undefined = options;
-    if (options?.stackFrame) {
-      const mapping = await this.renameProvider.provideOnStackframe(options.stackFrame);
-      prepareOptions = {
-        ...prepareOptions,
-        renames: { mapping, position: options.stackFrame.rawPosition },
-      };
+    const prepareOptions: IPrepareOptions | undefined = { ...options };
+    const { location, stackFrame } = options;
+    let hoist: PreparedHoistFn | undefined;
+    if (location) {
+      await Promise.all([
+        // 1. Get the rename mapping at the desired position
+        Promise.resolve(this.renameProvider.provideForSource(location.source)).then(mapping => {
+          prepareOptions.renames = { mapping, position: location.position };
+        }),
+        // 2. Hoist variables that may be shadowed. Identify the scope containing
+        // the location and mark any variables that appear in a higher scope
+        // (and therefore could be shadowed) as hoistable.
+        stackFrame &&
+          this.setupShadowedVariableHoisting(location, stackFrame).then(r => {
+            if (r) {
+              hoist = r.doHoist;
+              prepareOptions.hoist = [...r.hoistable];
+            }
+          }),
+      ]);
+    } else if (stackFrame) {
+      const mapping = await this.renameProvider.provideOnStackframe(stackFrame);
+      prepareOptions.renames = { mapping, position: stackFrame.rawPosition };
     }
 
-    return this.prepare(params.expression, prepareOptions).invoke(params);
+    return this.prepare(params.expression, prepareOptions).invoke(params, hoist);
   }
 
   /**
    * Hoists the return value of the expression to the `globalThis`.
    */
   public async hoistValue(object: Cdp.Runtime.RemoteObject | undefined, hoistedVar: string) {
+    if (object === undefined) {
+      return;
+    }
+
     const objectId = object?.objectId;
     const dehoist = `setTimeout(() => { delete globalThis.${hoistedVar} }, 0)`;
 
@@ -243,6 +279,61 @@ export class Evaluator implements IEvaluator {
     }
   }
 
+  /**
+   * Returns shadowed variables at the given location in the stack's scopes
+   * and a function that can be used to hoist the variables.
+   *
+   * It does this by identifying the scope the evaluation is being run in,
+   * marking all variables found in scopes above it as hoistable, and
+   * creating a function that will return the RemoteObject of a given
+   * shadowed variable.
+   */
+  private async setupShadowedVariableHoisting(
+    { position, source, variables }: LocationEvaluateOptions,
+    stackFrame: StackFrame,
+  ) {
+    const { scopes } = await stackFrame.scopes();
+
+    const scopeIndex = await findIndexAsync(
+      scopes,
+      async s =>
+        s.source &&
+        s.line &&
+        s.endLine &&
+        new Range(
+          new Base1Position(s.line, s.column || 1),
+          new Base1Position(s.endLine, s.endColumn || Infinity),
+        ).contains(position) &&
+        (await source.equalsDap(s.source)),
+    );
+    if (scopeIndex === -1) {
+      return;
+    }
+
+    const hoistable = new Set<string>();
+    await Promise.all(
+      scopes.slice(0, scopeIndex).map(async s => {
+        const vars = await variables.getVariableNames({
+          variablesReference: s.variablesReference,
+        });
+        for (const { name } of vars) {
+          hoistable.add(name);
+        }
+      }),
+    );
+
+    const doHoist: PreparedHoistFn = async variable => {
+      for (let i = scopeIndex; i < scopes.length; i++) {
+        const vars = await variables.getVariableNames({
+          variablesReference: scopes[i].variablesReference,
+        });
+        return vars.find(v => v.name === variable)?.remoteObject;
+      }
+    };
+
+    return { hoistable, doHoist };
+  }
+
   /**
    * Replaces a variable in the given expression with the `hoisted` variable,
    * returning the identifiers which were hoisted.

src/adapter/exceptionPauseService.ts

@@ -168,7 +168,9 @@ export class ExceptionPauseService implements IExceptionPauseService {
   }
 
   private async evalCondition(evt: Cdp.Debugger.PausedEvent, method: PreparedCallFrameExpr) {
-    const r = await method({ callFrameId: evt.callFrames[0].callFrameId }, { error: evt.data });
+    const r = await method({ callFrameId: evt.callFrames[0].callFrameId }, v =>
+      v === 'error' ? evt.data : undefined,
+    );
     return !!r?.result.value;
   }
 

src/adapter/source.ts

@@ -143,6 +143,13 @@ export class Source {
     return obj;
   }
 
+  public async equalsDap(s: Dap.Source) {
+    const existingAbsolutePath = await this._existingAbsolutePath;
+    return existingAbsolutePath
+      ? !s.sourceReference && existingAbsolutePath === s.path
+      : s.sourceReference === this.sourceReference;
+  }
+
   private setSourceMapUrl(sourceMapMetadata?: ISourceMapMetadata) {
     if (!sourceMapMetadata) {
       this.sourceMap = undefined;

src/adapter/stackTrace.ts

@@ -377,6 +377,11 @@ export class StackFrame implements IStackFrameElement {
     return new Base1Position(this._rawLocation.lineNumber, this._rawLocation.columnNumber);
   }
 
+  /** Raw chain from the runtime, applicable only to debug-triggered traces */
+  public get rawScopeChain() {
+    return this._scope?.chain || [];
+  }
+
   static fromRuntime(
     thread: Thread,
     callFrame: Cdp.Runtime.CallFrame,
@@ -674,7 +679,7 @@ export class StackFrame implements IStackFrameElement {
           .getVariableNames({
             variablesReference: scopeVariable.id,
           })
-          .then(variables => variables.map(label => ({ label, type: 'property' }))),
+          .then(variables => variables.map(({ name }) => ({ label: name, type: 'property' }))),
       );
     }
     const completions = await Promise.all(promises);

src/adapter/threads.ts

@@ -30,7 +30,7 @@ import { UserDefinedBreakpoint } from './breakpoints/userDefinedBreakpoint';
 import { ICompletions } from './completions';
 import { ExceptionMessage, IConsole, QueryObjectsMessage } from './console';
 import { customBreakpoints } from './customBreakpoints';
-import { IEvaluator } from './evaluator';
+import { IEvaluator, LocationEvaluateOptions } from './evaluator';
 import { IExceptionPauseService } from './exceptionPauseService';
 import * as objectPreview from './objectPreview';
 import { PreviewContextType, getContextForType } from './objectPreview/contexts';
@@ -524,7 +524,6 @@ export class Thread implements IVariableStoreLocationProvider {
     args: Dap.EvaluateParamsExtended,
     variables: VariableStore,
   ): Promise<Dap.Variable> {
-    const callFrameId = stackFrame?.root.callFrameId();
     // For clipboard evaluations, return a safe JSON-stringified string.
     const params: Cdp.Runtime.EvaluateParams =
       args.context === 'clipboard'
@@ -560,14 +559,24 @@ export class Thread implements IVariableStoreLocationProvider {
         type: 'evaluation',
       });
 
+    let location: LocationEvaluateOptions | undefined;
+    if (args.source && args.line && args.column) {
+      const variables = this._pausedVariables;
+      const source = this._sourceContainer.source(args.source);
+      if (source && variables) {
+        location = { source, position: new Base1Position(args.line, args.column), variables };
+      }
+    }
+
+    const callFrameId = stackFrame?.root.callFrameId();
     const responsePromise = this.evaluator.evaluate(
       callFrameId
         ? { ...params, callFrameId }
         : {
             ...params,
             contextId: this._selectedContext ? this._selectedContext.description.id : undefined,
           },
-      { isInternalScript: false, stackFrame: stackFrame?.root },
+      { isInternalScript: false, stackFrame: stackFrame?.root, location },
     );
 
     // Report result for repl immediately so that the user could see the expression they entered.

src/adapter/variableStore.ts

@@ -545,7 +545,7 @@ class Variable implements IVariable {
 
   constructor(
     protected readonly context: VariableContext,
-    protected readonly remoteObject: Cdp.Runtime.RemoteObject,
+    public readonly remoteObject: Cdp.Runtime.RemoteObject,
   ) {}
 
   /**
@@ -1392,14 +1392,16 @@ export class VariableStore {
    * Gets variable names from a known {@link IVariableContainer}. An optimized
    * version of `getVariables` that saves work generating previews.
    */
-  public async getVariableNames(params: Dap.VariablesParams): Promise<string[]> {
+  public async getVariableNames(
+    params: Dap.VariablesParams,
+  ): Promise<{ name: string; remoteObject: Cdp.Runtime.RemoteObject }[]> {
     const container = this.vars.get(params.variablesReference);
     if (!container) {
       return [];
     }
 
     const children = await container.getChildren(params);
-    return children.filter(isInstanceOf(Variable)).map(v => v.name);
+    return children.filter(isInstanceOf(Variable));
   }
 
   /** Gets variables from a known {@link IVariableContainer} */

src/common/promiseUtil.ts

@@ -46,6 +46,19 @@ export function some<T>(
   });
 }
 
+export async function findIndexAsync<T>(
+  array: ReadonlyArray<T>,
+  predicate: (item: T) => Promise<unknown>,
+): Promise<number> {
+  for (let i = 0; i < array.length; i++) {
+    if (await predicate(array[i])) {
+      return i;
+    }
+  }
+
+  return -1;
+}
+
 export function getDeferred<T>(): IDeferred<T> {
   // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
   let resolve: IDeferred<T>['resolve'] = null!;