Spring/spring-tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix PT-140709005 better underlining for missing property warnings

Browse Source 
Try to underline corresponding parent property key instead of the entire node that is missing the property.
This commit is contained in:
Kris De Volder
2017-04-03 12:11:34 -07:00
parent f64b90e19c
commit 88013ef803
5 changed files with 46 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java
View File

@@ -90,7 +90,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
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());
reconcile(ast.getDocument(), new YamlPath(YamlPathSegment.valueAt(i)), /*parent*/null, node, schema.getTopLevelType());
}
}
} finally {
@@ -116,7 +116,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
return allOf(ast, node);
}
private void reconcile(IDocument doc, YamlPath path, Node node, YType type) {
private void reconcile(IDocument doc, YamlPath path, Node parent, Node node, YType type) {
if (type!=null) {
DynamicSchemaContext schemaContext = new ASTDynamicSchemaContext(doc, path, node);
type = typeUtil.inferMoreSpecificType(type, schemaContext);
@@ -130,12 +130,12 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
if (typeUtil.isMap(type)) {
for (NodeTuple entry : map.getValue()) {
String key = NodeUtil.asScalar(entry.getKeyNode());
reconcile(doc, keyAt(path, key), entry.getKeyNode(), typeUtil.getKeyType(type));
reconcile(doc, valueAt(path, key), entry.getValueNode(), typeUtil.getDomainType(type));
reconcile(doc, keyAt(path, key), map, entry.getKeyNode(), typeUtil.getKeyType(type));
reconcile(doc, valueAt(path, key), map, entry.getValueNode(), typeUtil.getDomainType(type));
}
} else if (typeUtil.isBean(type)) {
Map<String, YTypedProperty> beanProperties = typeUtil.getPropertiesMap(type);
checkRequiredProperties(map, type, beanProperties, schemaContext);
checkRequiredProperties(parent, map, type, beanProperties, schemaContext);
for (NodeTuple entry : map.getValue()) {
Node keyNode = entry.getKeyNode();
String key = NodeUtil.asScalar(keyNode);
@@ -149,7 +149,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
if (prop.isDeprecated()) {
problems.accept(YamlSchemaProblems.deprecatedProperty(keyNode, type, prop));
}
reconcile(doc, valueAt(path, key), entry.getValueNode(), prop.getType());
reconcile(doc, valueAt(path, key), map, entry.getValueNode(), prop.getType());
}
}
}
@@ -162,7 +162,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
if (typeUtil.isSequencable(type)) {
for (int i = 0; i < seq.getValue().size(); i++) {
Node el = seq.getValue().get(i);
reconcile(doc, valueAt(path, i), el, typeUtil.getDomainType(type));
reconcile(doc, valueAt(path, i), seq, el, typeUtil.getDomainType(type));
}
} else {
expectTypeButFoundSequence(type, node);
@@ -214,7 +214,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
return e instanceof ProblemTypeProvider ? ((ProblemTypeProvider) e).getProblemType() : YamlSchemaProblems.SCHEMA_PROBLEM;
}
private void checkRequiredProperties(MappingNode map, YType type, Map<String, YTypedProperty> beanProperties, DynamicSchemaContext dc) {
private void checkRequiredProperties(Node parent, MappingNode map, YType type, Map<String, YTypedProperty> beanProperties, DynamicSchemaContext dc) {
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.
@@ -234,14 +234,14 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
} else {
message = "Properties "+missingProps+" are required for '"+type+"'";
}
problems.accept(YamlSchemaProblems.missingProperty(message, map));
problems.accept(YamlSchemaProblems.missingProperty(message, 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(map, type, foundProps, problems);
constraint.verify(parent, map, type, foundProps, problems);
}
}
}

14
vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java
View File

@@ -17,10 +17,15 @@ 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.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.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node;
import org.yaml.snakeyaml.nodes.NodeTuple;
import com.google.common.collect.ImmutableSet;
@@ -91,7 +96,14 @@ public class YamlSchemaProblems {
return new ReconcileProblemImpl(problemType, msg, start, end-start);
}
public static ReconcileProblem missingProperty(String msg, MappingNode map) {
public static ReconcileProblem missingProperty(String msg, 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());
}
}
}
return problem(MISSING_PROPERTY, msg, map);
}

3
vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraint.java
View File

@@ -15,6 +15,7 @@ import java.util.Set;
import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
import org.springframework.ide.vscode.commons.yaml.schema.YType;
import org.yaml.snakeyaml.nodes.MappingNode;
import org.yaml.snakeyaml.nodes.Node;
/**
* An implementations of this interface represents some 'programatic' constraint attached to a schema type.
@@ -35,6 +36,6 @@ 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(MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems);
void verify(Node parent, MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems);
}

6
vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java
View File

@@ -61,7 +61,7 @@ public class Constraints {
}
@Override
public void verify(MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) {
public void verify(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 +69,7 @@ public class Constraints {
if (foundPropsCount==0) {
if (!allowFewer) {
problems.accept(missingProperty(
"One of "+requiredProps+" is required for '"+type+"'", map));
"One of "+requiredProps+" is required for '"+type+"'", parent, map));
}
} else if (foundPropsCount>1) {
//Mark each of the found keys as a violation:
@@ -86,7 +86,7 @@ public class Constraints {
public static Constraint deprecated(Function<String, String> messageFormatter, String... _deprecatedNames) {
Set<String> deprecatedNames = ImmutableSet.copyOf(_deprecatedNames);
return (MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) -> {
return (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);

43
vscode-extensions/vscode-concourse/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java
View File

@@ -572,8 +572,8 @@ public class ConcourseEditorTest {
{
List<Diagnostic> problems = editor.assertProblems(
"config|Only one of [config, file]",
"{}|[platform, run] are required",
"{}|One of [image_resource, image]",
"config|[platform, run] are required",
"config|One of [image_resource, image]",
"file|Only one of [config, file]"
);
//All of the problems in this example are property contraint violations! So all should be warnings.
@@ -601,7 +601,7 @@ public class ConcourseEditorTest {
" args: [\"Hello, world!\"]"
);
editor.assertProblems(
"config|something"
"config|'platform' is required"
);
}
@@ -1007,7 +1007,7 @@ public class ConcourseEditorTest {
" - put: my-git\n" +
" params: {}\n"
);
editor.assertProblems("{}|'repository' is required");
editor.assertProblems("params|'repository' is required");
editor = harness.newEditor(
"resources:\n" +
@@ -1195,7 +1195,7 @@ public class ConcourseEditorTest {
" source:\n" +
" branch: master"
);
editor.assertProblems("branch: master|'uri' is required");
editor.assertProblems("source|'uri' is required");
//addProp(gitSource, "branch", t_string).isRequired(true);
editor = harness.newEditor(
@@ -1205,7 +1205,7 @@ public class ConcourseEditorTest {
" source:\n" +
" uri: https://yada"
);
editor.assertProblems("uri: https://yada|'branch' is required");
editor.assertProblems("source|'branch' is required");
//addProp(group, "name", t_ne_string).isRequired(true);
editor = harness.newEditor(
@@ -1226,7 +1226,7 @@ public class ConcourseEditorTest {
" source:\n" +
" tag: latest\n"
);
editor.assertProblems("tag: latest|'repository' is required");
editor.assertProblems("source|'repository' is required");
editor = harness.newEditor(
"resources:\n" +
@@ -1389,8 +1389,8 @@ public class ConcourseEditorTest {
" access_key_id: the-key"
);
editor.assertProblems(
"access_key_id: the-key|'bucket' is required",
"access_key_id: the-key|One of [regexp, versioned_file] is required"
"source|'bucket' is required",
"source|One of [regexp, versioned_file] is required"
);
editor = harness.newEditor(
@@ -1502,7 +1502,7 @@ public class ConcourseEditorTest {
" no-params-expected: bad"
);
editor.assertProblems(
"acl: public-read|'file' is required",
"params|'file' is required",
"no-params-expected|Unknown property"
);
@@ -1580,7 +1580,7 @@ public class ConcourseEditorTest {
" private_key: stuff"
);
editor.assertProblems(
"private_key: stuff|[branch, pool, uri] are required"
"source|[branch, pool, uri] are required"
);
editor = harness.newEditor(
@@ -1695,7 +1695,7 @@ public class ConcourseEditorTest {
" driver: s3"
);
editor.assertProblems(
"driver: s3|[access_key_id, bucket, key, secret_access_key] are required"
"source|[access_key_id, bucket, key, secret_access_key] are required"
);
editor = harness.newEditor(
@@ -1705,7 +1705,7 @@ public class ConcourseEditorTest {
" source: {}"
);
editor.assertProblems(
"{}|[access_key_id, bucket, key, secret_access_key] are required"
"source|[access_key_id, bucket, key, secret_access_key] are required"
);
// required props for git driver
@@ -1717,7 +1717,7 @@ public class ConcourseEditorTest {
" driver: git"
);
editor.assertProblems(
"driver: git|[branch, file, uri] are required"
"source|[branch, file, uri] are required"
);
//required props for swift driver
@@ -1729,7 +1729,7 @@ public class ConcourseEditorTest {
" driver: swift"
);
editor.assertProblems(
"driver: swift|'openstack' is required"
"source|'openstack' is required"
);
}
@@ -2437,7 +2437,7 @@ public class ConcourseEditorTest {
"run:\n" +
" user: admin\n"
);
editor.assertProblems("user: admin|'path' is required");
editor.assertProblems("run|'path' is required");
}
@Test public void nameAndPathHoversInTaskInputsAndOutputs() throws Exception {
@@ -2565,16 +2565,9 @@ public class ConcourseEditorTest {
" - mvn"
);
Diagnostic problem = editor.assertProblem(
"inputs:\n" +
" - name: commons-git\n" +
" platform: linux\n" +
" run:\n" +
" path: which\n" +
" args:\n" +
" - mvn"
editor.assertProblems(
"config|One of [image_resource, image] is required"
);
assertContains("One of [image_resource, image] is required", problem.getMessage());
}
@Test public void resourceInTaskConfigFileNotRequired() throws Exception {