Additional improvements for PT-140709005

For missing properties in nodes in a sequence, try to underline the corresponding '-' in the input associated with the sequence node,  instead of
underlining the entire node that is missing the property.
This commit is contained in:
Kris De Volder
2017-04-03 14:33:12 -07:00
parent 88013ef803
commit f96495e3ef
7 changed files with 85 additions and 30 deletions

View File

@@ -266,7 +266,21 @@ public class DocumentRegion implements CharSequence {
}
public int getLength() {
return getEnd() - getStart();
return length();
}
public boolean endsWith(CharSequence string) {
int myLen = length();
int strLen = string.length();
if (myLen>=strLen) {
for (int i = 0; i < strLen; i++) {
if (charAt(myLen-strLen+i)!=string.charAt(i)) {
return false;
}
}
return true;
}
return false;
}
}

View File

@@ -234,14 +234,14 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
} else {
message = "Properties "+missingProps+" are required for '"+type+"'";
}
problems.accept(YamlSchemaProblems.missingProperty(message, parent, map));
problems.accept(YamlSchemaProblems.missingProperty(message, dc.getDocument(), parent, map));
}
//Check for other constraints attached to the type
for (SchemaContextAware<Constraint> _constraint : typeUtil.getConstraints(type)) {
Constraint constraint = _constraint.withContext(dc);
if (constraint!=null) {
constraint.verify(parent, map, type, foundProps, problems);
constraint.verify(dc.getDocument(), parent, map, type, foundProps, problems);
}
}
}

View File

@@ -17,15 +17,18 @@ import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemTy
import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblem;
import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblemImpl;
import org.springframework.ide.vscode.commons.languageserver.util.DocumentRegion;
import org.springframework.ide.vscode.commons.util.text.IDocument;
import org.springframework.ide.vscode.commons.yaml.ast.NodeUtil;
import org.springframework.ide.vscode.commons.yaml.path.NodeCursor;
import org.springframework.ide.vscode.commons.yaml.path.YamlPath;
import org.springframework.ide.vscode.commons.yaml.path.YamlPathSegment;
import org.springframework.ide.vscode.commons.yaml.schema.YType;
import org.springframework.ide.vscode.commons.yaml.schema.YTypedProperty;
import org.yaml.snakeyaml.error.Mark;
import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node;
import org.yaml.snakeyaml.nodes.NodeTuple;
import org.yaml.snakeyaml.nodes.SequenceNode;
import com.google.common.collect.ImmutableSet;
@@ -90,19 +93,36 @@ public class YamlSchemaProblems {
return deprecatedProperty("Property '"+property.getName()+"' of '"+bean+"' is Deprecated", node);
}
public static ReconcileProblem problem(ProblemType problemType, String msg, DocumentRegion node) {
int start = node.getStart();
int end = node.getEnd();
return new ReconcileProblemImpl(problemType, msg, start, end-start);
}
public static ReconcileProblem problem(ProblemType problemType, String msg, Node node) {
int start = node.getStartMark().getIndex();
int end = node.getEndMark().getIndex();
return new ReconcileProblemImpl(problemType, msg, start, end-start);
}
public static ReconcileProblem missingProperty(String msg, Node parent, MappingNode map) {
public static ReconcileProblem missingProperty(String msg, IDocument doc, Node parent, MappingNode map) {
if (parent instanceof MappingNode) {
for (NodeTuple prop : ((MappingNode) parent).getValue()) {
if (prop.getValueNode()==map) {
return problem(MISSING_PROPERTY, msg, prop.getKeyNode());
}
}
} else if (parent instanceof SequenceNode) {
Boolean flowStyle = ((SequenceNode) parent).getFlowStyle();
if (flowStyle!=null && !flowStyle) {
Mark nodeStart = map.getStartMark();
DocumentRegion underline = new DocumentRegion(doc, 0, nodeStart.getIndex());
underline = underline.trimEnd();
if (underline.endsWith("-")) {
underline = underline.subSequence(underline.length()-1, underline.length());
return problem(MISSING_PROPERTY, msg, underline);
}
}
}
return problem(MISSING_PROPERTY, msg, map);
}

View File

@@ -13,6 +13,7 @@ package org.springframework.ide.vscode.commons.yaml.schema.constraints;
import java.util.Set;
import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
import org.springframework.ide.vscode.commons.util.text.IDocument;
import org.springframework.ide.vscode.commons.yaml.schema.YType;
import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node;
@@ -36,6 +37,7 @@ public interface Constraint {
* @param foundProps The properties this node defines.
* @param problems Problem collector where to which the constraint should add the validation problems it finds.
*/
void verify(Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems);
void verify(IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps,
IProblemCollector problems);
}

View File

@@ -21,6 +21,7 @@ import java.util.function.Function;
import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
import org.springframework.ide.vscode.commons.util.Assert;
import org.springframework.ide.vscode.commons.util.text.IDocument;
import org.springframework.ide.vscode.commons.yaml.ast.NodeUtil;
import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems;
import org.springframework.ide.vscode.commons.yaml.schema.YType;
@@ -61,7 +62,7 @@ public class Constraints {
}
@Override
public void verify(Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) {
public void verify(IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) {
List<String> requiredProps = Arrays.asList(_requiredProps);
long foundPropsCount = requiredProps.stream()
.filter(foundProps::contains)
@@ -69,7 +70,7 @@ public class Constraints {
if (foundPropsCount==0) {
if (!allowFewer) {
problems.accept(missingProperty(
"One of "+requiredProps+" is required for '"+type+"'", parent, map));
"One of "+requiredProps+" is required for '"+type+"'", doc, parent, map));
}
} else if (foundPropsCount>1) {
//Mark each of the found keys as a violation:
@@ -86,7 +87,7 @@ public class Constraints {
public static Constraint deprecated(Function<String, String> messageFormatter, String... _deprecatedNames) {
Set<String> deprecatedNames = ImmutableSet.copyOf(_deprecatedNames);
return (Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) -> {
return (IDocument doc, Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) -> {
for (NodeTuple prop : map.getValue()) {
Node keyNode = prop.getKeyNode();
String name = NodeUtil.asScalar(keyNode);

View File

@@ -112,10 +112,14 @@ public class Editor {
/**
* Check that a 'expectedProblems' are found by the reconciler. Expected problems are
* specified by string of the form "${badSnippet}|${messageSnippet}". The badSnippet
* is the text expected to be covered by the marker's region and the message snippet must
* specified by string of the form "${badSnippet}|${messageSnippet}" or
* "${badSnippet}^${followSnippet}|${messageSnippet}"
* <p>
* The badSnippet is the text expected to be covered by the marker's region and the message snippet must
* be found in the error marker's message.
* <p>
* In addition, if followSnippet is specified, the text that comes right after the error marker must match it.
* <p>
* The expected problems are matched one-to-one in the order given (so markers in the
* editor must appear in the expected order for the assert to pass).
*
@@ -206,6 +210,12 @@ public class Editor {
String[] parts = expect.split("\\|");
assertEquals(2, parts.length);
String badSnippet = parts[0];
String snippetFollow = null;
int carretOffset = badSnippet.indexOf('^');
if (carretOffset>=0) {
snippetFollow = badSnippet.substring(carretOffset+1);
badSnippet = badSnippet.substring(0, carretOffset);
}
String messageSnippet = parts[1];
boolean spaceSensitive = badSnippet.trim().length()<badSnippet.length();
boolean emptyRange = problem.getRange().getStart().equals(problem.getRange().getEnd());
@@ -216,9 +226,17 @@ public class Editor {
actualBadSnippet = actualBadSnippet.trim();
}
return actualBadSnippet.equals(badSnippet)
&& ( snippetFollow==null ||
snippetFollow.equals(getText(problem.getRange().getEnd(), snippetFollow.length())))
&& problem.getMessage().contains(messageSnippet);
}
private String getText(Position start, int length) {
int offset = document.toOffset(start);
String text = document.getText();
return text.substring(offset, offset+length);
}
private String getCharAt(Position start) {
String text = document.getText();
int offset = document.toOffset(start);

View File

@@ -117,7 +117,7 @@ public class ConcourseEditorTest {
" tags: a-single-string\n"
);
editor.assertProblems(
"task: a-task\n tags: a-single-string|One of [config, file] is required",
"-^ task|One of [config, file] is required",
"a-single-string|Expecting a 'Sequence'"
);
@@ -549,8 +549,8 @@ public class ConcourseEditorTest {
"jobs:\n" +
"- name: blah"
);
editor.assertProblems("name: blah|'plan' is required");
assertEquals(DiagnosticSeverity.Warning, editor.assertProblem("name: blah").getSeverity());
Diagnostic problem = editor.assertProblems("-^ name: blah|'plan' is required").get(0);
assertEquals(DiagnosticSeverity.Warning, problem.getSeverity());
editor = harness.newEditor(
"jobs:\n" +
@@ -558,8 +558,8 @@ public class ConcourseEditorTest {
" plan:\n" +
" - task: foo"
);
editor.assertProblems("task: foo|One of [config, file] is required");
assertEquals(DiagnosticSeverity.Warning, editor.assertProblem("task: foo").getSeverity());
problem = editor.assertProblems("-^ task: foo|One of [config, file] is required").get(0);
assertEquals(DiagnosticSeverity.Warning, problem.getSeverity());
editor = harness.newEditor(
"jobs:\n" +
@@ -614,10 +614,10 @@ public class ConcourseEditorTest {
"- name: job-1\n"
);
editor.assertProblems(
"name: job-1|'plan' is required",
"-^ name: job-1|'plan' is required",
"job-1|Duplicate job name",
"name: utils|'plan' is required",
"name: job-1|'plan' is required",
"-^ name: utils|'plan' is required",
"-^ name: job-1|'plan' is required",
"job-1|Duplicate job name"
);
}
@@ -1143,49 +1143,49 @@ public class ConcourseEditorTest {
"resources:\n" +
"- type: git"
);
editor.assertProblems("type: git|'name' is required");
editor.assertProblems("-^ type: git|'name' is required");
//addProp(resource, "type", t_resource_type_name).isRequired(true);
editor = harness.newEditor(
"resources:\n" +
"- name: foo"
);
editor.assertProblems("name: foo|'type' is required");
editor.assertProblems("-^ name: foo|'type' is required");
//Both name and type missing:
editor = harness.newEditor(
"resources:\n" +
"- source: {}"
);
editor.assertProblems("source: {}|[name, type] are required");
editor.assertProblems("-^ source:|[name, type] are required");
//addProp(job, "name", jobNameDef).isRequired(true);
editor = harness.newEditor(
"jobs:\n" +
"- name: foo"
);
editor.assertProblems("name: foo|'plan' is required");
editor.assertProblems("-^ name: foo|'plan' is required");
//addProp(job, "plan", f.yseq(step)).isRequired(true);
editor = harness.newEditor(
"jobs:\n" +
"- plan: []"
);
editor.assertProblems("plan: []|'name' is required");
editor.assertProblems("-^ plan: []|'name' is required");
//addProp(resourceType, "name", t_ne_string).isRequired(true);
editor = harness.newEditor(
"resource_types:\n" +
"- type: docker-image"
);
editor.assertProblems("type: docker-image|'name' is required");
editor.assertProblems("-^ type: docker-image|'name' is required");
//addProp(resourceType, "type", t_image_type).isRequired(true);
editor = harness.newEditor(
"resource_types:\n" +
"- name: foo"
);
editor.assertProblems("name: foo|'type' is required");
editor.assertProblems("-^ name: foo|'type' is required");
//addProp(gitSource, "uri", t_string).isRequired(true);
editor = harness.newEditor(
@@ -1212,7 +1212,7 @@ public class ConcourseEditorTest {
"groups:\n" +
"- jobs: []"
);
editor.assertProblems("jobs: []|'name' is required");
editor.assertProblems("-^ jobs: []|'name' is required");
}
@@ -2293,8 +2293,8 @@ public class ConcourseEditorTest {
"name|Unknown property",
"bogus-source-prop|Unknown property",
"image|Only one of [image_resource, image] should be defined",
"path: path/to/input|'name' is required",
"path: path/to/output|'name' is required",
"-^ path: path/to/input|'name' is required",
"-^ path: path/to/output|'name' is required",
"the-params|Expecting a 'Map'"
);
}
@@ -2388,8 +2388,8 @@ public class ConcourseEditorTest {
"name|Unknown property",
"bogus-source-prop|Unknown property",
"image|Only one of [image_resource, image] should be defined",
"path: path/to/input|'name' is required",
"path: path/to/output|'name' is required",
"-^ path: path/to/input|'name' is required",
"-^ path: path/to/output|'name' is required",
"the-params|Expecting a 'Map'"
);
}