fix(xref): don't let a for-scoped lowercase term shadow a canonical concept (#533)

#508 preserved original dfn term case, splitting "URL" and "url" into
separate keys in the store. A bare [=URL=] lowercases to "url", which
matched only the for-scoped basic URL parser variable and got filtered out
by the no-for context check, never falling back to the canonical URL
concept — breaking the link on every spec. filter() now tries the
exact-case bucket first, then falls back to the case-insensitive variants
when it yields nothing after filtering.

Author: marcoscaceres

routes/xref/lib/search.ts

@@ -126,18 +126,22 @@ function filter(query: Query, store: Store, options: Options) {
   const isIDL = types.some(t => IDL_TYPES.has(t));
   const allowCaseFallback = !isIDL;
 
-  let result: DataEntry[] = [];
   for (const term of getTermVariations(query)) {
-    const byTerm = filterByTerm(term, store, allowCaseFallback);
-    const bySpec = filterBySpec(byTerm, query);
-    const byType = filterByType(bySpec, query);
-    const byForContext = filterByForContext(byType, query, options);
-    if (byForContext.length) {
-      result = byForContext;
-      break;
+    // Try the exact-case bucket first (so `[=baseline=]` and `{{Baseline}}`
+    // stay distinct), then a case-insensitive fallback. The fallback is
+    // essential when an exact-case bucket exists but every entry is filtered
+    // out downstream — e.g. term "url" matches only the for-scoped basic URL
+    // parser variable, while the canonical URL concept lives under "URL".
+    for (const byTerm of termCandidates(term, store, allowCaseFallback)) {
+      const bySpec = filterBySpec(byTerm, query);
+      const byType = filterByType(bySpec, query);
+      const byForContext = filterByForContext(byType, query, options);
+      if (byForContext.length) {
+        return byForContext;
+      }
     }
   }
-  return result;
+  return [];
 }
 
 function getTermVariations(query: Query) {
@@ -161,20 +165,31 @@ function getTermVariations(query: Query) {
   }
 }
 
-function filterByTerm(term: Query["term"], store: Store, allowCaseFallback: boolean) {
-  if (term == null) return [];
+/**
+ * Yields candidate entry sets for a term in priority order: the exact-case
+ * bucket, then (non-IDL only) the case-insensitive fallback union, each
+ * fallback entry tagged with its canonical term. Two separate sets so the
+ * caller can prefer an exact match that survives filtering and reach for the
+ * fallback only when it doesn't.
+ */
+function* termCandidates(
+  term: Query["term"],
+  store: Store,
+  allowCaseFallback: boolean,
+) {
+  if (term == null) return;
   const direct = store.byTerm[term];
-  if (direct) return direct;
-  if (!allowCaseFallback) return [];
-  // Case-insensitive fallback: tag each entry with its canonical term so
-  // downstream consumers (e.g. the xref UI) can build correct cite syntax
-  // instead of using the user's potentially miscased input.
+  if (direct) yield direct;
+  if (!allowCaseFallback) return;
   const lower = term.toLowerCase();
   const variants = store.byTermLower.get(lower);
-  if (!variants) return [];
-  return variants.flatMap(v =>
-    (store.byTerm[v] || []).map(entry => ({ ...entry, term: v })),
-  );
+  if (!variants) return;
+  const fallback = variants
+    .filter(v => v !== term) // exclude the exact bucket already yielded above
+    .flatMap(v =>
+      (store.byTerm[v] || []).map(entry => ({ ...entry, term: v })),
+    );
+  if (fallback.length) yield fallback;
 }
 
 function filterBySpec(data: DataEntry[], query: Query) {

tests/routes/xref/lib/data-by-term.js

@@ -290,4 +290,29 @@ export default {
       normative: false,
     },
   ],
+  // The canonical URL concept is indexed under "URL" (case preserved), while a
+  // distinct for-scoped "url" (the basic URL parser's local variable) exists
+  // under the lowercase key. A lowercased query ("url") must still resolve the
+  // canonical concept and not be shadowed by the for-scoped lowercase entry.
+  URL: [
+    {
+      type: "dfn",
+      spec: "url",
+      shortname: "url",
+      status: "current",
+      uri: "#concept-url",
+      normative: true,
+    },
+  ],
+  url: [
+    {
+      type: "dfn",
+      spec: "url",
+      shortname: "url",
+      status: "current",
+      uri: "#basic-url-parser-url",
+      normative: true,
+      for: ["basic URL parser"],
+    },
+  ],
 };

tests/routes/xref/lib/search.test.js

@@ -21,7 +21,6 @@ describe("xref - search", () => {
   beforeEach(() => cache.clear());
 
   describe("options", () => {
-
     describe("query", () => {
       it("adds query back to response if requested", () => {
         expect(_search([], store, { query: true })).toEqual({
@@ -83,15 +82,15 @@ describe("xref - search", () => {
       });
 
       it("uses filter@for if options.all is set and for is provided", () => {
-        expect(
-          search({ term: "event", for: "Window" }, { all: true }),
-        ).toEqual([resultsAll[1]]);
+        expect(search({ term: "event", for: "Window" }, { all: true })).toEqual(
+          [resultsAll[1]],
+        );
       });
 
       it("uses filter@for if options.all is not set", () => {
-        expect(
-          search({ term: "event", for: "Window" }, { all: true }),
-        ).toEqual([resultsAll[1]]);
+        expect(search({ term: "event", for: "Window" })).toEqual([
+          resultsAll[1],
+        ]);
       });
     });
   });
@@ -169,7 +168,9 @@ describe("xref - search", () => {
 
       // Exact match: no term field (not a fallback hit)
       const exact = searchWithTerm({ term: "baseline" });
-      expect(exact).toEqual([{ uri: "text.html#TermBaseline", term: undefined }]);
+      expect(exact).toEqual([
+        { uri: "text.html#TermBaseline", term: undefined },
+      ]);
 
       // Case-insensitive fallback: term field present with canonical casing
       const fallback = searchWithTerm({ term: "baseLine" });
@@ -180,15 +181,13 @@ describe("xref - search", () => {
     });
 
     it("preserves case for element-type queries", () => {
-      const foreignObject = [
-        { uri: "embedded.html#elementdef-foreignObject" },
-      ];
-      expect(
-        search({ term: "foreignObject", types: ["element"] }),
-      ).toEqual(foreignObject);
-      expect(
-        search({ term: "foreignObject", types: ["_CONCEPT_"] }),
-      ).toEqual(foreignObject);
+      const foreignObject = [{ uri: "embedded.html#elementdef-foreignObject" }];
+      expect(search({ term: "foreignObject", types: ["element"] })).toEqual(
+        foreignObject,
+      );
+      expect(search({ term: "foreignObject", types: ["_CONCEPT_"] })).toEqual(
+        foreignObject,
+      );
       expect(
         search({ term: "foreignObject", types: ["element", "dfn"] }),
       ).toEqual(foreignObject);
@@ -200,9 +199,23 @@ describe("xref - search", () => {
       expect(search({ term: "clipPath", types: ["_CONCEPT_"] })).toEqual(
         clipPath,
       );
+      expect(search({ term: "clipPath", types: ["element", "dfn"] })).toEqual(
+        clipPath,
+      );
+    });
+
+    it("resolves a canonical concept shadowed by a for-scoped lowercase term", () => {
+      // Regression: [=URL=] sends term "url" (concepts are lowercased). The
+      // canonical URL concept is indexed under "URL"; a distinct for-scoped
+      // "url" (basic URL parser local variable) exists under the lowercase key.
+      // The lowercase direct hit must not shadow the canonical concept...
+      expect(search({ term: "url", types: ["_CONCEPT_"] })).toEqual([
+        { uri: "#concept-url" },
+      ]);
+      // ...but the for-scoped entry still wins when its for is given.
       expect(
-        search({ term: "clipPath", types: ["element", "dfn"] }),
-      ).toEqual(clipPath);
+        search({ term: "url", types: ["_CONCEPT_"], for: "basic URL parser" }),
+      ).toEqual([{ uri: "#basic-url-parser-url" }]);
     });
   });