fix type error handler to handle allOf conflicting type

[srivastava-diya]

Add Conflicting Type Detection to type Error Handler

This update implements support for detecting conflicting type constraints entirely within the type error handler, as recommended. The previously skipped “ allOf conflicting type” test passes with this change in type error handler.

This fixes : #99

To keep the existing type logic isolated and safe, the changes made are as follows:

• Starting with a set of all allowed JSON types:

  let allowedTypes = new Set(["null", "boolean", "number", "string", "array", "object", "integer"]);

• now we intersect it with the type set from each type keyword found at the same instance location. If the resulting set becomes empty, that means the schema’s type constraints are mutually exclusive, so I return a single getConflictingTypeMessage(). If the intersection is not empty, then it isn’t a conflict and the normal type-error handling continues as usual.

• This keeps allOf functioning purely as a simple applicator while placing all conflict detection logic inside the type handler, as intended.

I’ve implemented this and run the full test suite — all tests pass.

[jdesrosiers]

I left a couple notes and I pushed some additional tests to expose some gaps we need to fill. There were two main issues. One is dealing with "integer" because it's not a true JSON type and has some overlap with "number". The other issue is about removing duplicate type messages. There should only ever be one per schema.

[jdesrosiers]

It's impossible for failedTypeLocations to be empty if allowedTypes is empty, right? If allowedTypes is empty, there must be at least two occurrences of type that don't overlap. That means one must fail.

[srivastava-diya]

You are absolutely right here. One must fail. I will refine that .

Regarding integer vs number, I wasn’t fully clear on how to treat them either. I saw the patternProperties test using type:"integer", so I included "integer" in the initial allowedTypes set, . I initially did not include integer in the set which caused this particulr test to fail so i included it,but I now understand the problem: "integer" isn’t a standalone JSON type, and it has a subtype relationship with "number" .I also saw your note about needing to eliminate duplicate type messages so I'll try to incorporate that .

[jdesrosiers]

Did you forget to remove this if, or did you come up with a reason why it needs to be there?

[jdesrosiers]

Let's declare this set of all possible types as a constant at the top. It doesn't need to be recreated every time the function runs.

[srivastava-diya]

Hey @jdesrosiers
I did not declare allowedTypes as a constant at the top because it is getting updated ,let me take an example.

• For Example

allOf: [
  { "type": ["string", "number"] },
  { "type": ["string"] },
  { "type": ["string", "boolean"] }
]

• Instance

instance = "helloo"

and we have declared and initialized allowedTypes with let allowedTypes = new Set(["null", "boolean", "number", "string", "array", "object", "integer"]); and in the mean while all the typeKeywords are also getting processed one by one , So suppose the 1st typeKeyword the got processed was temp_set = {string,number} (i'm putting them in a set so that i can take intersection). Now we take intersection of this temp_set with allowedTypes.

temp_Set={string,number}
allowedTypes = new Set(["null", "boolean", "number", "string", "array", "object", "integer"]);

//updated allowedTypes
allowedTypes = (temp_Set) intersection (allowedTypes)  //we get string and number so it becomes allowedTypes={string,number}

Now next typeKeyword gets processed i.e string so,

temp_Set={string}
allowedTypes={string,number}

allowedTypes = (temp_Set) intersection (allowedTypes) // becomes allowedTypes={string}

Now at last typeKeyword that gets processed will be string & boolean

temp_Set={string, boolean}
allowedTypes={string}

allowedTypes = (temp_Set) intersection (allowedTypes) // becomes allowedTypes={string}

and hence our instance which was a string passes. and allowedType cannot be taken as constant.

• Now had our allowedTypes set finally became empty like in this case:

allOf: [
  { "type": ["string"] },
  { "type": ["number"] },
]

That means we have a type Conflict.

• But if its final size is greater than 1 then no Type conflict and normal type error handling will do the work.

This was my thought process but it seems like I need to do some more work to get the logic right . Thanks a lot for pinpointing the areas i need to consider.

[jdesrosiers]

I understand. I was thinking something like this,

const allTypes = new Set(["null", "boolean", "number", "string", "array", "object", "integer"]);

const type = async (normalizedErrors, instance, localization) => {
  if (normalizedErrors["https://json-schema.org/keyword/type"]) {
    let allowedTypes = allTypes;

    for (const schemaLocation in normalizedErrors["https://json-schema.org/keyword/type"]) {
      allowedTypes = allowedTypes.intersection(keywordTypes);
    }

allowedTypes needs to be reassigned because it's accumulating the result of multiple intersections, but we don't need to recreate the initial value every time. The set intersection operation is immutable, so we can create that set once and reuse it for each initialization of allowedTypes.

[jdesrosiers]

You shouldn't need to write custom set intersection logic. Conceptually, if a schema allows numbers, it also allows integers. If you code for that relationship, the rest should all just work.

Suggested change

	const keywordTypes = new Set(types);
	allowedTypes = intersectTypeSets(allowedTypes, keywordTypes);
	const keywordTypes = new Set(types);
	if (keywordTypes.has("number")) {
	keywordTypes.add("integer");
	}
	allowedTypes = allowedTypes.intersection(keywordTypes);

[jdesrosiers]

This isn't a great place to put this code. It would be more cohesive to put it after the for loop that builds allowedTypes. That way all the code that builds allowedTypes is in one place. Then all the code that reads allowedTypes comes after.

[jdesrosiers]

There's no reason to create a new set every time you initialize allowedTypes. ALL_TYPES can be a set.

[jdesrosiers]

Did you forget to remove this if, or did you come up with a reason why it needs to be there?