fix(legacy-json): misc promotion logic fix (#493)

* fix(legacy-json): misc promotion logic fix

Re nodejs/node#57343 (comment)

Signed-off-by: flakey5 <[email protected]>

* unit tests

Signed-off-by: flakey5 <[email protected]>

---------

Signed-off-by: flakey5 <[email protected]>

Author: flakey5

src/generators/legacy-json/utils/__tests__/buildSection.test.mjs

@@ -0,0 +1,138 @@
+'use strict';
+
+import assert from 'node:assert/strict';
+import { describe, test } from 'node:test';
+
+import { UNPROMOTED_KEYS } from '../../constants.mjs';
+import { promoteMiscChildren } from '../buildSection.mjs';
+
+describe('promoteMiscChildren', () => {
+  /**
+   * @template {object} T
+   *
+   * @param {T} base
+   * @param {'section'|'parent'} [type='section']
+   * @returns {T}
+   */
+  function buildReadOnlySection(base, type = 'section') {
+    return new Proxy(base, {
+      set(_, key) {
+        throw new Error(`${type} property '${String(key)} modified`);
+      },
+    });
+  }
+
+  test('ignores non-misc section', () => {
+    const section = buildReadOnlySection({
+      type: 'text',
+    });
+
+    const parent = buildReadOnlySection(
+      {
+        type: 'text',
+      },
+      'parent'
+    );
+
+    promoteMiscChildren(section, parent);
+  });
+
+  test('ignores misc parent', () => {
+    const section = buildReadOnlySection({
+      type: 'misc',
+    });
+
+    const parent = buildReadOnlySection(
+      {
+        type: 'misc',
+      },
+      'parent'
+    );
+
+    promoteMiscChildren(section, parent);
+  });
+
+  test('ignores keys in UNPROMOTED_KEYS', () => {
+    const sectionRaw = {
+      type: 'misc',
+      promotableKey: 'this should be promoted',
+    };
+
+    UNPROMOTED_KEYS.forEach(key => {
+      if (key === 'type') {
+        return;
+      }
+
+      sectionRaw[key] = 'this should be ignored';
+    });
+
+    const section = buildReadOnlySection(sectionRaw);
+
+    const parent = {
+      type: 'module',
+    };
+
+    promoteMiscChildren(section, parent);
+
+    UNPROMOTED_KEYS.forEach(key => {
+      if (key === 'type') {
+        return;
+      }
+
+      if (parent[key]) {
+        throw new Error(`'${key}' was promoted`);
+      }
+    });
+
+    assert.strictEqual(parent.promotableKey, section.promotableKey);
+  });
+
+  describe('merges properties correctly', () => {
+    test('pushes child property if parent is an array', () => {
+      const section = buildReadOnlySection({
+        type: 'misc',
+        someValue: 'bar',
+      });
+
+      const parent = {
+        type: 'module',
+        someValue: ['foo'],
+      };
+
+      promoteMiscChildren(section, parent);
+
+      assert.deepStrictEqual(parent.someValue, ['foo', 'bar']);
+    });
+
+    test('ignores child property if parent has a value that is not an array', () => {
+      const section = buildReadOnlySection({
+        type: 'misc',
+        someValue: 'bar',
+      });
+
+      const parent = {
+        type: 'module',
+        someValue: 'foo',
+      };
+
+      promoteMiscChildren(section, parent);
+
+      assert.strictEqual(parent.someValue, 'foo');
+    });
+
+    test('promotes child property if parent does not have the property', () => {
+      const section = buildReadOnlySection({
+        type: 'misc',
+        someValue: 'bar',
+      });
+
+      const parent = {
+        type: 'module',
+      };
+
+      promoteMiscChildren(section, parent);
+
+      assert.deepStrictEqual(parent.someValue, 'bar');
+    });
+  });
+});

src/generators/legacy-json/utils/buildSection.mjs

@@ -5,6 +5,28 @@ import { getRemarkRehype } from '../../../utils/remark.mjs';
 import { transformNodesToString } from '../../../utils/unist.mjs';
 import { SECTION_TYPE_PLURALS, UNPROMOTED_KEYS } from '../constants.mjs';
 
+/**
+ * Promotes children properties to the parent level if the section type is 'misc'.
+ * @param {import('../types.d.ts').Section} section - The section to promote.
+ * @param {import('../types.d.ts').Section} parent - The parent section.
+ */
+export const promoteMiscChildren = (section, parent) => {
+  // Only promote if the current section is of type 'misc' and the parent is not 'misc'
+  if (section.type === 'misc' && parent.type !== 'misc') {
+    Object.entries(section).forEach(([key, value]) => {
+      // Only promote certain keys
+      if (!UNPROMOTED_KEYS.includes(key)) {
+        // Merge the section's properties into the parent section
+        if (parent[key] && Array.isArray(parent[key])) {
+          parent[key] = parent[key].concat(value);
+        } else {
+          parent[key] ||= value;
+        }
+      }
+    });
+  }
+};
+
 /**
  *
  */
@@ -104,28 +126,6 @@ export const createSectionBuilder = () => {
     parent[key].push(section);
   };
 
-  /**
-   * Promotes children properties to the parent level if the section type is 'misc'.
-   * @param {import('../types.d.ts').Section} section - The section to promote.
-   * @param {import('../types.d.ts').Section} parent - The parent section.
-   */
-  const promoteMiscChildren = (section, parent) => {
-    // Only promote if the current section is of type 'misc' and the parent is not 'misc'
-    if (section.type === 'misc' && parent.type !== 'misc') {
-      Object.entries(section).forEach(([key, value]) => {
-        // Only promote certain keys
-        if (!UNPROMOTED_KEYS.includes(key)) {
-          // Merge the section's properties into the parent section
-          parent[key] = parent[key]
-            ? // If the parent already has this key, concatenate the values
-              [].concat(parent[key], value)
-            : // Otherwise, directly assign the section's value to the parent
-              [];
-        }
-      });
-    }
-  };
-
   /**
    * Processes children of a given entry and updates the section.
    * @param {import('../types.d.ts').HierarchizedEntry} entry - The current entry.