fix: increase type guard vigilance

Author: ca-d

convertEdit.spec.ts

@@ -1,10 +1,10 @@
 /* eslint-disable @typescript-eslint/no-unused-expressions */
 import { expect } from '@open-wc/testing';
 
-import { Insert, Remove, Update } from './editv1.js';
+import { Update } from './editv1.js';
 
 import { convertEdit } from './convertEdit.js';
-import { SetAttributes } from './editv2.js';
+import { Insert, Remove, SetAttributes } from './editv2.js';
 
 const doc = new DOMParser().parseFromString(
   '<SCL><Substation name="AA1"></Substation></SCL>',
@@ -26,7 +26,6 @@ const update: Update = {
   attributes: {
     name: 'A2',
     desc: null,
-    ['__proto__']: 'a string',
     'myns:attr': {
       value: 'value1',
       namespaceURI: 'http://example.org/myns',
@@ -43,9 +42,9 @@ const update: Update = {
       value: 'value2',
       namespaceURI: 'http://example.org/myns2',
     },
-    attr3: {
-      value: 'value3',
-      namespaceURI: null,
+    invalid: {
+      value: 'great, but with empty namespace URI',
+      namespaceURI: '',
     },
   },
 };

convertEdit.ts

@@ -1,20 +1,16 @@
+import { Edit, isComplex, isNamespaced, isUpdate, Update } from './editv1.js';
+
 import {
-  Edit,
-  isComplex,
+  Attributes,
+  AttributesNS,
+  EditV2,
   isInsert,
-  isNamespaced,
-  isUpdate,
   isRemove,
-  Update,
-} from './editv1.js';
-
-import { EditV2 } from './editv2.js';
+} from './editv2.js';
 
 function convertUpdate(edit: Update): EditV2 {
-  const attributes: Partial<Record<string, string | null>> = {};
-  const attributesNS: Partial<
-    Record<string, Partial<Record<string, string | null>>>
-  > = {};
+  const attributes: Attributes = {};
+  const attributesNS: AttributesNS = {};
 
   Object.entries(edit.attributes).forEach(([key, value]) => {
     if (isNamespaced(value!)) {

editv1.spec.ts

@@ -28,14 +28,11 @@ describe('type guard functions for editv1', () => {
 
   it('returns true for Remove', () => expect(remove).to.satisfy(isEdit));
 
-  it('returns false for SetAttributes', () =>
-    expect(setAttributes).to.not.satisfy(isEdit));
-
   it('returns true for SetTextContent', () =>
     expect(setTextContent).to.not.satisfy(isEdit));
 
   it('returns false on mixed edit and editV2 array', () =>
-    expect([update, setAttributes]).to.not.satisfy(isEdit));
+    expect([update, setTextContent]).to.not.satisfy(isEdit));
 
   it('returns true on edit array', () =>
     expect([update, remove, insert]).to.satisfy(isEdit));

editv1.ts

@@ -1,68 +1,67 @@
-import { isSetAttributes } from './editv2.js';
-
-/** Intent to `parent.insertBefore(node, reference)` */
-export type Insert = {
-  parent: Node;
-  node: Node;
-  reference: Node | null;
-};
+import { isInsert, isRemove, Insert, Remove } from './editv2.js';
 
 export type NamespacedAttributeValue = {
   value: string | null;
   namespaceURI: string | null;
 };
+
 export type AttributeValue = string | null | NamespacedAttributeValue;
+
+export type AttributesV1 = Partial<Record<string, AttributeValue>>;
+
 /** Intent to set or remove (if null) attributes on element */
 export type Update = {
   element: Element;
   attributes: Partial<Record<string, AttributeValue>>;
 };
 
-/** Intent to remove a node from its ownerDocument */
-export type Remove = {
-  node: Node;
-};
-
 /** Represents the user's intent to change an XMLDocument */
 export type Edit = Insert | Update | Remove | Edit[];
 
-export function isComplex(edit: Edit): edit is Edit[] {
-  return edit instanceof Array;
-}
-
-export function isInsert(edit: Edit): edit is Insert {
-  return (edit as Insert).parent !== undefined;
-}
-
 export function isNamespaced(
-  value: AttributeValue,
+  value: unknown,
 ): value is NamespacedAttributeValue {
-  return value !== null && typeof value !== 'string';
+  return (
+    value !== null &&
+    typeof value === 'object' &&
+    'namespaceURI' in value &&
+    typeof value.namespaceURI === 'string' &&
+    'value' in value &&
+    typeof value.value === 'string'
+  );
 }
 
-export function isUpdate(edit: Edit): edit is Update {
-  return (
-    (edit as Update).element !== undefined &&
-    (edit as Update).attributes !== undefined
+export function isAttributesV1(
+  attributes: unknown,
+): attributes is AttributesV1 {
+  if (attributes === null || typeof attributes !== 'object') {
+    return false;
+  }
+
+  return Object.entries(attributes).every(
+    ([key, value]) =>
+      typeof key === 'string' &&
+      (value === null || typeof value === 'string' || isNamespaced(value)),
   );
 }
 
-export function isRemove(edit: Edit): edit is Remove {
+export function isComplex(edit: unknown): edit is Edit[] {
+  return edit instanceof Array && edit.every(isEdit);
+}
+
+export function isUpdate(edit: unknown): edit is Update {
   return (
-    (edit as Insert).parent === undefined && (edit as Remove).node !== undefined
+    (edit as Update).element instanceof Element &&
+    isAttributesV1((edit as Update).attributes)
   );
 }
 
 export type EditEvent<E extends Edit = Edit> = CustomEvent<E>;
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export function isEdit(edit: any): edit is Edit {
+export function isEdit(edit: unknown): edit is Edit {
   if (isComplex(edit)) {
-    return !edit.some(e => !isEdit(e));
+    return true;
   }
 
-  return (
-    !isSetAttributes(edit) &&
-    (isUpdate(edit) || isInsert(edit) || isRemove(edit))
-  );
+  return isUpdate(edit) || isInsert(edit) || isRemove(edit);
 }

editv2.spec.ts

@@ -19,16 +19,16 @@ const insert: Insert = { parent: element, node: element, reference: null };
 const remove: Remove = { node: element };
 const setAttributes: SetAttributes = {
   element,
-  attributes: {},
-  attributesNS: {},
+  attributes: { name: 'value' },
+  attributesNS: { namespaceURI: { name: 'value' } },
 };
 const setTextContent: SetTextContent = { element, textContent: '' };
 
-describe('type guard functions for editv2', () => {
-  it('returns false on invalid Edit type', () =>
+describe('isEditV2', () => {
+  it('returns false for invalid Edit type', () =>
     expect('invalid edit').to.not.satisfy(isEditV2));
 
-  it('returns false on Update', () => expect(update).to.not.satisfy(isEditV2));
+  it('returns false for Update', () => expect(update).to.not.satisfy(isEditV2));
 
   it('returns true for Insert', () => expect(insert).to.satisfy(isEditV2));
 
@@ -40,13 +40,13 @@ describe('type guard functions for editv2', () => {
   it('returns true for SetTextContent', () =>
     expect(setTextContent).to.satisfy(isEditV2));
 
-  it('returns false on mixed edit and editV2 array', () =>
+  it('returns false for a mixed edit and editV2 array', () =>
     expect([update, setAttributes]).to.not.satisfy(isEditV2));
 
-  it('returns false on edit array', () =>
+  it('returns false for edit array', () =>
     expect([update, update]).to.not.satisfy(isEditV2));
 
-  it('returns true on editV2 array', () =>
+  it('returns true for editV2 array', () =>
     expect([setAttributes, remove, insert, setTextContent]).to.satisfy(
       isEditV2,
     ));

editv2.ts

@@ -16,6 +16,10 @@ export type SetTextContent = {
   textContent: string;
 };
 
+export type Attributes = Partial<Record<string, string | null>>;
+
+export type AttributesNS = Partial<Record<string, Attributes>>;
+
 /** Intent to set or remove (if `null`) `attributes`(-`NS`) on `element` */
 export type SetAttributes = {
   element: Element;
@@ -31,43 +35,69 @@ export type EditV2 =
   | Remove
   | EditV2[];
 
-export function isComplex(edit: EditV2): edit is EditV2[] {
-  return edit instanceof Array;
+export function isAttributes(attributes: unknown): attributes is Attributes {
+  if (typeof attributes !== 'object' || attributes === null) {
+    return false;
+  }
+  return Object.entries(attributes).every(
+    ([key, value]) =>
+      typeof key === 'string' && (value === null || typeof value === 'string'),
+  );
+}
+
+export function isAttributesNS(
+  attributesNS: unknown,
+): attributesNS is AttributesNS {
+  if (typeof attributesNS !== 'object' || attributesNS === null) {
+    return false;
+  }
+  return Object.entries(attributesNS).every(
+    ([namespace, attributes]) =>
+      typeof namespace === 'string' &&
+      isAttributes(attributes as Record<string, string | null>),
+  );
+}
+
+export function isComplex(edit: unknown): edit is EditV2[] {
+  return edit instanceof Array && edit.every(e => isEditV2(e));
 }
 
-export function isSetTextContent(edit: EditV2): edit is SetTextContent {
+export function isSetTextContent(edit: unknown): edit is SetTextContent {
   return (
-    (edit as SetTextContent).element !== undefined &&
-    (edit as SetTextContent).textContent !== undefined
+    (edit as SetTextContent).element instanceof Element &&
+    typeof (edit as SetTextContent).textContent === 'string'
   );
 }
 
-export function isRemove(edit: EditV2): edit is Remove {
+export function isRemove(edit: unknown): edit is Remove {
   return (
-    (edit as Insert).parent === undefined && (edit as Remove).node !== undefined
+    (edit as Insert).parent === undefined &&
+    (edit as Remove).node instanceof Node
   );
 }
 
-export function isSetAttributes(edit: EditV2): edit is SetAttributes {
+export function isSetAttributes(edit: unknown): edit is SetAttributes {
   return (
-    (edit as SetAttributes).element !== undefined &&
-    (edit as SetAttributes).attributes !== undefined &&
-    (edit as SetAttributes).attributesNS !== undefined
+    (edit as SetAttributes).element instanceof Element &&
+    isAttributes((edit as SetAttributes).attributes) &&
+    isAttributesNS((edit as SetAttributes).attributesNS)
   );
 }
 
-export function isInsert(edit: EditV2): edit is Insert {
+export function isInsert(edit: unknown): edit is Insert {
   return (
-    (edit as Insert).parent !== undefined &&
-    (edit as Insert).node !== undefined &&
-    (edit as Insert).reference !== undefined
+    ((edit as Insert).parent instanceof Element ||
+      (edit as Insert).parent instanceof Document ||
+      (edit as Insert).parent instanceof DocumentFragment) &&
+    (edit as Insert).node instanceof Node &&
+    ((edit as Insert).reference instanceof Node ||
+      (edit as Insert).reference === null)
   );
 }
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export function isEditV2(edit: any): edit is EditV2 {
+export function isEditV2(edit: unknown): edit is EditV2 {
   if (isComplex(edit)) {
-    return !edit.some(e => !isEditV2(e));
+    return true;
   }
 
   return (

package.json

@@ -86,6 +86,7 @@
     ],
     "rules": {
       "no-unused-vars": "off",
+      "no-use-before-define": "off",
       "class-methods-use-this": [
         "error",
         {