[CSS post-processing] Merge scopes when possible (breaking) (#1858)

A CSS feature may be defined within a single dfn for multiple scopes. These
features appeared in separate entries in the consolidated file. To ease
indexing and lookup by feature name, this update further merges scopes.

This does not fully turn feature names into identifiers, but only 8 features
remain that use the same name but have different definitions for different
scopes. Some (but not all) of them, we should be able to get rid of through
spec fixes and possible improvements to the curation logic in Webref.

See list and details at:
w3c/webref#1519 (comment)

This is a breaking change since it turns `for` keys into arrays in the
consolidated file.

Side note: the update also fixes an unrelated comment in the code.

Author: tidoust

schemas/postprocessing/css.json

@@ -2,6 +2,16 @@
   "$schema": "http://json-schema.org/schema#",
   "$id": "https://github.com/w3c/reffy/blob/main/schemas/postprocessing/css.json",
 
+  "$defs": {
+    "scopes": {
+      "type": "array",
+      "items": {
+        "type": "string"
+      },
+      "minItems": 1
+    }
+  },
+
   "type": "object",
   "additionalProperties": false,
   "required": ["atrules", "functions", "properties", "selectors", "types"],
@@ -42,7 +52,7 @@
         "additionalProperties": false,
         "properties": {
           "name": { "type": "string", "pattern": "^<[^>]+>$|^.*()$" },
-          "for": { "type": "string" },
+          "for": { "$ref": "#/$defs/scopes" },
           "href": { "$ref": "../common.json#/$defs/url" },
           "type": { "type": "string", "enum": ["function"] },
           "prose": { "type": "string" },
@@ -91,7 +101,7 @@
         "additionalProperties": false,
         "properties": {
           "name": { "type": "string", "pattern": "^<[^>]+>$|^.*()$" },
-          "for": { "type": "string" },
+          "for": { "$ref": "#/$defs/scopes" },
           "href": { "$ref": "../common.json#/$defs/url" },
           "type": { "type": "string", "enum": ["type"] },
           "prose": { "type": "string" },

src/postprocessing/cssmerge.js

@@ -13,7 +13,7 @@
  * In CSS extracts, functions and types that are defined for another construct
  * appear under the `values` key of that construct entry. In the resulting
  * construct, they get copied to the root lists under `functions` or `types`,
- * and get a `for` key that contains the name of the construct that they are
+ * and get a `for` key that contains the list of constructs that they are
  * defined for.
  *
  * CSS properties that are defined in one spec and extended in other specs get
@@ -178,8 +178,8 @@ export default {
       // (that has some known syntax) in the most recent level. Move that base
       // definition to the beginning of the array and get rid of other base
       // definitions.
-      // (Note: the code throws an error if duplicates of base definitions in
-      // unrelated specs still exist)
+      // (Note: the code chooses one definition if duplicates of base
+      // definitions in unrelated specs still exist)
       for (const [name, dfns] of Object.entries(featureDfns)) {
         let actualDfns = dfns.filter(dfn => dfn.value);
         if (actualDfns.length === 0) {
@@ -296,6 +296,32 @@ export default {
         }
       }
 
+      // The same feature may be defined for multiple scopes.
+      // To ease indexing and lookup by feature name, let's merge the scopes
+      // when possible, turning the `for` key into an array. This will not get
+      // rid of all scoping duplicates, but should still make the feature name
+      // unique for all but a handful of them.
+      categorized[category] = categorized[category]
+        .map((feature, idx, list) => {
+          const first = list.find(f => f.href === feature.href);
+          if (first === feature) {
+            if (feature.for) {
+              feature.for = [feature.for];
+            }
+            return feature;
+          }
+          // Not the first time we see this feature, let's merge scopes.
+          // Both scopes should be defined as there is no way to author a
+          // single dfn that defines a feature both as scoped and unscoped.
+          if (!first.for || !feature.for) {
+            throw new Error(`Feature ${feature.name} defined both as unscoped and scoped within the same dfn, see ${feature.href}`);
+          }
+          first.for.push(feature.for);
+          first.for.sort();
+          return null;
+        })
+        .filter(feature => !!feature);
+
       // Let's sort lists before we return to ease human-readability and
       // avoid non-substantive diff
       for (const feature of categorized[category]) {
@@ -313,13 +339,14 @@ export default {
 
 
 /**
- * Return the identifier of a feature, taking scoping construct into account
+ * Return the identifier of a feature, taking scoping construct(s) into account
  * when needed.
  */
 function getFeatureId(feature) {
   let featureId = feature.name;
   if (feature.for) {
-    featureId += ' for ' + feature.for;
+    featureId += ' for ' +
+      (Array.isArray(feature.for) ? feature.for.join(',') : feature.for);
   }
   return featureId;
 }

test/merge-css.js

@@ -193,12 +193,12 @@ describe('CSS extracts consolidation', function () {
     assert.deepEqual(result, Object.assign({}, emptyMerged, {
       functions: [
         Object.assign({}, functionEnv, {
-          for: '<track-repeat>'
+          for: ['<track-repeat>']
         })
       ],
       types: [
         Object.assign({}, type1, {
-          for: '<track-repeat>'
+          for: ['<track-repeat>']
         }),
         {
           name: '<track-repeat>',
@@ -504,4 +504,84 @@ describe('CSS extracts consolidation', function () {
       property1
     ]);
   });
+
+  it('merges scopes when possible', async () => {
+    // We'll have 3 definitions of the `env()` functions: one unscoped,
+    // another one scoped to two types, and a third one scoped to yet another
+    // type. Note syntaxes must differ otherwise merge will drop duplicates.
+    const scopedFunctionEnv = Object.assign({}, functionEnv,  {
+      href: 'https://drafts.csswg.org/css-first-1/#funcdef-env',
+      value: 'env(first)'
+    });
+    const otherScopedFunctionEnv = Object.assign({}, functionEnv,  {
+      href: 'https://drafts.csswg.org/css-second-1/#funcdef-env',
+      value: 'env(second)'
+    });
+    const results = structuredClone([
+      {
+        shortname: 'css-stuff-1',
+        series: { shortname: 'css-stuff' },
+        seriesVersion: '1',
+        css: Object.assign({}, emptyExtract, {
+          values: [
+            functionEnv,
+            {
+              name: '<track-repeat>',
+              href: 'https://drafts.csswg.org/css-grid-2/#typedef-track-repeat',
+              type: 'type',
+              values: [
+                scopedFunctionEnv
+              ]
+            },
+            {
+              name: '<repeat-ad-libitum>',
+              href: 'https://drafts.csswg.org/css-grid-2/#typedef-repeat-ad-libitum',
+              type: 'type',
+              values: [
+                scopedFunctionEnv
+              ]
+            },
+            {
+              name: '<another-repeat>',
+              href: 'https://drafts.csswg.org/css-grid-2/#typedef-another-repeat',
+              type: 'type',
+              values: [
+                otherScopedFunctionEnv
+              ]
+            }
+          ]
+        })
+      }
+    ]);
+    const result = await cssmerge.run({ results });
+    console.log(JSON.stringify(result, null, 2));
+    assert.deepEqual(result, Object.assign({}, emptyMerged, {
+      functions: [
+        functionEnv,
+        Object.assign({}, otherScopedFunctionEnv, {
+          for: ['<another-repeat>']
+        }),
+        Object.assign({}, scopedFunctionEnv, {
+          for: ['<repeat-ad-libitum>', '<track-repeat>']
+        }),
+      ],
+      types: [
+        {
+          name: '<another-repeat>',
+          href: 'https://drafts.csswg.org/css-grid-2/#typedef-another-repeat',
+          type: 'type'
+        },
+        {
+          name: '<repeat-ad-libitum>',
+          href: 'https://drafts.csswg.org/css-grid-2/#typedef-repeat-ad-libitum',
+          type: 'type'
+        },
+        {
+          name: '<track-repeat>',
+          href: 'https://drafts.csswg.org/css-grid-2/#typedef-track-repeat',
+          type: 'type'
+        }
+      ]
+    }));
+  });
 });