Support for required property checking. All tests passing.
This commit is contained in:
@@ -11,12 +11,18 @@
|
||||
|
||||
package org.springframework.ide.vscode.commons.yaml.ast;
|
||||
|
||||
import java.util.Collections;
|
||||
import java.util.Set;
|
||||
|
||||
import org.yaml.snakeyaml.nodes.MappingNode;
|
||||
import org.yaml.snakeyaml.nodes.Node;
|
||||
import org.yaml.snakeyaml.nodes.NodeId;
|
||||
import org.yaml.snakeyaml.nodes.NodeTuple;
|
||||
import org.yaml.snakeyaml.nodes.ScalarNode;
|
||||
import org.yaml.snakeyaml.nodes.SequenceNode;
|
||||
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
|
||||
/**
|
||||
* @author Kris De Volder
|
||||
*/
|
||||
@@ -74,4 +80,22 @@ public class NodeUtil {
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the scalar values of all keys of the given {@link MappingNode} as Strings.
|
||||
* Any non-scalar keys are silently ignored.
|
||||
*/
|
||||
public static Set<String> getScalarKeys(MappingNode mapNode) {
|
||||
if (mapNode!=null) {
|
||||
ImmutableSet.Builder<String> builder = ImmutableSet.builder();
|
||||
for (NodeTuple entry : mapNode.getValue()) {
|
||||
String key = NodeUtil.asScalar(entry.getKeyNode());
|
||||
if (key!=null) { //key not a scalar? => something funky so skip it
|
||||
builder.add(key);
|
||||
}
|
||||
}
|
||||
return builder.build();
|
||||
}
|
||||
return Collections.emptySet();
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@@ -19,9 +19,12 @@ import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
import java.util.regex.Pattern;
|
||||
import java.util.stream.Collectors;
|
||||
import java.util.stream.Stream;
|
||||
|
||||
import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
|
||||
import org.springframework.ide.vscode.commons.languageserver.util.DocumentRegion;
|
||||
import org.springframework.ide.vscode.commons.util.CollectionUtil;
|
||||
import org.springframework.ide.vscode.commons.util.ExceptionUtil;
|
||||
import org.springframework.ide.vscode.commons.util.IntegerRange;
|
||||
import org.springframework.ide.vscode.commons.util.Log;
|
||||
@@ -61,14 +64,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
|
||||
public void reconcile(YamlFileAST ast) {
|
||||
List<Node> nodes = ast.getNodes();
|
||||
IntegerRange expectedDocs = schema.expectedNumberOfDocuments();
|
||||
if (expectedDocs.isInRange(nodes.size())) {
|
||||
if (nodes!=null && !nodes.isEmpty()) {
|
||||
for (int i = 0; i < nodes.size(); i++) {
|
||||
Node node = nodes.get(i);
|
||||
reconcile(ast.getDocument(), new YamlPath(YamlPathSegment.valueAt(i)), node, schema.getTopLevelType());
|
||||
}
|
||||
}
|
||||
} else {
|
||||
if (!expectedDocs.isInRange(nodes.size())) {
|
||||
//wrong number of documents in the file. Figure out a good error message.
|
||||
if (nodes.isEmpty()) {
|
||||
problem(allOf(ast.getDocument()), "'"+schema.getName()+"' must have at least some Yaml content");
|
||||
@@ -81,6 +77,12 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
|
||||
problem(endOf(ast.getDocument()), "'"+schema.getName()+"' should have at least "+lowerBound+" Yaml Documents");
|
||||
}
|
||||
}
|
||||
if (nodes!=null && !nodes.isEmpty()) {
|
||||
for (int i = 0; i < nodes.size(); i++) {
|
||||
Node node = nodes.get(i);
|
||||
reconcile(ast.getDocument(), new YamlPath(YamlPathSegment.valueAt(i)), node, schema.getTopLevelType());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private DocumentRegion dashesAtStartOf(YamlFileAST ast, Node node) {
|
||||
@@ -117,6 +119,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
|
||||
}
|
||||
} else if (typeUtil.isBean(type)) {
|
||||
Map<String, YTypedProperty> beanProperties = typeUtil.getPropertiesMap(type, schemaContext);
|
||||
checkRequiredProperties(map, type, beanProperties);
|
||||
for (NodeTuple entry : map.getValue()) {
|
||||
Node keyNode = entry.getKeyNode();
|
||||
String key = NodeUtil.asScalar(keyNode);
|
||||
@@ -170,6 +173,31 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
|
||||
}
|
||||
}
|
||||
|
||||
private void checkRequiredProperties(MappingNode map, YType type, Map<String, YTypedProperty> beanProperties) {
|
||||
Set<String> foundProps = NodeUtil.getScalarKeys(map);
|
||||
boolean allPropertiesKnown = beanProperties.keySet().containsAll(foundProps);
|
||||
//Don't check for missing properties if some properties look like they might be spelled incorrectly.
|
||||
if (allPropertiesKnown) {
|
||||
Set<String> missingProps = beanProperties.values().stream()
|
||||
.filter(YTypedProperty::isRequired)
|
||||
.map(YTypedProperty::getName)
|
||||
.filter((required) -> !foundProps.contains(required))
|
||||
.collect(Collectors.toSet());
|
||||
if (!missingProps.isEmpty()) {
|
||||
String message;
|
||||
if (missingProps.size()==1) {
|
||||
// slightly more specific message when only one missing property
|
||||
String missing = missingProps.stream().findFirst().get();
|
||||
message = "Property '"+missing+"' is required for '"+type+"'";
|
||||
} else {
|
||||
message = "Properties "+missingProps+" are required for '"+type+"'";
|
||||
}
|
||||
problem(map, message);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
protected NodeId getNodeId(Node node) {
|
||||
NodeId id = node.getNodeId();
|
||||
if (id==NodeId.mapping && isMoustacheVar(node)) {
|
||||
|
||||
@@ -50,24 +50,14 @@ public class ASTDynamicSchemaContext extends CachingSchemaContext {
|
||||
|
||||
@Override
|
||||
protected Set<String> computeDefinedProperties() {
|
||||
if (mapNode!=null) {
|
||||
ImmutableSet.Builder<String> builder = ImmutableSet.builder();
|
||||
for (NodeTuple entry : mapNode.getValue()) {
|
||||
String key = NodeUtil.asScalar(entry.getKeyNode());
|
||||
if (key!=null) { //key not a scalar? => something funky so skip it
|
||||
builder.add(key);
|
||||
}
|
||||
}
|
||||
return builder.build();
|
||||
}
|
||||
return Collections.emptySet();
|
||||
return NodeUtil.getScalarKeys(mapNode);
|
||||
}
|
||||
|
||||
@Override
|
||||
public IDocument getDocument() {
|
||||
return doc;
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public YamlPath getPath() {
|
||||
return path;
|
||||
|
||||
@@ -24,8 +24,6 @@ import java.util.Iterator;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.stream.Collectors;
|
||||
import java.util.stream.Stream;
|
||||
import java.util.stream.Stream.Builder;
|
||||
|
||||
import javax.swing.text.BadLocationException;
|
||||
|
||||
@@ -39,8 +37,6 @@ import org.eclipse.lsp4j.Range;
|
||||
import org.eclipse.lsp4j.TextEdit;
|
||||
import org.junit.Assert;
|
||||
|
||||
import com.google.common.base.Strings;
|
||||
|
||||
import reactor.core.publisher.Flux;
|
||||
|
||||
public class Editor {
|
||||
|
||||
@@ -10,7 +10,6 @@
|
||||
*******************************************************************************/
|
||||
package org.springframework.ide.vscode.manifest.yaml;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.mockito.Mockito.when;
|
||||
|
||||
@@ -132,6 +131,8 @@ public class ManifestYamlEditorTest {
|
||||
|
||||
//Using a 'composite' element where a scalar type is expected
|
||||
editor = harness.newEditor(
|
||||
"applications:\n" +
|
||||
"- name: foo\n" +
|
||||
"memory:\n"+
|
||||
"- bad sequence\n" +
|
||||
"buildpack:\n" +
|
||||
@@ -698,12 +699,16 @@ public class ManifestYamlEditorTest {
|
||||
"---\n" +
|
||||
"applications:\n"+
|
||||
"- name: foo\n" +
|
||||
" bad-one: xx\n" +
|
||||
"---\n" +
|
||||
"applications:\n"+
|
||||
"- name: foo\n"
|
||||
"- name: foo\n" +
|
||||
" bad-two: xx"
|
||||
);
|
||||
editor.assertProblems(
|
||||
"---|'Cloudfoundry Manifest' should not have more than 1 Yaml Document"
|
||||
"bad-one|Unknown property", //should still reconcile the documents even thought there's too many of them!
|
||||
"---|'Cloudfoundry Manifest' should not have more than 1 Yaml Document",
|
||||
"bad-two|Unknown property" //should still reconcile the documents even thought there's too many of them!
|
||||
);
|
||||
//also check the location of the marker since there are two occurrences of '---' in the editor text.
|
||||
Diagnostic problem = editor.assertProblem("---");
|
||||
@@ -737,12 +742,10 @@ public class ManifestYamlEditorTest {
|
||||
|
||||
//when the file is empty (there is no AST at all)
|
||||
editor = harness.newEditor(
|
||||
"foo: v1\n"
|
||||
"buildpack: some-buildpack"
|
||||
);
|
||||
editor.assertProblems(
|
||||
|
||||
"foo|Unkown property",
|
||||
"foo: v1|'applications' is required"
|
||||
"buildpack: some-buildpack|'applications' is required"
|
||||
);
|
||||
|
||||
}
|
||||
@@ -756,7 +759,7 @@ public class ManifestYamlEditorTest {
|
||||
);
|
||||
editor.assertProblems(
|
||||
"memory: 1G|Property 'name' is required",
|
||||
":|should not be empty"
|
||||
"|should not be empty"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user