feedback

Author: tomas-zijdemans

data_structures/unstable_indexed_heap.ts

@@ -55,6 +55,20 @@ function checkPriority(priority: unknown): void {
   }
 }
 
+/**
+ * Conditional rest-parameter tuple for the comparator options bag. When
+ * the priority `P` is one of the primitive types that {@linkcode ascend}
+ * orders correctly (`number`, `bigint`, `string`, plus their literal and
+ * union subtypes), the options bag — and therefore `compare` itself —
+ * is optional. For any other `P` (object, tuple, custom class), the
+ * options bag is required and must provide `compare`; without it,
+ * `ascend` would silently produce nonsense ordering (e.g., lexicographic
+ * string comparison of `[number, number]` tuples).
+ */
+type IndexedHeapCompareArgs<P> = [P] extends [number | bigint | string]
+  ? [options?: { compare?: (a: P, b: P) => number }]
+  : [options: { compare: (a: P, b: P) => number }];
+
 /**
  * A priority queue that supports looking up, removing, and re-prioritizing
  * entries by key. Each entry is a unique `(key, priority)` pair. The entry
@@ -102,7 +116,10 @@ function checkPriority(priority: unknown): void {
  * priority's contents (e.g., `heap.peek().priority[0] = 0`) corrupts
  * the heap invariant. Use
  * {@linkcode IndexedHeap.prototype.set | set} to change a key's
- * priority.
+ * priority. `set` itself uses reference equality (`===`) to short-circuit
+ * when the priority is unchanged, so passing a structurally-equal but
+ * distinct object/tuple priority replaces the reference and runs a sift
+ * (no observable reorder, but not a no-op).
  *
  * @experimental **UNSTABLE**: New API, yet to be vetted.
  *
@@ -183,9 +200,12 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * @param entries An optional iterable of `[key, priority]` pairs. Each key
    * must be unique; duplicates throw a `TypeError`. Pass `null` or
    * `undefined` to create an empty heap with options.
-   * @param options Optional configuration. `compare` overrides the default
-   * ascending-numeric ordering; pass {@linkcode descend} for max-heap
-   * order, or a custom function for tuple/`bigint`/etc. priorities.
+   * @param options Optional configuration when `P` is `number`, `bigint`,
+   * or `string`; required when `P` is any other type (e.g. tuples or
+   * objects) so the heap has a valid comparator. `compare` overrides the
+   * default ascending-numeric ordering; pass {@linkcode descend} for
+   * max-heap order, or a custom function for tuple/`bigint`/etc.
+   * priorities.
    *
    * @example Empty heap
    * ```ts
@@ -227,6 +247,10 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * assertEquals(heap.peek(), { key: "b", priority: 5 });
    * ```
    */
+  constructor(
+    entries?: Iterable<readonly [K, P]> | null,
+    ...options: IndexedHeapCompareArgs<P>
+  );
   constructor(
     entries?: Iterable<readonly [K, P]> | null,
     options?: { compare?: (a: P, b: P) => number },
@@ -240,7 +264,7 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
     this.#compare = compare;
     if (entries === undefined || entries === null) return;
     if (
-      typeof entries !== "object" && typeof entries !== "string" ||
+      typeof entries !== "object" ||
       !(Symbol.iterator in Object(entries))
     ) {
       throw new TypeError(
@@ -267,22 +291,14 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
   readonly [Symbol.toStringTag] = "IndexedHeap" as const;
 
   /**
-   * Create a new {@linkcode IndexedHeap} from an iterable of key-priority
-   * pairs, an array-like of key-priority pairs, or an existing
-   * {@linkcode IndexedHeap}. When copying from another `IndexedHeap`, the
-   * source's comparator is inherited unless `options.compare` overrides it.
+   * Create a shallow copy of an existing {@linkcode IndexedHeap}. The
+   * source's comparator is inherited unless `options.compare` overrides
+   * it; when overridden, the copy is re-heapified in O(n).
    *
-   * @experimental **UNSTABLE**: New API, yet to be vetted.
+   * Because the source already carries a valid comparator, `compare`
+   * stays optional regardless of the priority type `P`.
    *
-   * @example Creating from an array of pairs
-   * ```ts
-   * import { IndexedHeap } from "@std/data-structures/unstable-indexed-heap";
-   * import { assertEquals } from "@std/assert";
-   *
-   * const heap = IndexedHeap.from([["a", 3], ["b", 1], ["c", 2]]);
-   * assertEquals(heap.peek(), { key: "b", priority: 1 });
-   * assertEquals(heap.size, 3);
-   * ```
+   * @experimental **UNSTABLE**: New API, yet to be vetted.
    *
    * @example Creating from another IndexedHeap (shallow copy)
    * ```ts
@@ -299,6 +315,56 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * assertEquals(original.size, 2);
    * ```
    *
+   * @example Re-ordering an existing heap with a different comparator
+   * ```ts
+   * import { IndexedHeap } from "@std/data-structures/unstable-indexed-heap";
+   * import { descend } from "@std/data-structures";
+   * import { assertEquals } from "@std/assert";
+   *
+   * const minHeap = new IndexedHeap<string>([["a", 1], ["b", 5], ["c", 3]]);
+   * const maxHeap = IndexedHeap.from(minHeap, { compare: descend });
+   *
+   * assertEquals(minHeap.peek(), { key: "a", priority: 1 });
+   * assertEquals(maxHeap.peek(), { key: "b", priority: 5 });
+   * ```
+   *
+   * @typeParam K The type of the keys in the heap.
+   * @typeParam P The type of the priority. Defaults to `number`.
+   * @param collection The source {@linkcode IndexedHeap} to copy.
+   * @param options Optional configuration. Omitting it inherits the
+   *   source's comparator; passing `compare` overrides it and triggers
+   *   a re-heapify.
+   * @returns A new heap containing all entries from the source heap.
+   */
+  static from<K, P = number>(
+    collection: IndexedHeap<K, P>,
+    options?: { compare?: (a: P, b: P) => number },
+  ): IndexedHeap<K, P>;
+  /**
+   * Create a new {@linkcode IndexedHeap} from an iterable or array-like
+   * of `[key, priority]` pairs.
+   *
+   * When the priority type `P` is `number`, `bigint`, or `string`, the
+   * default {@linkcode ascend} comparator orders correctly and the
+   * `options` bag is optional. For any other `P` (tuples, objects), the
+   * `compare` option is required at the type level so the heap has a
+   * valid comparator — otherwise `ascend` would silently mis-order.
+   *
+   * Use the {@linkcode IndexedHeap.from} overload that takes an existing
+   * {@linkcode IndexedHeap} to copy from another heap instead.
+   *
+   * @experimental **UNSTABLE**: New API, yet to be vetted.
+   *
+   * @example Creating from an array of pairs
+   * ```ts
+   * import { IndexedHeap } from "@std/data-structures/unstable-indexed-heap";
+   * import { assertEquals } from "@std/assert";
+   *
+   * const heap = IndexedHeap.from([["a", 3], ["b", 1], ["c", 2]]);
+   * assertEquals(heap.peek(), { key: "b", priority: 1 });
+   * assertEquals(heap.size, 3);
+   * ```
+   *
    * @example Creating from a Map (iterable of [key, value] pairs)
    * ```ts
    * import { IndexedHeap } from "@std/data-structures/unstable-indexed-heap";
@@ -309,28 +375,30 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * assertEquals(heap.peek(), { key: "task-b", priority: 1 });
    * ```
    *
-   * @example Re-ordering an existing heap with a different comparator
+   * @example Tuple priority requires an explicit comparator
    * ```ts
    * import { IndexedHeap } from "@std/data-structures/unstable-indexed-heap";
-   * import { descend } from "@std/data-structures";
    * import { assertEquals } from "@std/assert";
    *
-   * const minHeap = new IndexedHeap<string>([["a", 1], ["b", 5], ["c", 3]]);
-   * const maxHeap = IndexedHeap.from(minHeap, { compare: descend });
-   *
-   * assertEquals(minHeap.peek(), { key: "a", priority: 1 });
-   * assertEquals(maxHeap.peek(), { key: "b", priority: 5 });
+   * const heap = IndexedHeap.from<string, [number, number]>(
+   *   [["a", [5, 0]], ["b", [3, 1]]],
+   *   { compare: (a, b) => a[0] - b[0] || a[1] - b[1] },
+   * );
+   * assertEquals(heap.peek(), { key: "b", priority: [3, 1] });
    * ```
    *
    * @typeParam K The type of the keys in the heap.
    * @typeParam P The type of the priority. Defaults to `number`.
-   * @param collection An iterable or array-like of `[key, priority]` pairs,
-   *   or an existing {@linkcode IndexedHeap} to copy.
-   * @param options Optional configuration. `compare` overrides the
-   *   ordering; when copying from another `IndexedHeap`, omitting this
-   *   inherits the source's comparator.
+   * @param collection An iterable or array-like of `[key, priority]` pairs.
+   * @param options Optional configuration when `P` is `number`, `bigint`,
+   *   or `string`; required when `P` is any other type. `compare`
+   *   overrides the default ascending-numeric ordering.
    * @returns A new heap containing all entries from the collection.
    */
+  static from<K, P = number>(
+    collection: Iterable<readonly [K, P]> | ArrayLike<readonly [K, P]>,
+    ...options: IndexedHeapCompareArgs<P>
+  ): IndexedHeap<K, P>;
   static from<K, P = number>(
     collection:
       | IndexedHeap<K, P>
@@ -340,7 +408,7 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
   ): IndexedHeap<K, P> {
     if (
       collection === null || collection === undefined ||
-      typeof collection !== "object" && typeof collection !== "string" ||
+      typeof collection !== "object" ||
       !(
         Symbol.iterator in Object(collection) ||
         "length" in Object(collection)
@@ -351,7 +419,7 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
       );
     }
     if (collection instanceof IndexedHeap) {
-      const heap = new IndexedHeap<K, P>(null, {
+      const heap = IndexedHeap.#createInternal<K, P>({
         compare: options?.compare ?? collection.#compare,
       });
       for (const entry of collection.#data) {
@@ -368,7 +436,7 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
       }
       return heap;
     }
-    const heap = new IndexedHeap<K, P>(null, options);
+    const heap = IndexedHeap.#createInternal<K, P>(options);
     heap.#bulkLoad(
       Symbol.iterator in Object(collection)
         ? collection as Iterable<readonly [K, P]>
@@ -377,6 +445,22 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
     return heap;
   }
 
+  /**
+   * Allocate a new {@linkcode IndexedHeap} from inside the class, bypassing
+   * the public constructor's conditional rest-tuple constraint. Used by
+   * {@linkcode IndexedHeap.from} where `P` is still generic and the
+   * conditional `IndexedHeapCompareArgs<P>` cannot be resolved.
+   */
+  static #createInternal<K, P>(
+    options?: { compare?: (a: P, b: P) => number },
+  ): IndexedHeap<K, P> {
+    type RelaxedCtor = new <K2, P2>(
+      entries?: Iterable<readonly [K2, P2]> | null,
+      options?: { compare?: (a: P2, b: P2) => number },
+    ) => IndexedHeap<K2, P2>;
+    return new (IndexedHeap as RelaxedCtor)<K, P>(null, options);
+  }
+
   /**
    * Append all `[key, priority]` pairs and heapify in linear time. Used by
    * the constructor and by {@linkcode IndexedHeap.from} for the
@@ -533,11 +617,9 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * Return the front entry (smallest priority) without removing it, or
    * `undefined` if the heap is empty.
    *
-   * The returned wrapper is a fresh object — replacing its `key` or
-   * `priority` property has no effect on the heap. Object-typed
-   * priorities (e.g., tuples) share their reference with the heap and
-   * must not be mutated; see the class-level note on priority
-   * mutability.
+   * The returned entry is a fresh wrapper, but for object-typed priorities
+   * the `priority` field shares its reference with the heap — see the
+   * class-level note on treating object priorities as immutable.
    *
    * @experimental **UNSTABLE**: New API, yet to be vetted.
    *
@@ -668,6 +750,13 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * throws on existing keys) and is the natural operation for relaxation
    * steps in graph algorithms like Dijkstra's.
    *
+   * When `key` already exists, the no-op fast path uses reference equality
+   * (`===`) to skip work when the new priority is identical to the old.
+   * For object/tuple priorities this means a structurally-equal but
+   * distinct reference still replaces the stored reference and runs a
+   * sift (no observable reorder, but not free). See the class-level note
+   * on treating object priorities as immutable.
+   *
    * @experimental **UNSTABLE**: New API, yet to be vetted.
    *
    * @example Inserting and updating
@@ -880,6 +969,10 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * Use {@linkcode IndexedHeap.prototype.drain | drain} to retrieve entries
    * in sorted (smallest-first) order.
    *
+   * Each returned entry is a fresh wrapper, but for object-typed priorities
+   * the `priority` field shares its reference with the heap — see the
+   * class-level note on treating object priorities as immutable.
+   *
    * @experimental **UNSTABLE**: New API, yet to be vetted.
    *
    * @example Usage
@@ -914,6 +1007,10 @@ export class IndexedHeap<K, P = number> implements Iterable<HeapEntry<K, P>> {
    * Mutating the heap (`push`, `pop`, `set`, `delete`, `clear`) while
    * iterating is not supported and may skip or repeat entries.
    *
+   * Each yielded entry is a fresh wrapper, but for object-typed priorities
+   * the `priority` field shares its reference with the heap — see the
+   * class-level note on treating object priorities as immutable.
+   *
    * @experimental **UNSTABLE**: New API, yet to be vetted.
    *
    * @example Usage

data_structures/unstable_indexed_heap_test.ts

@@ -727,14 +727,31 @@ Deno.test("IndexedHeap.from() throws TypeError on non-iterable, non-array-like i
   assertThrows(() => IndexedHeap.from({} as any), TypeError, message);
 });
 
-Deno.test("IndexedHeap.from() accepts strings (iterable of characters)", () => {
-  // A string is iterable, so it passes the `from()` guard. Each destructured
-  // character yields `key = char`, `priority = undefined`, resulting in a
-  // heap with two undefined-priority entries. Not useful in practice, but
-  // documents that strings bypass the type guard.
-  // deno-lint-ignore no-explicit-any
-  const heap = IndexedHeap.from("ab" as any);
-  assertEquals(heap.size, 2);
+Deno.test("IndexedHeap constructor rejects a string with a helpful error", () => {
+  // Strings are technically iterable but never as `Iterable<[K, P]>`.
+  // Route them through the same "did you mean `from`?" guard as any other
+  // non-iterable-of-pairs input, instead of letting them silently fail
+  // inside the destructuring loop with a confusing error.
+  assertThrows(
+    // deno-lint-ignore no-explicit-any
+    () => new IndexedHeap("ab" as any),
+    TypeError,
+    "the 'entries' parameter is not iterable, did you mean to call IndexedHeap.from?",
+  );
+});
+
+Deno.test("IndexedHeap.from() rejects a string with a helpful error", () => {
+  // A string is both iterable (yielding characters) and array-like (has
+  // `length`), so it would otherwise bypass the type guard and silently
+  // build a heap of undefined-priority entries from destructured chars.
+  // Reject it explicitly so the caller learns the type was wrong, instead
+  // of debugging a heap full of `priority: undefined`.
+  assertThrows(
+    // deno-lint-ignore no-explicit-any
+    () => IndexedHeap.from("ab" as any),
+    TypeError,
+    "the 'collection' parameter is not iterable or array-like",
+  );
 });
 
 Deno.test("IndexedHeap toArray() returns all entries without modifying heap", () => {
@@ -951,6 +968,57 @@ Deno.test("IndexedHeap throws TypeError when compare option is not a function",
   );
 });
 
+Deno.test("IndexedHeap requires compare at the type level when priority is not a primitive", () => {
+  // Primitive priorities (number, bigint, string) accept ascend by default,
+  // so the options bag — and therefore `compare` — stays optional.
+  const _numHeap = new IndexedHeap<string>();
+  const _bigintHeap = new IndexedHeap<string, bigint>();
+  const _strHeap = new IndexedHeap<string, string>();
+
+  // Non-primitive priorities (tuples, objects) would silently lexicographically
+  // mis-order under the default `ascend`, so `compare` is required at the
+  // type level. The casts below pin the @ts-expect-error to the missing
+  // `compare`, not to anything else; if the typed overload regresses the
+  // compiler will stop flagging these lines and the test will fail to
+  // typecheck (because @ts-expect-error itself errors when nothing errors).
+
+  // @ts-expect-error - 'compare' is required when P is a tuple
+  const _tupleHeap = new IndexedHeap<string, [number, number]>();
+  // @ts-expect-error - 'compare' is required when P is an object
+  const _objHeap = new IndexedHeap<string, { score: number }>();
+  // @ts-expect-error - 'compare' is required when P is a tuple
+  const _tupleFromIter = IndexedHeap.from<string, [number, number]>([]);
+
+  // Supplying `compare` satisfies the requirement.
+  const _tupleHeapOk = new IndexedHeap<string, [number, number]>(null, {
+    compare: (a, b) => a[0] - b[0] || a[1] - b[1],
+  });
+  const _objHeapOk = new IndexedHeap<string, { score: number }>([], {
+    compare: (a, b) => a.score - b.score,
+  });
+
+  // Copying from an existing IndexedHeap inherits the source's comparator,
+  // so `compare` stays optional even when P is non-primitive.
+  const _tupleCopy = IndexedHeap.from(_tupleHeapOk);
+
+  // Reference all locals so this stays a runtime-passing test even if the
+  // type-only assertions above are ever removed.
+  assertEquals(
+    [
+      _numHeap.size,
+      _bigintHeap.size,
+      _strHeap.size,
+      _tupleHeap.size,
+      _objHeap.size,
+      _tupleFromIter.size,
+      _tupleHeapOk.size,
+      _objHeapOk.size,
+      _tupleCopy.size,
+    ],
+    [0, 0, 0, 0, 0, 0, 0, 0, 0],
+  );
+});
+
 Deno.test("IndexedHeap peek wrapper is fresh; replacing priority is safe even with object priorities", () => {
   // Replacing the priority property on the returned wrapper is a no-op on
   // the heap because the wrapper is a fresh object. Mutating the priority