fix: fix resolution of various type annotation edge cases

.changeset/display-name-type-unions.md

@@ -0,0 +1,5 @@
+---
+'@node-core/doc-kit': patch
+---
+
+Resolve unions and arrays of display-name types (`{HTTP/2 Headers Object | vm.Module}`, `{HTTP/2 Headers Object[]}`), and stop capturing prose such as `U+007B ({), and U+007D (}).` as a type annotation.

packages/core/src/generators/metadata/utils/__tests__/resolveTypes.test.mjs

@@ -0,0 +1,162 @@
+import assert from 'node:assert/strict';
+import { describe, it, mock, beforeEach } from 'node:test';
+
+const warnings = [];
+
+mock.module('../../../../logger/index.mjs', {
+  defaultExport: {
+    warn: message => warnings.push(message),
+  },
+});
+
+const { resolveTypeAnnotations } = await import('../resolveTypes.mjs');
+
+const HTTP2_TYPE_MAP = {
+  'HTTP/2 Headers Object': 'http2.html#headers-object',
+};
+
+/**
+ * Resolves a single annotation and returns its node
+ *
+ * @param {string} value The type value
+ * @param {Record<string, string>} typeMap The mapping of types to links
+ */
+const resolve = (value, typeMap = {}) => {
+  const node = { type: 'typeAnnotation', value };
+
+  resolveTypeAnnotations(
+    { type: 'root', children: [node] },
+    typeMap,
+    'test.md'
+  );
+
+  return node;
+};
+
+/**
+ * Resolves a single annotation and returns its links as `[text, href]` pairs,
+ * asserting that every range lines up with the text it links
+ *
+ * @param {string} value The type value
+ * @param {Record<string, string>} typeMap The mapping of types to links
+ */
+const linksIn = (value, typeMap = {}) => {
+  const { data } = resolve(value, typeMap);
+
+  for (const link of data.links) {
+    assert.equal(value.slice(link.start, link.end), link.text);
+  }
+
+  return data.links.map(({ text, href }) => [text, href]);
+};
+
+describe('resolveTypeAnnotations', () => {
+  beforeEach(() => {
+    warnings.length = 0;
+  });
+
+  it('links a type that is a whole map key', () => {
+    assert.deepEqual(linksIn('HTTP/2 Headers Object', HTTP2_TYPE_MAP), [
+      ['HTTP/2 Headers Object', 'http2.html#headers-object'],
+    ]);
+    assert.deepEqual(warnings, []);
+  });
+
+  it('links an array of a display name', () => {
+    assert.deepEqual(linksIn('HTTP/2 Headers Object[]', HTTP2_TYPE_MAP), [
+      ['HTTP/2 Headers Object', 'http2.html#headers-object'],
+    ]);
+    assert.deepEqual(warnings, []);
+  });
+
+  it('resolves module-qualified names alongside display names', () => {
+    assert.deepEqual(linksIn('Module Namespace Object | vm.Module'), [
+      [
+        'Module Namespace Object',
+        'https://tc39.github.io/ecma262/#sec-module-namespace-exotic-objects',
+      ],
+      ['vm.Module', 'vm.html#class-vmmodule'],
+    ]);
+    assert.deepEqual(warnings, []);
+  });
+
+  it('keeps the resolvable parts of a partly unknown union', () => {
+    assert.deepEqual(
+      linksIn('HTTP/2 Headers Object | HTTP/2 Raw Headers', HTTP2_TYPE_MAP),
+      [['HTTP/2 Headers Object', 'http2.html#headers-object']]
+    );
+    assert.deepEqual(warnings, []);
+  });
+
+  it('warns when no part of an unparsable type resolves', () => {
+    const node = resolve('HTTP/2 Raw Headers', HTTP2_TYPE_MAP);
+
+    assert.equal(node.data.parseError, true);
+    assert.deepEqual(node.data.links, []);
+    assert.deepEqual(warnings, [
+      'Invalid type annotation: {HTTP/2 Raw Headers}',
+    ]);
+  });
+
+  it('does not read `||` in default-value prose as a union', () => {
+    const node = resolve("req.url || '/'");
+
+    assert.deepEqual(node.data.links, []);
+    assert.deepEqual(warnings, ["Invalid type annotation: {req.url || '/'}"]);
+  });
+
+  it('marks which values are TypeScript, for the highlighter', () => {
+    const isTypeScript = (value, typeMap) =>
+      resolve(value, typeMap).data.typescript;
+
+    assert.equal(isTypeScript('Promise<string> | null'), true);
+    assert.equal(isTypeScript('HTTP/2 Headers Object', HTTP2_TYPE_MAP), false);
+    assert.equal(isTypeScript('Module Namespace Object | vm.Module'), false);
+    assert.equal(isTypeScript('HTTP/2 Raw Headers', HTTP2_TYPE_MAP), false);
+  });
+
+  it('parses valid TypeScript rather than splitting it', () => {
+    assert.deepEqual(
+      linksIn('Promise<string> | Buffer', { Buffer: 'buffer.html#buffer' }),
+      [
+        [
+          'Promise',
+          'https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Promise',
+        ],
+        [
+          'string',
+          'https://developer.mozilla.org/docs/Web/JavaScript/Data_structures#string_type',
+        ],
+        ['Buffer', 'buffer.html#buffer'],
+      ]
+    );
+    assert.deepEqual(warnings, []);
+  });
+
+  it('resolves every annotation in a nested tree', () => {
+    const STRING_URL =
+      'https://developer.mozilla.org/docs/Web/JavaScript/Data_structures#string_type';
+
+    // Two per paragraph: a visitor that skips siblings still reaches the first
+    const nodes = ['string', 'HTTP/2 Headers Object', 'string', 'string'].map(
+      value => ({ type: 'typeAnnotation', value })
+    );
+
+    resolveTypeAnnotations(
+      {
+        type: 'root',
+        children: [
+          { type: 'paragraph', children: nodes.slice(0, 2) },
+          { type: 'paragraph', children: nodes.slice(2) },
+        ],
+      },
+      HTTP2_TYPE_MAP,
+      'test.md'
+    );
+
+    assert.deepEqual(
+      nodes.map(({ data }) => data.links.map(({ href }) => href)),
+      [[STRING_URL], ['http2.html#headers-object'], [STRING_URL], [STRING_URL]]
+    );
+  });
+});

packages/core/src/generators/metadata/utils/__tests__/transformers.test.mjs

@@ -61,4 +61,15 @@ describe('resolveTypeReference', () => {
   it('returns an empty string for unknown plain names', () => {
     strictEqual(resolveTypeReference('NotAThing'), '');
   });
+
+  it('applies the dotted heuristic to identifier paths only', () => {
+    strictEqual(
+      resolveTypeReference('os.constants.dlopen'),
+      'os.html#osconstantsdlopen'
+    );
+
+    for (const name of ['Buffer or fs.Stats', 'Object.<string, string>']) {
+      strictEqual(resolveTypeReference(name), '');
+    }
+  });
 });

packages/core/src/generators/metadata/utils/resolveTypes.mjs

@@ -112,52 +112,93 @@ export const parseTypeValues = values => {
 };
 
 /**
- * Resolves every `typeAnnotation` node in a file's tree
+ * Resolves a type value as a `|`-separated union of display names (e.g.
+ * `HTTP/2 Headers Object | HTTP/2 Raw Headers`). Map keys may contain spaces
+ * and slashes, so such values may not parse as TypeScript, but each part is
+ * still resolvable on its own.
+ *
+ * @param {string} value The type value
+ * @param {Record<string, string>} typeMap The toolchain mapping of types to links
+ * @returns {Array<{ start: number, end: number, text: string, href: string }>} The resolved links
  */
-export const resolveTypeAnnotations = (tree, typeMap, path) => {
-  const pending = [];
+const resolveDisplayNames = (value, typeMap) => {
+  const links = [];
 
-  visit(tree, 'typeAnnotation', node => {
-    node.data = { links: [] };
+  for (const { 0: part, index } of value.matchAll(/(?:[^|]|\|\|)+/g)) {
+    // An array of a type links to the type itself
+    const name = part.trim().replace(/(\[\])+$/, '');
+    const href = name && resolveTypeReference(name, typeMap);
 
-    const url = lookupTypeName(node.value, typeMap);
+    if (href) {
+      const start = index + part.indexOf(name);
 
-    if (url) {
-      node.data.links.push({
-        start: 0,
-        end: node.value.length,
-        text: node.value,
-        href: url,
-      });
-    } else {
-      pending.push(node);
+      links.push({ start, end: start + name.length, text: name, href });
     }
-  });
+  }
+
+  return links;
+};
+
+/**
+ * Resolves a type value's links: the whole value when it is a map key in its
+ * own right, then its parsed identifiers, then its display names
+ *
+ * @param {string} value The type value
+ * @param {Record<string, string>} typeMap The toolchain mapping of types to links
+ * @param {{ error?: boolean, identifiers?: Array<object> }} result The parse of `value`
+ * @returns {Array<{ start: number, end: number, text: string, href: string }>} The resolved links
+ */
+const resolveLinks = (value, typeMap, result) => {
+  const url = lookupTypeName(value, typeMap);
+
+  if (url) {
+    return [{ start: 0, end: value.length, text: value, href: url }];
+  }
+
+  if (result.error) {
+    return resolveDisplayNames(value, typeMap);
+  }
 
-  parseTypeValues(pending.map(({ value }) => value)).forEach(
-    (result, index) => {
-      const node = pending[index];
+  return (
+    result.identifiers
+      .map(({ start, end, text, lookup }) => ({
+        start,
+        end,
+        text,
+        href: resolveTypeReference(lookup, typeMap),
+      }))
+      .filter(({ href }) => href)
+      // The hast handlers slice the value by these ranges in order
+      .sort((a, b) => a.start - b.start)
+  );
+};
 
-      if (result.error) {
-        node.data.parseError = true;
+/**
+ * Resolves every `typeAnnotation` node in a file's tree
+ */
+export const resolveTypeAnnotations = (tree, typeMap, path) => {
+  const nodes = [];
 
-        logger.warn(`Invalid type annotation: {${node.value}}`, {
-          file: { path, position: node.position },
-        });
+  // A visitor must not return a value: `visit` reads a number as the index to
+  // continue from, which would skip siblings
+  visit(tree, 'typeAnnotation', node => {
+    nodes.push(node);
+  });
 
-        return;
-      }
+  parseTypeValues(nodes.map(({ value }) => value)).forEach((result, index) => {
+    const node = nodes[index];
+    const links = resolveLinks(node.value, typeMap, result);
 
-      for (const { start, end, text, lookup } of result.identifiers) {
-        const href = resolveTypeReference(lookup, typeMap);
+    // Unparseable display names must not be highlighted as the TypeScript
+    // they are not (which colours the `/` and `2` of `HTTP/2`)
+    node.data = { links, typescript: !result.error };
 
-        if (href) {
-          node.data.links.push({ start, end, text, href });
-        }
-      }
+    if (result.error && !links.length) {
+      node.data.parseError = true;
 
-      // The hast handlers slice the value by these ranges in order
-      node.data.links.sort((a, b) => a.start - b.start);
+      logger.warn(`Invalid type annotation: {${node.value}}`, {
+        file: { path, position: node.position },
+      });
     }
-  );
+  });
 };

packages/core/src/generators/metadata/utils/transformers.mjs

@@ -48,6 +48,11 @@ export const lookupTypeName = (name, typeMap) => {
   return '';
 };
 
+// A dotted identifier path, e.g. `vm.Module` or `os.constants.dlopen`. Names
+// that merely contain a dot (prose, `Object.<string, string>`) must not be
+// turned into a module link.
+const MODULE_QUALIFIED_NAME = /^[A-Za-z_$][\w$]*(?:\.[A-Za-z_$][\w$]*)+$/;
+
 /**
  * Resolves a type identifier to a documentation URL: map lookups first, then
  * the dotted-name heuristic for Node.js types like `vm.Module` (which links
@@ -65,7 +70,7 @@ export const resolveTypeReference = (name, typeMap) => {
   }
 
   // Transform Node.js types like 'vm.Something'.
-  if (name.indexOf('.') >= 0) {
+  if (MODULE_QUALIFIED_NAME.test(name)) {
     const [mod, ...pieces] = name.split('.');
     const isClass = pieces.at(-1).match(/^[A-Z][a-z]/);
 

packages/core/src/utils/type-annotations/__tests__/hast.test.mjs

@@ -2,6 +2,7 @@ import assert from 'node:assert/strict';
 import { describe, it } from 'node:test';
 
 import { toHtml } from 'hast-util-to-html';
+import { toString } from 'hast-util-to-string';
 
 import {
   typeAnnotationToHast,
@@ -63,6 +64,7 @@ describe('typeAnnotationToHast', () => {
 describe('typeAnnotationToHighlightedHast', () => {
   it('produces one inline <code> with shiki tokens and embedded links', () => {
     const node = makeNode('Promise<string>', {
+      typescript: true,
       links: [{ start: 0, end: 7, text: 'Promise', href: 'mdn.io/promise' }],
     });
 
@@ -84,6 +86,7 @@ describe('typeAnnotationToHighlightedHast', () => {
 
   it('splits a link range that crosses token boundaries', () => {
     const node = makeNode('vm.Module', {
+      typescript: true,
       links: [
         { start: 0, end: 9, text: 'vm.Module', href: 'vm.html#class-module' },
       ],
@@ -97,6 +100,32 @@ describe('typeAnnotationToHighlightedHast', () => {
     assert.match(html.replace(/<[^>]+>/g, ''), /^vm\.Module$/);
   });
 
+  it('highlights a display name as plain text', () => {
+    const value = 'HTTP/2 Headers Object';
+    const links = [
+      { start: 0, end: value.length, text: value, href: 'http2.html#headers' },
+    ];
+
+    const asTypeScript = typeAnnotationToHighlightedHast(
+      state,
+      makeNode(value, { typescript: true, links })
+    );
+
+    const asDisplayName = typeAnnotationToHighlightedHast(
+      state,
+      makeNode(value, { links })
+    );
+
+    // TypeScript colours `/` as an operator and `2` as a numeric literal
+    assert.match(toHtml(asTypeScript), /<span style="color:/);
+    assert.doesNotMatch(toHtml(asDisplayName), /<span style="color:/);
+
+    // Still one highlighted <code> with the whole name linked
+    assert.match(toHtml(asDisplayName), /^<code[^>]*class="[^"]*shiki[^"]*/);
+    assert.match(toHtml(asDisplayName), /<a href="http2.html#headers"/);
+    assert.equal(toString(asDisplayName), value);
+  });
+
   it('falls back to plain code when nothing resolved', () => {
     const node = makeNode('Whatever', { links: [] });