Change how the builder handles definition OriginalScopes

Author: szuend

src/builder/builder.test.ts

@@ -168,9 +168,10 @@ describe("ScopeInfoBuilder", () => {
 
   describe("startRange", () => {
     it("sets the definition scope when it's provided as a number", () => {
-      const info = builder.startScope(0, 0).endScope(10, 0).startRange(0, 0, {
-        scope: 0,
-      }).endRange(0, 10).build();
+      const info = builder.startScope(0, 0, { key: 0 }).endScope(10, 0)
+        .startRange(0, 0, {
+          scopeKey: 0,
+        }).endRange(0, 10).build();
 
       assertStrictEquals(info.scopes[0], info.ranges[0].originalScope);
     });
@@ -204,24 +205,32 @@ describe("ScopeInfoBuilder", () => {
   });
 
   describe("setRangeDefinitionScope", () => {
-    it("sets the definition scope when it's provided as a number", () => {
-      const info = builder.startScope(0, 0).endScope(10, 0).startRange(0, 0)
-        .setRangeDefinitionScope(0).endRange(0, 10).build();
+    it("sets the definition scope when it's provided directly", () => {
+      const scope = builder.startScope(0, 0).endScope(10, 0).lastScope()!;
+      const info = builder.startRange(0, 0).setRangeDefinitionScope(scope)
+        .endRange(0, 10).build();
 
       assertStrictEquals(info.scopes[0], info.ranges[0].originalScope);
+      assertStrictEquals(info.ranges[0].originalScope, scope);
     });
 
+    it("does nothing when no range is on the stack", () => {
+      const scope = builder.startScope(0, 0).endScope(10, 0).lastScope()!;
+      builder.setRangeDefinitionScope(scope);
+    });
+  });
+
+  describe("setRangeDefinitionScopeKey", () => {
     it("sets the definition scope when it's provided directly", () => {
-      const scope = builder.startScope(0, 0).endScope(10, 0).lastScope();
-      const info = builder.startRange(0, 0).setRangeDefinitionScope(scope!)
+      builder.startScope(0, 0, { key: "my key" }).endScope(10, 0);
+      const info = builder.startRange(0, 0).setRangeDefinitionScopeKey("my key")
         .endRange(0, 10).build();
 
-      assertStrictEquals(info.scopes[0], info.ranges[0].originalScope);
-      assertStrictEquals(info.ranges[0].originalScope, scope);
+      assertStrictEquals(info.ranges[0].originalScope, info.scopes[0]);
     });
 
     it("does nothing when no range is on the stack", () => {
-      builder.setRangeDefinitionScope(0);
+      builder.setRangeDefinitionScopeKey("foo");
     });
   });
 
@@ -297,4 +306,14 @@ describe("ScopeInfoBuilder", () => {
       assertEquals(builder.lastScope()?.start, { line: 10, column: 0 });
     });
   });
+
+  describe("scope key", () => {
+    it("can set the scope key via options", () => {
+      builder.startScope(0, 0, { key: "my custom key" }).endScope(10, 0);
+      builder.startRange(0, 0, { scopeKey: "my custom key" }).endRange(0, 10);
+      const info = builder.build();
+
+      assertStrictEquals(info.ranges[0].originalScope, info.scopes[0]);
+    });
+  });
 });

src/builder/builder.ts

@@ -21,9 +21,8 @@ export class ScopeInfoBuilder {
   #scopeStack: OriginalScope[] = [];
   #rangeStack: GeneratedRange[] = [];
 
-  #scopeCounter = 0;
-  #scopeToCount = new Map<OriginalScope, number>();
-  #countToScope = new Map<number, OriginalScope>();
+  #knownScopes = new Set<OriginalScope>();
+  #keyToScope = new Map<ScopeKey, OriginalScope>();
   #lastScope: OriginalScope | null = null;
 
   addNullScope(): this {
@@ -39,6 +38,7 @@ export class ScopeInfoBuilder {
       kind?: string;
       isStackFrame?: boolean;
       variables?: string[];
+      key?: ScopeKey;
     },
   ): this {
     const scope: OriginalScope = {
@@ -56,8 +56,8 @@ export class ScopeInfoBuilder {
       scope.parent = this.#scopeStack.at(-1);
     }
     this.#scopeStack.push(scope);
-    this.#scopeToCount.set(scope, this.#scopeCounter);
-    this.#countToScope.set(this.#scopeCounter++, scope);
+    this.#knownScopes.add(scope);
+    if (options?.key !== undefined) this.#keyToScope.set(options.key, scope);
 
     return this;
   }
@@ -119,14 +119,14 @@ export class ScopeInfoBuilder {
 
   /**
    * @param option The definition 'scope' of this range can either be the "OriginalScope" directly
-   * (produced by this builder) or the scope's number.
-   * If a scope was started with the n-th call to `startScope` then n is the scope's number.
+   * (produced by this builder) or the scope's key set while building the scope.
    */
   startRange(
     line: number,
     column: number,
     options?: {
-      scope?: number | OriginalScope;
+      scope?: OriginalScope;
+      scopeKey?: ScopeKey;
       isStackFrame?: boolean;
       isHidden?: boolean;
     },
@@ -144,27 +144,26 @@ export class ScopeInfoBuilder {
       range.parent = this.#rangeStack.at(-1);
     }
 
-    if (typeof options?.scope === "number") {
-      range.originalScope = this.#countToScope.get(options.scope);
-    } else if (options?.scope !== undefined) {
+    if (options?.scope !== undefined) {
       range.originalScope = options.scope;
+    } else if (options?.scopeKey !== undefined) {
+      range.originalScope = this.#keyToScope.get(options.scopeKey);
     }
 
     this.#rangeStack.push(range);
 
     return this;
   }
 
-  setRangeDefinitionScope(scope: number | OriginalScope): this {
+  setRangeDefinitionScope(scope: OriginalScope): this {
     const range = this.#rangeStack.at(-1);
-    if (!range) return this;
-
-    if (typeof scope === "number") {
-      range.originalScope = this.#countToScope.get(scope);
-    } else {
-      range.originalScope = scope;
-    }
+    if (range) range.originalScope = scope;
+    return this;
+  }
 
+  setRangeDefinitionScopeKey(scopeKey: ScopeKey): this {
+    const range = this.#rangeStack.at(-1);
+    if (range) range.originalScope = this.#keyToScope.get(scopeKey);
     return this;
   }
 
@@ -202,9 +201,7 @@ export class ScopeInfoBuilder {
 
     this.#scopes = [];
     this.#ranges = [];
-    this.#scopeCounter = 0;
-    this.#scopeToCount.clear();
-    this.#countToScope.clear();
+    this.#knownScopes.clear();
 
     return info;
   }
@@ -217,11 +214,19 @@ export class ScopeInfoBuilder {
     return this.#rangeStack;
   }
 
-  protected isValidScopeNumber(n: number): boolean {
-    return this.#countToScope.has(n);
+  protected isKnownScope(scope: OriginalScope): boolean {
+    return this.#knownScopes.has(scope);
   }
 
-  protected isKnownScope(scope: OriginalScope): boolean {
-    return this.#scopeToCount.has(scope);
+  protected isValidScopeKey(key: ScopeKey): boolean {
+    return this.#keyToScope.has(key);
   }
 }
+
+/**
+ * Users of the {@link ScopeInfoBuilder} can provide their own keys to uniquely identify a scope,
+ * and use the key later when building the corresponding range to connect them.
+ *
+ * The only requirement for ScopeKey is that it can be used as a key in a `Map`.
+ */
+export type ScopeKey = unknown;

src/builder/safe_builder.test.ts

@@ -154,8 +154,8 @@ describe("SafeScopeInfoBuilder", () => {
       builder.startRange(10, 5);
     });
 
-    it("throws when the definition scope doesnt point to a valid scope", () => {
-      assertThrows(() => builder.startRange(0, 0, { scope: 0 }));
+    it("throws when the definition scope doesn't point to a valid scope", () => {
+      assertThrows(() => builder.startRange(0, 0, { scopeKey: 0 }));
     });
 
     it("throws when the definition scope is not known to the builder", () => {
@@ -175,17 +175,39 @@ describe("SafeScopeInfoBuilder", () => {
 
   describe("setRangeDefinitionScope", () => {
     it("throws when no range is open", () => {
-      assertThrows(() => builder.setRangeDefinitionScope(0));
+      const scope = builder.startScope(0, 0).endScope(10, 0).lastScope()!;
+
+      assertThrows(() => builder.setRangeDefinitionScope(scope));
     });
 
     it("throws while building a scope", () => {
-      builder.startScope(0, 0);
+      const scope = builder.startScope(0, 0).currentScope()!;
+
+      assertThrows(() => builder.setRangeDefinitionScope(scope));
+    });
+
+    it("throws when the definition scope is not known to the builder", () => {
+      assertThrows(() =>
+        builder.startRange(0, 0).setRangeDefinitionScope({
+          start: { line: 0, column: 0 },
+          end: { line: 10, column: 10 },
+          isStackFrame: false,
+          variables: [],
+          children: [],
+        })
+      );
+    });
+  });
 
-      assertThrows(() => builder.setRangeDefinitionScope(0));
+  describe("setRangeDefinitionScopeKey", () => {
+    it("throws when no range is open", () => {
+      assertThrows(() => builder.setRangeDefinitionScopeKey("my key"));
     });
 
-    it("throws when the definition scope doesnt point to a valid scope", () => {
-      assertThrows(() => builder.startRange(0, 0).setRangeDefinitionScope(0));
+    it("throws while building a scope", () => {
+      builder.startScope(0, 0, { key: "my key" });
+
+      assertThrows(() => builder.setRangeDefinitionScopeKey("my key"));
     });
 
     it("throws when the definition scope is not known to the builder", () => {

src/builder/safe_builder.ts

@@ -4,7 +4,7 @@
 
 import type { OriginalScope, ScopeInfo } from "../scopes.d.ts";
 import { comparePositions } from "../util.ts";
-import { ScopeInfoBuilder } from "./builder.ts";
+import { ScopeInfoBuilder, type ScopeKey } from "./builder.ts";
 
 /**
  * Similar to `ScopeInfoBuilder`, but with checks that scopes/ranges are well
@@ -27,6 +27,7 @@ export class SafeScopeInfoBuilder extends ScopeInfoBuilder {
       kind?: string;
       isStackFrame?: boolean;
       variables?: string[];
+      key?: ScopeKey;
     },
   ): this {
     this.#verifyEmptyRangeStack("start scope");
@@ -109,7 +110,8 @@ export class SafeScopeInfoBuilder extends ScopeInfoBuilder {
     line: number,
     column: number,
     options?: {
-      scope?: number | OriginalScope;
+      scope?: OriginalScope;
+      scopeKey?: ScopeKey;
       isStackFrame?: boolean;
       isHidden?: boolean;
     },
@@ -136,16 +138,14 @@ export class SafeScopeInfoBuilder extends ScopeInfoBuilder {
     }
 
     if (
-      typeof options?.scope === "number" &&
-      !this.isValidScopeNumber(options.scope)
+      options?.scopeKey !== undefined &&
+      !this.isValidScopeKey(options.scopeKey)
     ) {
       throw new Error(
-        `${options.scope} does not reference a valid OriginalScope`,
+        `${options.scopeKey} does not reference a valid OriginalScope`,
       );
     }
-    if (
-      typeof options?.scope === "object" && !this.isKnownScope(options.scope)
-    ) {
+    if (options?.scope && !this.isKnownScope(options.scope)) {
       throw new Error(
         "The provided definition scope was not produced by this builder!",
       );
@@ -155,27 +155,31 @@ export class SafeScopeInfoBuilder extends ScopeInfoBuilder {
     return this;
   }
 
-  override setRangeDefinitionScope(scope: number | OriginalScope): this {
+  override setRangeDefinitionScope(scope: OriginalScope): this {
     this.#verifyEmptyScopeStack("setRangeDefinitionScope");
     this.#verifyRangePresent("setRangeDefinitionScope");
 
-    if (
-      typeof scope === "number" &&
-      !this.isValidScopeNumber(scope)
-    ) {
+    if (!this.isKnownScope(scope)) {
       throw new Error(
-        `${scope} does not reference a valid OriginalScope`,
+        "The provided definition scope was not produced by this builder!",
       );
     }
-    if (
-      typeof scope === "object" && !this.isKnownScope(scope)
-    ) {
+
+    super.setRangeDefinitionScope(scope);
+    return this;
+  }
+
+  override setRangeDefinitionScopeKey(scopeKey: ScopeKey): this {
+    this.#verifyEmptyScopeStack("setRangeDefinitionScope");
+    this.#verifyRangePresent("setRangeDefinitionScope");
+
+    if (!this.isValidScopeKey(scopeKey)) {
       throw new Error(
-        "The provided definition scope was not produced by this builder!",
+        `The provided scope key ${scopeKey} is not know nto the builder!`,
       );
     }
 
-    super.setRangeDefinitionScope(scope);
+    super.setRangeDefinitionScopeKey(scopeKey);
     return this;
   }
 

src/roundtrip.test.ts

@@ -103,18 +103,22 @@ describe("round trip", () => {
   });
 
   it("handles a single GeneratedRange with a definition scope", () => {
-    builder.startScope(0, 0).endScope(10, 0).startRange(0, 0, { scope: 0 })
+    builder.startScope(0, 0, { key: 0 }).endScope(10, 0).startRange(0, 0, {
+      scopeKey: 0,
+    })
       .endRange(0, 10);
 
     assertCodec(builder.build());
   });
 
   it("handles multiple GeneratedRanges with different definition scopes", () => {
-    builder.startScope(0, 0).endScope(10, 0).startScope(0, 0).endScope(20, 0)
+    builder.startScope(0, 0, { key: 0 }).endScope(10, 0).startScope(0, 0, {
+      key: 1,
+    }).endScope(20, 0)
       .startRange(0, 0)
-      .startRange(0, 10, { scope: 0 })
+      .startRange(0, 10, { scopeKey: 0 })
       .endRange(0, 40)
-      .startRange(0, 50, { scope: 1 })
+      .startRange(0, 50, { scopeKey: 1 })
       .endRange(0, 80)
       .endRange(0, 100);