From 39bc5a4966b8536d50e4c89ff0870fadbc831d2c Mon Sep 17 00:00:00 2001 From: Anton Lobov Date: Wed, 19 Sep 2018 12:48:58 +0200 Subject: [PATCH] WEB-34888 Validation of 'allOf' condition is too weak --- .../jsonSchema/impl/JsonSchemaObject.java | 106 ++++++++++++++++-- .../impl/JsonSchemaVariantsTreeBuilder.java | 6 +- .../JsonSchemaReSharperHighlightingTest.java | 2 +- .../schemaFile/JsonSchemaTestSuite.java | 1 + .../resharper/Test012_2.json.gold | 10 -- .../highlighting/resharper/test012.json | 2 +- .../highlighting/resharper/test012_2.json | 2 +- 7 files changed, 103 insertions(+), 26 deletions(-) delete mode 100644 json/tests/testData/jsonSchema/highlighting/resharper/Test012_2.json.gold diff --git a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java index 4c2aac40b25f..7733dd2c7ff7 100644 --- a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java +++ b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java @@ -2,6 +2,7 @@ package com.jetbrains.jsonSchema.impl; import com.google.gson.Gson; +import com.google.gson.JsonObject; import com.google.gson.JsonParseException; import com.intellij.json.psi.JsonContainer; import com.intellij.openapi.diagnostic.Logger; @@ -107,6 +108,12 @@ public class JsonSchemaObject { @Nullable private JsonSchemaObject myElse; private boolean myShouldValidateAgainstJSType; + public boolean isValidByExclusion() { + return myIsValidByExclusion; + } + + private boolean myIsValidByExclusion = true; + public JsonSchemaObject(@NotNull JsonContainer object) { myJsonObject = object; myProperties = new HashMap<>(); @@ -117,6 +124,89 @@ public class JsonSchemaObject { myProperties = new HashMap<>(); } + @Nullable + private static JsonSchemaType getSubtypeOfBoth(@NotNull JsonSchemaType selfType, + @NotNull JsonSchemaType otherType) { + if (otherType == JsonSchemaType._any) return selfType; + if (selfType == JsonSchemaType._any) return otherType; + //noinspection EnumSwitchStatementWhichMissesCases + switch (selfType) { + case _string: + return otherType == JsonSchemaType._string || otherType == JsonSchemaType._string_number ? JsonSchemaType._string : null; + case _number: + if (otherType == JsonSchemaType._integer) return JsonSchemaType._integer; + return otherType == JsonSchemaType._number || otherType == JsonSchemaType._string_number ? JsonSchemaType._number : null; + case _integer: + return otherType == JsonSchemaType._number + || otherType == JsonSchemaType._string_number + || otherType == JsonSchemaType._integer ? JsonSchemaType._integer : null; + case _object: + return otherType == JsonSchemaType._object ? JsonSchemaType._object : null; + case _array: + return otherType == JsonSchemaType._array ? JsonSchemaType._array : null; + case _boolean: + return otherType == JsonSchemaType._boolean ? JsonSchemaType._boolean : null; + case _null: + return otherType == JsonSchemaType._null ? JsonSchemaType._null : null; + case _string_number: + return otherType == JsonSchemaType._integer + || otherType == JsonSchemaType._number + || otherType == JsonSchemaType._string + || otherType == JsonSchemaType._string_number ? otherType : null; + } + return otherType; + } + + @Nullable + private JsonSchemaType mergeTypes(@Nullable JsonSchemaType selfType, + @Nullable JsonSchemaType otherType, + @Nullable Set otherTypeVariants) { + if (selfType == null) return otherType; + if (otherType == null) { + if (otherTypeVariants != null && !otherTypeVariants.isEmpty()) { + Set filteredVariants = ContainerUtil.newHashSet(otherTypeVariants.size()); + for (JsonSchemaType variant : otherTypeVariants) { + JsonSchemaType subtype = getSubtypeOfBoth(selfType, variant); + if (subtype != null) filteredVariants.add(subtype); + } + if (filteredVariants.size() == 0) { + myIsValidByExclusion = false; + return selfType; + } + if (filteredVariants.size() == 1) { + return filteredVariants.iterator().next(); + } + return null; // will be handled by variants + } + return selfType; + } + + JsonSchemaType subtypeOfBoth = getSubtypeOfBoth(selfType, otherType); + if (subtypeOfBoth == null){ + myIsValidByExclusion = false; + return otherType; + } + return subtypeOfBoth; + } + + private Set mergeTypeVariantSets(@Nullable Set self, @Nullable Set other) { + if (self == null) return other; + if (other == null) return self; + + Set resultSet = ContainerUtil.newHashSet(self.size()); + for (JsonSchemaType type : self) { + JsonSchemaType merged = mergeTypes(type, null, other); + if (merged != null) resultSet.add(merged); + } + + if (resultSet.isEmpty()) { + myIsValidByExclusion = false; + return other; + } + + return resultSet; + } + // peer pointer is not merged! public void mergeValues(@NotNull JsonSchemaObject other) { // we do not copy id, schema @@ -136,11 +226,12 @@ public class JsonSchemaObject { myHtmlDescription = other.myHtmlDescription; } - if (other.myType != null) myType = other.myType; + myType = mergeTypes(myType, other.myType, other.myTypeVariants); + if (other.myDefault != null) myDefault = other.myDefault; if (other.myRef != null) myRef = other.myRef; if (other.myFormat != null) myFormat = other.myFormat; - myTypeVariants = copySet(myTypeVariants, other.myTypeVariants); + myTypeVariants = mergeTypeVariantSets(myTypeVariants, other.myTypeVariants); if (other.myMultipleOf != null) myMultipleOf = other.myMultipleOf; if (other.myMaximum != null) myMaximum = other.myMaximum; if (other.myExclusiveMaximumNumber != null) myExclusiveMaximumNumber = other.myExclusiveMaximumNumber; @@ -208,15 +299,6 @@ public class JsonSchemaObject { return target; } - @Nullable - private static Set copySet(@Nullable Set target, @Nullable Set source) { - if (source == null || source.isEmpty()) return target; - if (target != null && source.containsAll(target)) return target; - if (target == null) target = ContainerUtil.newHashSet(source.size()); - target.addAll(source); - return target; - } - @Nullable private static Map copyMap(@Nullable Map target, @Nullable Map source) { if (source == null || source.isEmpty()) return target; @@ -660,7 +742,7 @@ public class JsonSchemaObject { private static String unescapeJsonString(@NotNull final String text) { try { final String object = String.format("{\"prop\": \"%s\"}", text); - return new Gson().fromJson(object, com.google.gson.JsonObject.class).get("prop").getAsString(); + return new Gson().fromJson(object, JsonObject.class).get("prop").getAsString(); } catch (JsonParseException e) { return text; } diff --git a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaVariantsTreeBuilder.java b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaVariantsTreeBuilder.java index 9035f1ae107c..921d486d175b 100644 --- a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaVariantsTreeBuilder.java +++ b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaVariantsTreeBuilder.java @@ -308,7 +308,10 @@ public class JsonSchemaVariantsTreeBuilder { private static List andGroup(@NotNull JsonSchemaObject object, @NotNull List group) { List list = ContainerUtil.newArrayListWithCapacity(group.size()); for (JsonSchemaObject s: group) { - list.add(merge(object, s, s)); + JsonSchemaObject schemaObject = merge(object, s, s); + if (schemaObject.isValidByExclusion()) { + list.add(schemaObject); + } } return list; } @@ -369,6 +372,7 @@ public class JsonSchemaVariantsTreeBuilder { } } + @NotNull public static JsonSchemaObject merge(@NotNull JsonSchemaObject base, @NotNull JsonSchemaObject other, @NotNull JsonSchemaObject pointTo) { diff --git a/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaReSharperHighlightingTest.java b/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaReSharperHighlightingTest.java index 8c65849c1ba7..b912f9fc07c3 100644 --- a/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaReSharperHighlightingTest.java +++ b/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaReSharperHighlightingTest.java @@ -99,7 +99,7 @@ public class JsonSchemaReSharperHighlightingTest extends JsonSchemaHighlightingT public void test012() throws Exception { doTestFiles("test012", "schema012"); } - public void _test012_2() throws Exception { // TODO bug + public void test012_2() throws Exception { doTestFiles("test012_2", "schema012"); } public void test012_3() throws Exception { diff --git a/json/tests/test/com/jetbrains/jsonSchema/schemaFile/JsonSchemaTestSuite.java b/json/tests/test/com/jetbrains/jsonSchema/schemaFile/JsonSchemaTestSuite.java index 59b20ad40793..cc603a6b4fa7 100644 --- a/json/tests/test/com/jetbrains/jsonSchema/schemaFile/JsonSchemaTestSuite.java +++ b/json/tests/test/com/jetbrains/jsonSchema/schemaFile/JsonSchemaTestSuite.java @@ -34,6 +34,7 @@ public class JsonSchemaTestSuite extends TestCase { suite.addTestSuite(JsonSchemaCrossReferencesTest.class); suite.addTestSuite(JsonSchemaDocumentationTest.class); suite.addTestSuite(JsonSchemaHighlightingTest.class); + suite.addTestSuite(JsonSchemaReSharperHighlightingTest.class); suite.addTestSuite(JsonSchemaPatternComparatorTest.class); suite.addTestSuite(JsonSchemaSelfHighligthingTest.class); suite.addTestSuite(JsonBySchemaCompletionTest.class); diff --git a/json/tests/testData/jsonSchema/highlighting/resharper/Test012_2.json.gold b/json/tests/testData/jsonSchema/highlighting/resharper/Test012_2.json.gold deleted file mode 100644 index 5133c8fb3b71..000000000000 --- a/json/tests/testData/jsonSchema/highlighting/resharper/Test012_2.json.gold +++ /dev/null @@ -1,10 +0,0 @@ -{ - - "a": |true|(0) -} - - ---------------------------------------------------------- -(0): ReSharper Underlined Error Highlighting: JSON validation failed: Should satisfy at least one of constraints: - Expression must be of type 'string' - Expression must be of type 'number' diff --git a/json/tests/testData/jsonSchema/highlighting/resharper/test012.json b/json/tests/testData/jsonSchema/highlighting/resharper/test012.json index 83376fac6642..e83c3941e520 100644 --- a/json/tests/testData/jsonSchema/highlighting/resharper/test012.json +++ b/json/tests/testData/jsonSchema/highlighting/resharper/test012.json @@ -1,5 +1,5 @@ { - "a": 5 + "a": 5 } diff --git a/json/tests/testData/jsonSchema/highlighting/resharper/test012_2.json b/json/tests/testData/jsonSchema/highlighting/resharper/test012_2.json index 4ed24bc40c43..8f1ab339d494 100644 --- a/json/tests/testData/jsonSchema/highlighting/resharper/test012_2.json +++ b/json/tests/testData/jsonSchema/highlighting/resharper/test012_2.json @@ -1,5 +1,5 @@ { - "a": true + "a": true }