From cb7ebe7ebbf0766e27b6cd88b7ba3257c87d7355 Mon Sep 17 00:00:00 2001 From: Anton Lobov Date: Tue, 24 Jul 2018 14:15:52 +0200 Subject: [PATCH] WEB-33880 JSON Schemas: false positive 'Validates to more than one variant' warning --- .../impl/JsonSchemaAnnotatorChecker.java | 6 +- .../jsonSchema/impl/JsonSchemaObject.java | 33 +++--- .../jsonSchema/impl/JsonSchemaReader.java | 4 +- .../JsonSchemaHighlightingTest.java | 10 ++ .../highlighting/complexOneOfSchema.json | 106 ++++++++++++++++++ 5 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 json/tests/testData/jsonSchema/highlighting/complexOneOfSchema.json diff --git a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaAnnotatorChecker.java b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaAnnotatorChecker.java index cce4ce4ed796..2867d3143ae1 100644 --- a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaAnnotatorChecker.java +++ b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaAnnotatorChecker.java @@ -540,7 +540,7 @@ class JsonSchemaAnnotatorChecker { if (type != null) { list.add(type); } else { - final List variants = schema.getTypeVariants(); + final Set variants = schema.getTypeVariants(); if (variants != null) { list.addAll(variants); } @@ -575,7 +575,7 @@ class JsonSchemaAnnotatorChecker { } } if (schema.getTypeVariants() != null) { - List matchTypes = schema.getTypeVariants(); + Set matchTypes = schema.getTypeVariants(); if (matchTypes.contains(input)) { return input; } @@ -583,7 +583,7 @@ class JsonSchemaAnnotatorChecker { return input; } //nothing matches, lets return one of the list so that other heuristics does not match - return matchTypes.get(0); + return matchTypes.iterator().next(); } if (!schema.getProperties().isEmpty() && JsonSchemaType._object.equals(input)) return JsonSchemaType._object; return null; diff --git a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java index 2c4e115d0388..514ef7e9c3ee 100644 --- a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java +++ b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaObject.java @@ -10,14 +10,12 @@ import com.intellij.openapi.util.Pair; import com.intellij.openapi.util.text.StringUtil; import com.intellij.openapi.vfs.VirtualFile; import com.intellij.util.containers.ContainerUtil; +import com.intellij.util.containers.ContainerUtilRt; import org.jetbrains.annotations.NonNls; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.regex.Pattern; import java.util.regex.PatternSyntaxException; import java.util.stream.Collectors; @@ -50,7 +48,7 @@ public class JsonSchemaObject { @Nullable private Object myDefault; @Nullable private String myRef; @Nullable private String myFormat; - @Nullable private List myTypeVariants; + @Nullable private Set myTypeVariants; @Nullable private Number myMultipleOf; @Nullable private Number myMaximum; private boolean myExclusiveMaximum; @@ -123,7 +121,7 @@ public class JsonSchemaObject { if (other.myDefault != null) myDefault = other.myDefault; if (other.myRef != null) myRef = other.myRef; if (other.myFormat != null) myFormat = other.myFormat; - myTypeVariants = copyList(myTypeVariants, other.myTypeVariants); + myTypeVariants = copySet(myTypeVariants, other.myTypeVariants); if (other.myMultipleOf != null) myMultipleOf = other.myMultipleOf; if (other.myMaximum != null) myMaximum = other.myMaximum; if (other.myExclusiveMaximumNumber != null) myExclusiveMaximumNumber = other.myExclusiveMaximumNumber; @@ -186,7 +184,16 @@ public class JsonSchemaObject { @Nullable private static List copyList(@Nullable List target, @Nullable List source) { if (source == null || source.isEmpty()) return target; - if (target == null) target = new ArrayList<>(); + if (target == null) target = ContainerUtil.newArrayListWithCapacity(source.size()); + target.addAll(source); + 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; } @@ -194,7 +201,7 @@ public class JsonSchemaObject { @Nullable private static Map copyMap(@Nullable Map target, @Nullable Map source) { if (source == null || source.isEmpty()) return target; - if (target == null) target = new HashMap<>(); + if (target == null) target = ContainerUtilRt.newHashMap(source.size()); target.putAll(source); return target; } @@ -548,11 +555,11 @@ public class JsonSchemaObject { } @Nullable - public List getTypeVariants() { + public Set getTypeVariants() { return myTypeVariants; } - public void setTypeVariants(@Nullable List typeVariants) { + public void setTypeVariants(@Nullable Set typeVariants) { myTypeVariants = typeVariants; } @@ -782,7 +789,7 @@ public class JsonSchemaObject { JsonSchemaType type = getType(); if (type != null) return type.getDescription(); - List possibleTypes = getTypeVariants(); + Set possibleTypes = getTypeVariants(); String description = getTypesDescription(shortDesc, possibleTypes); if (description != null) return description; @@ -796,9 +803,9 @@ public class JsonSchemaObject { } @Nullable - static String getTypesDescription(boolean shortDesc, @Nullable List possibleTypes) { + static String getTypesDescription(boolean shortDesc, @Nullable Collection possibleTypes) { if (possibleTypes == null || possibleTypes.size() == 0) return null; - if (possibleTypes.size() == 1) return possibleTypes.get(0).getDescription(); + if (possibleTypes.size() == 1) return possibleTypes.iterator().next().getDescription(); if (possibleTypes.contains(JsonSchemaType._any)) return JsonSchemaType._any.getDescription(); Stream typeDescriptions = possibleTypes.stream().map(t -> t.getDescription()).distinct().sorted(); diff --git a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaReader.java b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaReader.java index 87ecabbd99a4..f34b3eb5a7c1 100644 --- a/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaReader.java +++ b/json/src/com/jetbrains/jsonSchema/impl/JsonSchemaReader.java @@ -270,9 +270,9 @@ public class JsonSchemaReader { final JsonSchemaType type = parseType(StringUtil.unquoteString(element.getText())); if (type != null) object.setType(type); } else if (element instanceof JsonArray) { - final List typeList = ((JsonArray)element).getValueList().stream() + final Set typeList = ((JsonArray)element).getValueList().stream() .filter(notEmptyString()).map(el -> parseType(StringUtil.unquoteString(el.getText()))) - .filter(el -> el != null).collect(Collectors.toList()); + .filter(el -> el != null).collect(Collectors.toSet()); if (!typeList.isEmpty()) object.setTypeVariants(typeList); } }; diff --git a/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaHighlightingTest.java b/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaHighlightingTest.java index 76b8ca1f9579..31e22b620f82 100644 --- a/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaHighlightingTest.java +++ b/json/tests/test/com/jetbrains/jsonSchema/JsonSchemaHighlightingTest.java @@ -866,6 +866,16 @@ public class JsonSchemaHighlightingTest extends JsonSchemaHighlightingTestBase { "] "); } + public void testComplexOneOfSchema() throws Exception { + @Language("JSON") String schemaText = FileUtil.loadFile(new File(getTestDataPath() + "/complexOneOfSchema.json")); + doTest(schemaText, "{\n" + + " \"indentation\": \"tab\"\n" + + " }"); + doTest(schemaText, "{\n" + + " \"indentation\": \"ttab\"\n" + + " }"); + } + public void testEnumCasing() throws Exception { @Language("JSON") String schema = "{\n" + " \"type\": \"object\",\n" + diff --git a/json/tests/testData/jsonSchema/highlighting/complexOneOfSchema.json b/json/tests/testData/jsonSchema/highlighting/complexOneOfSchema.json new file mode 100644 index 000000000000..2df31ee714f2 --- /dev/null +++ b/json/tests/testData/jsonSchema/highlighting/complexOneOfSchema.json @@ -0,0 +1,106 @@ +{ + "properties": { + "indentation": { + "description": "Specify indentation", + "type": [ + "null", + "integer", + "string", + "array" + ], + "oneOf": [ + { + "type": [ + "null", + "integer" + ] + }, + { + "type": "string", + "enum": [ + "tab", + [] + ] + }, + { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "integer" + } + }, + { + "type": "array", + "minItems": 2, + "maxItems": 2, + "uniqueItems": true, + "items": { + "type": [ + "integer", + "string", + "object" + ], + "anyOf": [ + { + "type": "integer" + }, + { + "type": "string", + "enum": [ + "tab", + {} + ] + }, + { + "type": "object", + "allOf": [ + { + "$ref": "#/definitions/coreRule" + } + ], + "properties": { + "indentInsideParens": { + "description": "If `true`, the closing brace of a block (rule or at-rule) will be expected at the same indentation level as the block's inner nodes", + "type": "string", + "enum": [ + "twice", + "once-at-root-twice-in-block" + ] + }, + "except": { + "description": "Do not indent for these things", + "type": "array", + "uniqueItems": true, + "minItems": 1, + "items": { + "type": "string", + "enum": [ + "block", + "param", + "value" + ] + } + }, + "ignore": { + "description": "Ignore the indentation inside parentheses", + "type": "array", + "uniqueItems": true, + "minItems": 1, + "items": { + "type": "string", + "enum": [ + "inside-parens", + "param", + "value" + ] + } + } + } + } + ] + } + } + ] + } +}