Merge pull request #2831 from hey-api/refactor/graph-declarations

refactor: add walk function instead of hard-coded logic

Author: mrlubos

packages/openapi-ts-tests/main/test/openapi-ts.config.ts

@@ -40,11 +40,11 @@ export default defineConfig(() => {
             // 'dutchie.json',
             // 'invalid',
             // 'openai.yaml',
-            // 'full.yaml',
+            'full.yaml',
             // 'opencode.yaml',
             // 'sdk-instance.yaml',
             // 'string-with-format.yaml',
-            'transformers.json',
+            // 'transformers.json',
             // 'type-format.yaml',
             // 'validators.yaml',
             // 'validators-circular-ref.json',

packages/openapi-ts/src/debug/graph.ts

@@ -1,4 +1,4 @@
-import type { Graph } from '~/openApi/shared/utils/graph';
+import type { Graph } from '~/graph';
 
 const analyzeStructure = (graph: Graph) => {
   let maxDepth = 0;

packages/openapi-ts/src/graph/__tests__/walk.test.ts

@@ -0,0 +1,306 @@
+import { describe, expect, it } from 'vitest';
+
+import type { Graph } from '~/graph';
+import {
+  getIrPointerPriority,
+  matchIrPointerToGroup,
+  preferGroups,
+} from '~/ir/graph';
+import { buildGraph } from '~/openApi/shared/utils/graph';
+
+import { walk } from '../walk';
+
+const loggerStub = {
+  timeEvent: () => ({ timeEnd: () => {} }),
+} as any;
+
+describe('walkTopological', () => {
+  const makeGraph = (
+    deps: Array<[string, Array<string>]>,
+    nodes: Array<string>,
+  ) => {
+    const nodeDependencies = new Map<string, Set<string>>();
+    const subtreeDependencies = new Map<string, Set<string>>();
+    const reverseNodeDependencies = new Map<string, Set<string>>();
+    const nodesMap = new Map<string, any>();
+
+    for (const name of nodes) {
+      nodesMap.set(name, { key: null, node: {}, parentPointer: null });
+    }
+
+    for (const [from, toList] of deps) {
+      const s = new Set<string>(toList);
+      nodeDependencies.set(from, s);
+      subtreeDependencies.set(from, new Set<string>(toList));
+      for (const to of toList) {
+        if (!reverseNodeDependencies.has(to))
+          reverseNodeDependencies.set(to, new Set());
+        reverseNodeDependencies.get(to)!.add(from);
+      }
+    }
+
+    return {
+      nodeDependencies,
+      nodes: nodesMap,
+      reverseNodeDependencies,
+      subtreeDependencies,
+      transitiveDependencies: new Map<string, Set<string>>(),
+    } as unknown as Graph;
+  };
+
+  it('walks nodes in topological order for a simple acyclic graph', () => {
+    // Graph: A -> B -> C
+    const graph = makeGraph(
+      [
+        ['A', ['B']],
+        ['B', ['C']],
+      ],
+      ['A', 'B', 'C'],
+    );
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order.indexOf('C')).toBeLessThan(order.indexOf('B'));
+    expect(order.indexOf('B')).toBeLessThan(order.indexOf('A'));
+    expect(order).toHaveLength(3);
+  });
+
+  it('walks nodes in topological order for multiple roots', () => {
+    // Graph: A -> B, C -> D
+    const graph = makeGraph(
+      [
+        ['A', ['B']],
+        ['C', ['D']],
+      ],
+      ['A', 'B', 'C', 'D'],
+    );
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order.indexOf('B')).toBeLessThan(order.indexOf('A'));
+    expect(order.indexOf('D')).toBeLessThan(order.indexOf('C'));
+    expect(order).toHaveLength(4);
+  });
+
+  it('walks nodes in topological order for a disconnected graph', () => {
+    // Graph: A -> B, C (no deps), D (no deps)
+    const graph = makeGraph([['A', ['B']]], ['A', 'B', 'C', 'D']);
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order.indexOf('B')).toBeLessThan(order.indexOf('A'));
+    expect(order).toHaveLength(4);
+    expect(order).toContain('C');
+    expect(order).toContain('D');
+  });
+
+  it('walks nodes in topological order for a diamond dependency', () => {
+    // Graph:   A
+    //         / \
+    //        B   C
+    //         \ /
+    //          D
+    const graph = makeGraph(
+      [
+        ['A', ['B', 'C']],
+        ['B', ['D']],
+        ['C', ['D']],
+      ],
+      ['A', 'B', 'C', 'D'],
+    );
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order.indexOf('D')).toBeLessThan(order.indexOf('B'));
+    expect(order.indexOf('D')).toBeLessThan(order.indexOf('C'));
+    expect(order.indexOf('B')).toBeLessThan(order.indexOf('A'));
+    expect(order.indexOf('C')).toBeLessThan(order.indexOf('A'));
+    expect(order).toHaveLength(4);
+  });
+
+  it('walks nodes in topological order for a long chain', () => {
+    // Graph: A -> B -> C -> D -> E
+    const graph = makeGraph(
+      [
+        ['A', ['B']],
+        ['B', ['C']],
+        ['C', ['D']],
+        ['D', ['E']],
+      ],
+      ['A', 'B', 'C', 'D', 'E'],
+    );
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order.indexOf('E')).toBeLessThan(order.indexOf('D'));
+    expect(order.indexOf('D')).toBeLessThan(order.indexOf('C'));
+    expect(order.indexOf('C')).toBeLessThan(order.indexOf('B'));
+    expect(order.indexOf('B')).toBeLessThan(order.indexOf('A'));
+    expect(order).toHaveLength(5);
+  });
+
+  it('walks all nodes, including cycles', () => {
+    // Graph: A <-> B (cycle), C (no deps)
+    const graph = makeGraph(
+      [
+        ['A', ['B']],
+        ['B', ['A']],
+      ],
+      ['A', 'B', 'C'],
+    );
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order.sort()).toEqual(['A', 'B', 'C']);
+  });
+
+  it('matches ordering for validators-circular-ref spec', async () => {
+    const specModule = await import(
+      '../../../../../specs/3.1.x/validators-circular-ref.json'
+    );
+    const spec = specModule.default ?? specModule;
+    const { graph } = buildGraph(spec, loggerStub);
+
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+
+    const foo = '#/components/schemas/Foo';
+    const bar = '#/components/schemas/Bar';
+    const baz = '#/components/schemas/Baz';
+    const qux = '#/components/schemas/Qux';
+
+    // Bar should come before Foo because Foo depends on Bar
+    expect(order.indexOf(bar)).toBeLessThan(order.indexOf(foo));
+
+    // Baz and Qux form a mutual $ref cycle; both must be present
+    expect(order).toContain(baz);
+    expect(order).toContain(qux);
+  });
+
+  it('prefers schema group before parameter when safe (default)', () => {
+    // parameter then schema in declaration order, no deps -> schema should move before parameter
+    const param = '#/components/parameters/P';
+    const schema = '#/components/schemas/A';
+    const nodes = [param, schema];
+    const graph = makeGraph([], nodes);
+
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), {
+      getPointerPriority: getIrPointerPriority,
+      matchPointerToGroup: matchIrPointerToGroup,
+      order: 'topological',
+      preferGroups,
+    });
+    expect(order.indexOf(schema)).toBeLessThan(order.indexOf(param));
+  });
+
+  it('does not apply preferGroups when it would violate dependencies (fallback)', () => {
+    // declaration order: param, schema; schema depends on param -> cannot move before param
+    const param = '#/components/parameters/P';
+    const schema = '#/components/schemas/S';
+    const nodes = [param, schema];
+    const nodeDependencies = new Map<string, Set<string>>();
+    nodeDependencies.set(schema, new Set([param]));
+    const subtreeDependencies = new Map<string, Set<string>>();
+    const reverseNodeDependencies = new Map<string, Set<string>>();
+    const nodesMap = new Map<string, any>();
+    for (const n of nodes)
+      nodesMap.set(n, { key: null, node: {}, parentPointer: null });
+    const graph = {
+      nodeDependencies,
+      nodes: nodesMap,
+      reverseNodeDependencies,
+      subtreeDependencies,
+      transitiveDependencies: new Map<string, Set<string>>(),
+    } as unknown as Graph;
+
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    // schema depends on param so param must remain before schema
+    expect(order.indexOf(param)).toBeLessThan(order.indexOf(schema));
+  });
+
+  it('ignores self-dependencies when ordering', () => {
+    // Foo has self-ref only, Bar references Foo -> Foo should come before Bar
+    const foo = '#/components/schemas/Foo';
+    const bar = '#/components/schemas/Bar';
+    const nodes = [foo, bar];
+    const nodeDependencies = new Map<string, Set<string>>();
+    nodeDependencies.set(foo, new Set([foo]));
+    nodeDependencies.set(bar, new Set([foo]));
+
+    const nodesMap = new Map<string, any>();
+    for (const n of nodes)
+      nodesMap.set(n, { key: null, node: {}, parentPointer: null });
+
+    const graph = {
+      nodeDependencies,
+      nodes: nodesMap,
+      reverseNodeDependencies: new Map<string, Set<string>>(),
+      subtreeDependencies: new Map<string, Set<string>>(),
+      transitiveDependencies: new Map<string, Set<string>>(),
+    } as unknown as Graph;
+
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    // Foo is a dependency of Bar, so Foo should come before Bar
+    expect(order.indexOf(foo)).toBeLessThan(order.indexOf(bar));
+  });
+
+  it('uses subtreeDependencies when nodeDependencies are absent', () => {
+    const parent = '#/components/schemas/Parent';
+    const child = '#/components/schemas/Child';
+    const nodes = [parent, child];
+    const nodeDependencies = new Map<string, Set<string>>();
+    const subtreeDependencies = new Map<string, Set<string>>();
+    subtreeDependencies.set(parent, new Set([child]));
+
+    const nodesMap = new Map<string, any>();
+    for (const n of nodes)
+      nodesMap.set(n, { key: null, node: {}, parentPointer: null });
+
+    const graph = {
+      nodeDependencies,
+      nodes: nodesMap,
+      reverseNodeDependencies: new Map<string, Set<string>>(),
+      subtreeDependencies,
+      transitiveDependencies: new Map<string, Set<string>>(),
+    } as unknown as Graph;
+
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order.indexOf(child)).toBeLessThan(order.indexOf(parent));
+  });
+
+  it('preserves declaration order for equal-priority items (stability)', () => {
+    const a = '#/components/schemas/A';
+    const b = '#/components/schemas/B';
+    const c = '#/components/schemas/C';
+    const nodes = [a, b, c];
+    const graph = makeGraph([], nodes);
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'topological' });
+    expect(order).toEqual(nodes);
+  });
+
+  it('walks nodes in declaration order when order=declarations', () => {
+    const a = '#/components/schemas/A';
+    const b = '#/components/schemas/B';
+    const c = '#/components/schemas/C';
+    const nodes = [a, b, c];
+    const graph = makeGraph([], nodes);
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), { order: 'declarations' });
+    expect(order).toEqual(nodes);
+  });
+
+  it('applies preferGroups ordering in declaration mode', () => {
+    const param = '#/components/parameters/P';
+    const schema = '#/components/schemas/A';
+    const nodes = [param, schema];
+    const graph = makeGraph([], nodes);
+
+    const order: Array<string> = [];
+    walk(graph, (pointer) => order.push(pointer), {
+      matchPointerToGroup: matchIrPointerToGroup,
+      order: 'declarations',
+      preferGroups,
+    });
+    // preferGroups puts schema before parameter
+    expect(order.indexOf(schema)).toBeLessThan(order.indexOf(param));
+  });
+});

packages/openapi-ts/src/graph/index.ts

@@ -0,0 +1,7 @@
+export type { Graph, NodeInfo } from './types/graph';
+export type {
+  GetPointerPriorityFn,
+  PointerGroupMatch,
+  WalkOptions,
+} from './types/walk';
+export { walk } from './walk';

packages/openapi-ts/src/graph/types/graph.d.ts

@@ -0,0 +1,60 @@
+/**
+ * The main graph structure for OpenAPI node analysis.
+ *
+ * @property nodeDependencies - For each node with at least one dependency, the set of normalized JSON Pointers it references via $ref. Nodes with no dependencies are omitted.
+ * @property nodes - Map from normalized JSON Pointer to NodeInfo for every node in the spec.
+ * @property reverseNodeDependencies - For each node with at least one dependent, the set of nodes that reference it via $ref. Nodes with no dependents are omitted.
+ */
+export type Graph = {
+  /**
+   * For each node with at least one dependency, the set of normalized JSON Pointers it references via $ref.
+   * Nodes with no dependencies are omitted from this map.
+   */
+  nodeDependencies: Map<string, Set<string>>;
+  /**
+   * Map from normalized JSON Pointer to NodeInfo for every node in the spec.
+   */
+  nodes: Map<string, NodeInfo>;
+  /**
+   * For each node with at least one dependent, the set of nodes that reference it via $ref.
+   * Nodes with no dependents are omitted from this map.
+   */
+  reverseNodeDependencies: Map<string, Set<string>>;
+  /**
+   * For each node, the set of direct $ref targets that appear anywhere inside the node's
+   * subtree (the node itself and its children). This is populated during graph construction
+   * and is used to compute top-level dependency relationships where $ref may be attached to
+   * child pointers instead of the parent.
+   */
+  subtreeDependencies: Map<string, Set<string>>;
+  /**
+   * For each node, the set of all (transitive) normalized JSON Pointers it references via $ref anywhere in its subtree.
+   * This includes both direct and indirect dependencies, making it useful for filtering, codegen, and tree-shaking.
+   */
+  transitiveDependencies: Map<string, Set<string>>;
+};
+
+/**
+ * Information about a node in the OpenAPI graph.
+ *
+ * @property deprecated - Whether the node is deprecated. Optional.
+ * @property key - The property name or array index in the parent, or null for root.
+ * @property node - The actual object at this pointer in the spec.
+ * @property parentPointer - The JSON Pointer of the parent node, or null for root.
+ * @property scopes - The set of access scopes for this node, if any. Optional.
+ * @property tags - The set of tags for this node, if any. Optional.
+ */
+export type NodeInfo = {
+  /** Whether the node is deprecated. Optional. */
+  deprecated?: boolean;
+  /** The property name or array index in the parent, or null for root. */
+  key: string | number | null;
+  /** The actual object at this pointer in the spec.  */
+  node: unknown;
+  /** The JSON Pointer of the parent node, or null for root. */
+  parentPointer: string | null;
+  /** The set of access scopes for this node, if any. Optional. */
+  scopes?: Set<Scope>;
+  /** The set of tags for this node, if any. Optional. */
+  tags?: Set<string>;
+};