Combine required and dependentRequired error messages (#160)

* combined required and dependentRequired error messages

* unified error handler for each keyword

* code style is fixed

* Update code style

* Schema AST function is now used in all interations

* clean up is done

* helper function is positioned at bottom now

* Some cleanup

---------

Co-authored-by: Jason Desrosiers <jdesrosi@gmail.com>

Author: Sam-61s

src/error-handlers/dependentRequired.js

@@ -1,6 +1,3 @@
-import { getSchema } from "@hyperjump/json-schema/experimental";
-import * as Schema from "@hyperjump/browser";
-import * as Instance from "@hyperjump/json-schema/instance/experimental";
 import { getErrors } from "../../json-schema-errors.js";
 
 /**
@@ -22,25 +19,6 @@ const dependenciesErrorHandler = async (normalizedErrors, instance, localization
       const dependentSchemaErrors = await getErrors(dependentSchemaOutput, instance, localization);
       errors.push(...dependentSchemaErrors);
     }
-
-    const dependencies = await getSchema(schemaLocation);
-    for await (const [propertyName, dependency] of Schema.entries(dependencies)) {
-      if (!Instance.has(propertyName, instance)) {
-        continue;
-      }
-
-      if (Schema.typeOf(dependency) !== "array") {
-        continue;
-      }
-
-      const dependentRequired = /** @type {string[]} */ (Schema.value(dependency));
-      const missing = dependentRequired.filter((required) => !Instance.has(required, instance));
-      errors.push({
-        message: localization.getRequiredErrorMessage(missing),
-        instanceLocation: Instance.uri(instance),
-        schemaLocations: [schemaLocation]
-      });
-    }
   }
 
   return errors;

src/error-handlers/draft-04/dependencies.js

@@ -3,37 +3,87 @@ import * as Schema from "@hyperjump/browser";
 import * as Instance from "@hyperjump/json-schema/instance/experimental";
 
 /**
- * @import { ErrorHandler, ErrorObject } from "../index.d.ts"
+ * @import { ErrorHandler } from "../index.d.ts"
+ * @import { JsonNode } from "@hyperjump/json-schema/instance/experimental"
  */
 
 /** @type ErrorHandler */
 const requiredErrorHandler = async (normalizedErrors, instance, localization) => {
-  /** @type ErrorObject[] */
-  const errors = [];
+  /** @type {Set<string>} */
+  const allMissingRequired = new Set();
+  const allSchemaLocations = [];
 
   for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/required"]) {
     if (normalizedErrors["https://json-schema.org/keyword/required"][schemaLocation]) {
       continue;
     }
 
+    allSchemaLocations.push(schemaLocation);
     const keyword = await getSchema(schemaLocation);
     const required = /** @type string[] */ (Schema.value(keyword));
 
-    const missingRequired = [];
-    for (const propertyName of required) {
+    addMissingProperties(required, instance, allMissingRequired);
+  }
+
+  for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/dependentRequired"]) {
+    if (normalizedErrors["https://json-schema.org/keyword/dependentRequired"][schemaLocation]) {
+      continue;
+    }
+
+    allSchemaLocations.push(schemaLocation);
+    const keyword = await getSchema(schemaLocation);
+
+    for await (const [propertyName, dependencyNode] of Schema.entries(keyword)) {
       if (!Instance.has(propertyName, instance)) {
-        missingRequired.push(propertyName);
+        continue;
       }
+
+      const requiredProperties = /** @type string[] */ (Schema.value(dependencyNode));
+      addMissingProperties(requiredProperties, instance, allMissingRequired);
+    }
+  }
+
+  for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/draft-04/dependencies"]) {
+    if (typeof normalizedErrors["https://json-schema.org/keyword/draft-04/dependencies"][schemaLocation] === "boolean") {
+      continue;
     }
 
-    errors.push({
-      message: localization.getRequiredErrorMessage(missingRequired),
-      instanceLocation: Instance.uri(instance),
-      schemaLocations: [schemaLocation]
-    });
+    const keyword = await getSchema(schemaLocation);
+
+    let hasArrayFormDependencies = false;
+    for await (const [propertyName, dependency] of Schema.entries(keyword)) {
+      if (!Instance.has(propertyName, instance) || Schema.typeOf(dependency) !== "array") {
+        continue;
+      }
+
+      hasArrayFormDependencies = true;
+      const dependencyArray = /** @type {string[]} */ (Schema.value(dependency));
+      addMissingProperties(dependencyArray, instance, allMissingRequired);
+    }
+
+    if (hasArrayFormDependencies) {
+      allSchemaLocations.push(schemaLocation);
+    }
   }
 
-  return errors;
+  if (allMissingRequired.size === 0) {
+    return [];
+  }
+
+  return [{
+    message: localization.getRequiredErrorMessage([...allMissingRequired]),
+    instanceLocation: Instance.uri(instance),
+    schemaLocations: /** @type {string[]} */ ([...allSchemaLocations])
+  }];
+};
+
+/** @type (requiredProperties: string[], instance: JsonNode, missingSet: Set<string>) => void */
+const addMissingProperties = (requiredProperties, instance, missingSet) => {
+  for (const propertyName of requiredProperties) {
+    if (!Instance.has(propertyName, instance)) {
+      missingSet.add(propertyName);
+    }
+  }
 };
 
 export default requiredErrorHandler;

src/error-handlers/required.js

@@ -58,7 +58,6 @@ import booleanSchemaErrorHandler from "./error-handlers/boolean-schema.js";
 import constErrorHandler from "./error-handlers/const.js";
 import containsErrorHandler from "./error-handlers/contains.js";
 import dependenciesErrorHandler from "./error-handlers/draft-04/dependencies.js";
-import dependentRequiredErrorHandler from "./error-handlers/dependentRequired.js";
 import enumErrorHandler from "./error-handlers/enum.js";
 import exclusiveMaximumErrorHandler from "./error-handlers/exclusiveMaximum.js";
 import exclusiveMinimumErrorHandler from "./error-handlers/exclusiveMinimum.js";
@@ -144,7 +143,6 @@ addErrorHandler(booleanSchemaErrorHandler);
 addErrorHandler(constErrorHandler);
 addErrorHandler(containsErrorHandler);
 addErrorHandler(dependenciesErrorHandler);
-addErrorHandler(dependentRequiredErrorHandler);
 addErrorHandler(enumErrorHandler);
 addErrorHandler(exclusiveMaximumErrorHandler);
 addErrorHandler(exclusiveMinimumErrorHandler);

src/index.js

@@ -47,17 +47,8 @@
         {
           "messageId": "required-message",
           "messageParams": {
-            "count": 1,
-            "required": { "and": ["baz"] }
-          },
-          "instanceLocation": "#",
-          "schemaLocations": ["#/dependencies"]
-        },
-        {
-          "messageId": "required-message",
-          "messageParams": {
-            "count": 1,
-            "required": { "and": ["bbb"] }
+            "count": 2,
+            "required": { "and": ["baz", "bbb"] }
           },
           "instanceLocation": "#",
           "schemaLocations": ["#/dependencies"]
@@ -110,20 +101,14 @@
         {
           "messageId": "required-message",
           "messageParams": {
-            "count": 1,
-            "required": { "and": ["baz"] }
-          },
-          "instanceLocation": "#",
-          "schemaLocations": ["#/dependentSchemas/foo/required"]
-        },
-        {
-          "messageId": "required-message",
-          "messageParams": {
-            "count": 1,
-            "required": { "and": ["bbb"] }
+            "count": 2,
+            "required": { "and": ["baz", "bbb"] }
           },
           "instanceLocation": "#",
-          "schemaLocations": ["#/dependentSchemas/aaa/required"]
+          "schemaLocations": [
+            "#/dependentSchemas/foo/required",
+            "#/dependentSchemas/aaa/required"
+          ]
         }
       ]
     },

src/test-suite/tests/dependencies.json

@@ -14,13 +14,38 @@
           "messageId": "required-message",
           "messageParams": {
             "count": 1,
-            "required": { "or": ["bar"] }
+            "required": { "and": ["bar"] }
           },
           "instanceLocation": "#",
           "schemaLocations": ["#/required"]
         }
       ]
     },
+    {
+      "description": "combined required and dependentRequired",
+      "compatibility": "2019",
+      "schema": {
+        "required": ["foo", "bar"],
+        "dependentRequired": {
+          "a": ["b"]
+        }
+      },
+      "instance": { "a": 1 },
+      "errors": [
+        {
+          "messageId": "required-message",
+          "messageParams": {
+            "count": 3,
+            "required": { "and": ["foo", "bar", "b"] }
+          },
+          "instanceLocation": "#",
+          "schemaLocations": [
+            "#/required",
+            "#/dependentRequired"
+          ]
+        }
+      ]
+    },
     {
       "description": "required pass",
       "schema": {