refactor: audit code

packages/plugin-manifest-validator/src/__tests__/check-required-properties.ts

@@ -0,0 +1,60 @@
+import assert from "assert";
+import checkRequiredProperties from "../check-required-properties";
+
+describe("checkRequiredProperties", () => {
+  it(`should return errors when missing the required properties`, () => {
+    const source: Record<string, any> = {
+      name: {
+        en: "名前",
+      },
+      description: {
+        en: "desc",
+      },
+      homepage_url: {},
+      icon: "image/icon.png",
+    };
+
+    const jsonSchema = {
+      items: [
+        {
+          homepage_url: {
+            properties: ["en"],
+          },
+        },
+        "icon",
+        "none-exist-property",
+      ],
+      warn: true,
+    };
+    assert.deepStrictEqual(checkRequiredProperties(source, jsonSchema), [
+      `Property "homepage_url.en" is missing.`,
+      `Property "none-exist-property" is missing.`,
+    ]);
+  });
+
+  it(`should return the correct error message when the "warn" setting is false`, () => {
+    const source: Record<string, any> = {
+      name: {
+        en: "名前",
+      },
+      description: {
+        en: "desc",
+      },
+      homepage_url: {},
+    };
+
+    const jsonSchema = {
+      items: [
+        {
+          homepage_url: {
+            properties: ["en"],
+          },
+        },
+      ],
+      warn: false,
+    };
+    assert.deepStrictEqual(checkRequiredProperties(source, jsonSchema), [
+      `Property "homepage_url.en" is required.`,
+    ]);
+  });
+});

packages/plugin-manifest-validator/src/__tests__/index.ts

@@ -1,7 +1,7 @@
 "use strict";
 
 import assert from "assert";
-import validator, { checkRequiredProperties } from "../index";
+import validator from "../index";
 
 // 20MB
 const MAX_FILE_SIZE = 20 * 1024 * 1024;
@@ -583,68 +583,6 @@ describe("validator", () => {
       },
     );
   });
-
-  describe("checkRequiredProperties", () => {
-    it(`should return errors when missing the required properties`, () => {
-      const source: Record<string, any> = {
-        name: {
-          en: "名前",
-        },
-        description: {
-          en: "desc",
-        },
-        homepage_url: {},
-        icon: "image/icon.png",
-      };
-
-      const jsonSchema = {
-        items: [
-          {
-            homepage_url: {
-              properties: ["en"],
-            },
-          },
-          "icon",
-          "none-exist-property",
-        ],
-        warn: true,
-      };
-      assert.deepStrictEqual(
-        checkRequiredProperties(json(source), jsonSchema),
-        [
-          `Property "homepage_url.en" is missing.`,
-          `Property "none-exist-property" is missing.`,
-        ],
-      );
-    });
-
-    it(`should return the correct error message when the "warn" setting is false`, () => {
-      const source: Record<string, any> = {
-        name: {
-          en: "名前",
-        },
-        description: {
-          en: "desc",
-        },
-        homepage_url: {},
-      };
-
-      const jsonSchema = {
-        items: [
-          {
-            homepage_url: {
-              properties: ["en"],
-            },
-          },
-        ],
-        warn: false,
-      };
-      assert.deepStrictEqual(
-        checkRequiredProperties(json(source), jsonSchema),
-        [`Property "homepage_url.en" is required.`],
-      );
-    });
-  });
 });
 
 /**

packages/plugin-manifest-validator/src/check-required-properties.ts

@@ -0,0 +1,44 @@
+import type { RequiredProperties } from "./index";
+
+const checkRequiredProperties = (
+  json: Record<string, any>,
+  schema: RequiredProperties,
+): string[] => {
+  if (!schema.items || schema.items.length === 0) {
+    return [];
+  }
+
+  const errors: string[] = [];
+  for (let i = 0; i < schema.items.length; i++) {
+    const item = schema.items[i];
+    if (typeof item === "object") {
+      for (const property in item) {
+        if (
+          !item[property].properties ||
+          item[property].properties.length === 0
+        ) {
+          continue;
+        }
+
+        item[property].properties.forEach((prop: string) => {
+          if (!json[property] || !json[property][prop]) {
+            errors.push(
+              generateErrorMessage(`${property}.${prop}`, schema.warn),
+            );
+          }
+        });
+      }
+    } else if (!json[item]) {
+      errors.push(generateErrorMessage(item, schema.warn));
+    }
+  }
+
+  return errors;
+};
+
+const generateErrorMessage = (
+  property: string,
+  warning: boolean = false,
+): string => `Property "${property}" is ${warning ? "missing" : "required"}.`;
+
+export default checkRequiredProperties;

packages/plugin-manifest-validator/src/index.ts

@@ -5,14 +5,15 @@ import Ajv from "ajv";
 import bytes from "bytes";
 import jsonSchema from "../manifest-schema.json";
 import validateUrl from "./validate-https-url";
+import checkRequiredProperties from "./check-required-properties";
 
 type ValidateResult = {
   valid: boolean | PromiseLike<any>;
   errors: null | ErrorObject[];
   warnings: null | string[];
 };
 
-type RequiredProperties = {
+export type RequiredProperties = {
   items: Array<{ [key: string]: { properties: string[] } } | string>;
   warn?: boolean;
 };
@@ -145,31 +146,26 @@ export default (
     schema: RequiredProperties,
     data: string,
   ) => {
-    if (
-      !schema ||
-      !schema.items ||
-      schema.items.length === 0 ||
-      !data ||
-      data.length === 0
-    ) {
+    if (!data || data.length === 0) {
       return true;
     }
 
     const errors = checkRequiredProperties(json, schema);
-    if (errors.length > 0) {
-      if (schema.warn) {
-        warnings.push(...errors.map((error) => error));
-      } else {
-        validateRequiredProperties.errors = errors.map((error) => ({
-          keyword: "requiredProperties",
-          message: error,
-        }));
-
-        return false;
-      }
+    if (errors.length === 0) {
+      return true;
     }
 
-    return true;
+    if (schema.warn) {
+      warnings.push(...errors.map((error) => error));
+      return true;
+    }
+
+    validateRequiredProperties.errors = errors.map((error) => ({
+      keyword: "requiredProperties",
+      message: error,
+    }));
+
+    return false;
   };
 
   ajv.addKeyword({
@@ -209,44 +205,3 @@ const transformErrors = (
   // shallow copy
   return errors.slice();
 };
-
-export const checkRequiredProperties = (
-  json: Record<string, any>,
-  schema: RequiredProperties,
-): string[] => {
-  if (!schema.items || schema.items.length === 0) {
-    return [];
-  }
-
-  const errors: string[] = [];
-  const generateErrorMessage = (
-    property: string,
-    warning: boolean = false,
-  ): string => `Property "${property}" is ${warning ? "missing" : "required"}.`;
-
-  for (let i = 0; i < schema.items.length; i++) {
-    const item = schema.items[i];
-    if (typeof item === "object") {
-      for (const property in item) {
-        if (
-          !item[property].properties ||
-          item[property].properties.length === 0
-        ) {
-          continue;
-        }
-
-        item[property].properties.forEach((prop: string) => {
-          if (!json[property] || !json[property][prop]) {
-            errors.push(
-              generateErrorMessage(`${property}.${prop}`, schema.warn),
-            );
-          }
-        });
-      }
-    } else if (!json[item]) {
-      errors.push(generateErrorMessage(item, schema.warn));
-    }
-  }
-
-  return errors;
-};