Schema validation should check the object's oneOf if it has instead of the object type (#663)

* Schema validation should check the object's oneOf if it has instead of the object type

* reused existing method

Author: msivasubramaniaan

src/languageservice/parser/jsonParser07.ts

@@ -602,7 +602,8 @@ function validate(
   originalSchema: JSONSchema,
   validationResult: ValidationResult,
   matchingSchemas: ISchemaCollector,
-  options: Options
+  options: Options,
+  isAdditionalPropertiesCheck?: boolean
   // eslint-disable-next-line @typescript-eslint/no-explicit-any
 ): any {
   const { isKubernetes } = options;
@@ -646,6 +647,18 @@ function validate(
       return node.type === type || (type === 'integer' && node.type === 'number' && node.isInteger);
     }
 
+    function matchesSchemaType(schema: JSONSchema): boolean {
+      /* when schema type is object and it contains oneOf then needs to validate that particular types
+        reference issue 692
+      */
+      const type = Array.isArray(schema.type) ? undefined : schema.type;
+      if (type === 'object' && typeof schema.additionalProperties === 'object' && schema.additionalProperties.oneOf) {
+        return true;
+      } else if (type) {
+        return matchesType(type);
+      }
+    }
+
     if (Array.isArray(schema.type)) {
       if (!schema.type.some(matchesType)) {
         validationResult.problems.push({
@@ -659,7 +672,7 @@ function validate(
         });
       }
     } else if (schema.type) {
-      if (!matchesType(schema.type)) {
+      if (!matchesSchemaType(schema)) {
         //get more specific name than just object
         const schemaType = schema.type === 'object' ? getSchemaTypeName(schema) : schema.type;
         validationResult.problems.push({
@@ -678,6 +691,13 @@ function validate(
         validate(node, asSchema(subSchemaRef), schema, validationResult, matchingSchemas, options);
       }
     }
+    if (schema.additionalProperties && typeof schema.additionalProperties === 'object' && schema.additionalProperties.oneOf) {
+      const propertyValidationResult = new ValidationResult(isKubernetes);
+      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      validate(node, <any>schema.additionalProperties, schema, propertyValidationResult, matchingSchemas, options, true);
+      validationResult.mergePropertyMatch(propertyValidationResult);
+      validationResult.mergeEnumValues(propertyValidationResult);
+    }
     const notSchema = asSchema(schema.not);
     if (notSchema) {
       const subValidationResult = new ValidationResult(isKubernetes);
@@ -727,7 +747,14 @@ function validate(
         } else if (isKubernetes) {
           bestMatch = alternativeComparison(subValidationResult, bestMatch, subSchema, subMatchingSchemas);
         } else {
-          bestMatch = genericComparison(maxOneMatch, subValidationResult, bestMatch, subSchema, subMatchingSchemas);
+          bestMatch = genericComparison(
+            maxOneMatch,
+            subValidationResult,
+            bestMatch,
+            subSchema,
+            subMatchingSchemas,
+            isAdditionalPropertiesCheck
+          );
         }
       }
 
@@ -1429,7 +1456,8 @@ function validate(
       matchingSchemas: ISchemaCollector;
     },
     subSchema,
-    subMatchingSchemas
+    subMatchingSchemas,
+    isAdditionalPropertiesCheck?: boolean
   ): {
     schema: JSONSchema;
     validationResult: ValidationResult;
@@ -1442,7 +1470,7 @@ function validate(
       bestMatch.validationResult.propertiesValueMatches += subValidationResult.propertiesValueMatches;
     } else {
       const compareResult = subValidationResult.compareGeneric(bestMatch.validationResult);
-      if (compareResult > 0) {
+      if (compareResult > 0 || isAdditionalPropertiesCheck) {
         // our node is the best matching so far
         bestMatch = {
           schema: subSchema,

test/schemaValidation.test.ts

@@ -1557,18 +1557,58 @@ obj:
         expect(result.length).to.eq(0);
       });
     });
-  });
 
-  describe('Bug fixes', () => {
-    it('should handle not valid schema object', async () => {
-      const schema = 'Foo';
-      languageService.addSchema(SCHEMA_ID, schema as JSONSchema);
-      const content = `foo: bar`;
+    it('schema should validate additionalProp oneOf', async () => {
+      const schema = {
+        type: 'object',
+        definitions: {
+          expressionSyntax: {
+            type: 'string',
+            pattern: '^\\$\\{\\{\\s*fromJSON\\(.*\\)\\s*\\}\\}$',
+          },
+        },
+        properties: {
+          env: {
+            type: 'object',
+            additionalProperties: {
+              oneOf: [
+                {
+                  type: 'object',
+                  additionalProperties: {
+                    oneOf: [
+                      {
+                        type: 'string',
+                      },
+                      {
+                        type: 'number',
+                      },
+                      {
+                        type: 'boolean',
+                      },
+                    ],
+                  },
+                  minProperties: 1,
+                },
+                {
+                  $ref: '#/definitions/expressionSyntax',
+                },
+              ],
+            },
+          },
+        },
+      };
+      languageService.addSchema(SCHEMA_ID, schema);
+      const content = `env: matrix.env`;
       const result = await parseSetup(content);
-      expect(result).to.be.empty;
-      expect(telemetry.messages).to.be.empty;
+      expect(result.length).to.eq(1);
+      assert.deepStrictEqual(
+        result[0].message,
+        'String does not match the pattern of "^\\$\\{\\{\\s*fromJSON\\(.*\\)\\s*\\}\\}$".'
+      );
     });
+  });
 
+  describe('Bug fixes', () => {
     it('should handle bad schema refs', async () => {
       const schema = {
         type: 'object',