Fix PT-142788419 more accurate checking of constraints

The use of 'image' and 'image_resource' in an embedded task config is only
required if it is not already defined in the task itself.

vscode-extensions/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/text/IDocument.java

@@ -28,5 +28,6 @@ public interface IDocument {
 	int getLineOffset(int line) throws BadLocationException;
 	void replace(int start, int len, String text) throws BadLocationException;
 	String textBetween(int start, int end) throws BadLocationException;
+	String getLanguageId();
 
 }

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

@@ -27,7 +27,6 @@ import java.util.stream.Collectors;
 import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
 import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemType;
 import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemTypeProvider;
-import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileException;
 import org.springframework.ide.vscode.commons.languageserver.util.DocumentRegion;
 import org.springframework.ide.vscode.commons.util.ExceptionUtil;
 import org.springframework.ide.vscode.commons.util.IntegerRange;
@@ -42,10 +41,12 @@ 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.ASTDynamicSchemaContext;
 import org.springframework.ide.vscode.commons.yaml.schema.DynamicSchemaContext;
+import org.springframework.ide.vscode.commons.yaml.schema.SchemaContextAware;
 import org.springframework.ide.vscode.commons.yaml.schema.YType;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeUtil;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypedProperty;
 import org.springframework.ide.vscode.commons.yaml.schema.YamlSchema;
+import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint;
 import org.yaml.snakeyaml.nodes.MappingNode;
 import org.yaml.snakeyaml.nodes.Node;
 import org.yaml.snakeyaml.nodes.NodeId;
@@ -53,8 +54,6 @@ import org.yaml.snakeyaml.nodes.NodeTuple;
 import org.yaml.snakeyaml.nodes.ScalarNode;
 import org.yaml.snakeyaml.nodes.SequenceNode;
 
-import com.google.gson.internal.Streams;
-
 public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
 
 	private final IProblemCollector problems;
@@ -136,7 +135,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
 					}
 				} else if (typeUtil.isBean(type)) {
 					Map<String, YTypedProperty> beanProperties = typeUtil.getPropertiesMap(type);
-					checkRequiredProperties(map, type, beanProperties);
+					checkRequiredProperties(map, type, beanProperties, schemaContext);
 					for (NodeTuple entry : map.getValue()) {
 						Node keyNode = entry.getKeyNode();
 						String key = NodeUtil.asScalar(keyNode);
@@ -215,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) {
+	private void checkRequiredProperties(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.
@@ -238,22 +237,11 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler {
 				problem(map, message, YamlSchemaProblems.MISSING_PROPERTY);
 			}
 
-			//Check for missing/extra 'one-of' constrained properties
-			for (String[] _requiredProps : typeUtil.getOneOfConstraints(type)) {
-				List<String> requiredProps = Arrays.asList(_requiredProps);
-				long foundPropsCount = requiredProps.stream()
-					.filter(foundProps::contains)
-					.count();
-				if (foundPropsCount==0) {
-					problem(map, "One of "+requiredProps+" is required for '"+type+"'", YamlSchemaProblems.MISSING_PROPERTY);
-				} else if (foundPropsCount>1) {
-					//Mark each of the found keys as a violation:
-					for (NodeTuple entry : map.getValue()) {
-						String key = NodeUtil.asScalar(entry.getKeyNode());
-						if (key!=null && requiredProps.contains(key)) {
-							problem(entry.getKeyNode(), "Only one of "+requiredProps+" should be defined for '"+type+"'", YamlSchemaProblems.EXTRA_PROPERTY);
-						}
-					}
+			//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);
 				}
 			}
 		}

vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/SchemaContextAware.java

@@ -14,9 +14,16 @@ package org.springframework.ide.vscode.commons.yaml.schema;
  * Interface that can be implemented by something producing another
  * component (of some type `T`) where the returned component needs to
  * configured with a {@link DynamicSchemaContext}.
- * 
+ *
  * @author Kris De Volder
  */
 public interface SchemaContextAware<T> {
 	T withContext(DynamicSchemaContext dc);
+
+	/**
+	 * Convert a plain value into a {@link SchemaContextAware} that ignores the context and simply returns the value.
+	 */
+	public static <T> SchemaContextAware<T> just(T it) {
+		return (dc) -> it;
+	}
 }

vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java

@@ -30,6 +30,8 @@ import org.springframework.ide.vscode.commons.util.Renderable;
 import org.springframework.ide.vscode.commons.util.Renderables;
 import org.springframework.ide.vscode.commons.util.ValueParser;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.AbstractType;
+import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint;
+import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraints;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
@@ -143,8 +145,8 @@ public class YTypeFactory {
 		}
 
 		@Override
-		public List<String[]> getOneOfConstraints(YType type) {
-			return ((AbstractType)type).getOneOfConstraints();
+		public List<SchemaContextAware<Constraint>> getConstraints(YType type) {
+			return ((AbstractType)type).getConstraints();
 		}
 	};
 
@@ -161,7 +163,7 @@ public class YTypeFactory {
 		private Map<String, YTypedProperty> cachedPropertyMap;
 		private SchemaContextAware<Callable<Collection<YValueHint>>> hintProvider;
 
-		private List<String[]> oneOfConstraints = new ArrayList<>(1);
+		private List<SchemaContextAware<Constraint>> constraints = new ArrayList<>(2);
 
 		public boolean isSequenceable() {
 			return false;
@@ -224,8 +226,8 @@ public class YTypeFactory {
 			return ImmutableList.of();
 		}
 
-		public List<String[]> getOneOfConstraints() {
-			return ImmutableList.copyOf(oneOfConstraints);
+		public List<SchemaContextAware<Constraint>> getConstraints() {
+			return ImmutableList.copyOf(constraints);
 		}
 
 		public List<YTypedProperty> getProperties() {
@@ -299,9 +301,12 @@ public class YTypeFactory {
 			return parser == null ? null : parser.withContext(dc);
 		}
 
+		public void require(SchemaContextAware<Constraint> dynamicConstraint) {
+			this.constraints.add(dynamicConstraint);
+		}
+
 		public void requireOneOf(String... properties) {
-			Assert.isLegal(properties.length>1);
-			this.oneOfConstraints.add(properties);
+			this.constraints.add(SchemaContextAware.just(Constraints.requireOneOf(properties)));
 		}
 
 		public String[] getPropertyNames() {

vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeUtil.java

@@ -14,6 +14,7 @@ import java.util.List;
 import java.util.Map;
 
 import org.springframework.ide.vscode.commons.util.ValueParser;
+import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint;
 
 /**
  * An implementation of YTypeUtil provides implementations of various
@@ -45,5 +46,5 @@ public interface YTypeUtil {
 	 * should be returned.
 	 */
 	YType inferMoreSpecificType(YType type, DynamicSchemaContext dc);
-	List<String[]> getOneOfConstraints(YType type);
+	List<SchemaContextAware<Constraint>> getConstraints(YType type);
 }

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

@@ -0,0 +1,40 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Pivotal, Inc.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Pivotal, Inc. - initial API and implementation
+ *******************************************************************************/
+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.yaml.schema.YType;
+import org.yaml.snakeyaml.nodes.MappingNode;
+
+/**
+ * An implementations of this interface represents some 'programatic' constraint attached to a schema type.
+ * Essentially, it encapsulates a verification procedure to be called upon to validate something when
+ * visiting a node in the YamlAST that has been deterimed to be of that type.
+ */
+@FunctionalInterface
+public interface Constraint {
+
+	/**
+	 * Implemetors gain access to various bits of context information passed as parameters and
+	 * are supposed to use this information in whatever way they like to check if the
+	 * constraint is satisfied. When the constrain is not satisfied they should report any
+	 * violations by adding problems to the provide {@link IProblemCollector}.
+	 *
+	 * @param map    The node being validated
+	 * @param type   The inferred type of the node.
+	 * @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);
+
+}

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

@@ -0,0 +1,81 @@
+/*******************************************************************************
+ * Copyright (c) 2017 Pivotal, Inc.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Pivotal, Inc. - initial API and implementation
+ *******************************************************************************/
+package org.springframework.ide.vscode.commons.yaml.schema.constraints;
+
+import static org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems.EXTRA_PROPERTY;
+import static org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems.MISSING_PROPERTY;
+import static org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems.problem;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+
+import org.springframework.ide.vscode.commons.languageserver.reconcile.IProblemCollector;
+import org.springframework.ide.vscode.commons.util.Assert;
+import org.springframework.ide.vscode.commons.yaml.ast.NodeUtil;
+import org.springframework.ide.vscode.commons.yaml.schema.YType;
+import org.yaml.snakeyaml.nodes.MappingNode;
+import org.yaml.snakeyaml.nodes.NodeTuple;
+
+/**
+ * Various static methods for constructing/composing {@link Constraint}s.
+ *
+ * @author Kris De Volder
+ */
+public class Constraints {
+
+	public static Constraint requireOneOf(String... properties) {
+		return new RequireOneOf(properties);
+	}
+
+	public static Constraint requireAtMostOneOf(String... properties) {
+		return new RequireOneOf(properties).allowFewer(true);
+	}
+
+	static private class RequireOneOf implements Constraint {
+
+		private final String[] _requiredProps;
+		private boolean allowFewer = false;
+
+		public RequireOneOf(String[] properties) {
+			Assert.isLegal(properties.length>1);
+			this._requiredProps = properties;
+		}
+
+		public Constraint allowFewer(boolean b) {
+			this.allowFewer = b;
+			return this;
+		}
+
+		@Override
+		public void verify(MappingNode map, YType type, Set<String> foundProps, IProblemCollector problems) {
+			List<String> requiredProps = Arrays.asList(_requiredProps);
+			long foundPropsCount = requiredProps.stream()
+				.filter(foundProps::contains)
+				.count();
+			if (foundPropsCount==0) {
+				if (!allowFewer) {
+					problems.accept(problem(MISSING_PROPERTY,
+							"One of "+requiredProps+" is required for '"+type+"'", map));
+				}
+			} else if (foundPropsCount>1) {
+				//Mark each of the found keys as a violation:
+				for (NodeTuple entry : map.getValue()) {
+					String key = NodeUtil.asScalar(entry.getKeyNode());
+					if (key!=null && requiredProps.contains(key)) {
+						problems.accept(problem(EXTRA_PROPERTY,
+								"Only one of "+requiredProps+" should be defined for '"+type+"'",  entry.getKeyNode()));
+					}
+				}
+			}
+		}
+	}
+}

vscode-extensions/vscode-concourse/src/main/java/org/springframework/ide/vscode/concourse/PipelineYmlSchema.java

@@ -13,6 +13,7 @@ package org.springframework.ide.vscode.concourse;
 import java.util.Set;
 import java.util.stream.Collectors;
 
+import org.springframework.ide.vscode.commons.languageserver.LanguageIds;
 import org.springframework.ide.vscode.commons.util.MimeTypes;
 import org.springframework.ide.vscode.commons.util.Renderable;
 import org.springframework.ide.vscode.commons.util.Renderables;
@@ -22,6 +23,7 @@ import org.springframework.ide.vscode.commons.yaml.ast.YamlFileAST;
 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.DynamicSchemaContext;
+import org.springframework.ide.vscode.commons.yaml.schema.SchemaContextAware;
 import org.springframework.ide.vscode.commons.yaml.schema.YType;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.AbstractType;
@@ -29,6 +31,8 @@ import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YAtomicTy
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YBeanType;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YBeanUnionType;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YTypedPropertyImpl;
+import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint;
+import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraints;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeUtil;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypedProperty;
 import org.springframework.ide.vscode.commons.yaml.schema.YValueHint;
@@ -211,7 +215,20 @@ public class PipelineYmlSchema implements YamlSchema {
 		addProp(task, "outputs", f.yseq(t_output));
 		addProp(task, "run", t_command).isRequired(true);
 		addProp(task, "params", t_string_params);
-		task.requireOneOf("image_resource", "image");
+		task.require((dc) -> {
+			String languageId = dc.getDocument().getLanguageId();
+			if (languageId==LanguageIds.CONCOURSE_PIPELINE) {
+				Node parentImageDef = getParentPropertyNode("image", models, dc);
+				if (parentImageDef==null) {
+					return Constraints.requireOneOf("image_resource", "image");
+				} else {
+					// TODO: something like this: return Constraints.deprecated("image_resource", "image");
+					return null;
+				}
+			} else {
+				return Constraints.requireAtMostOneOf("image_resource", "image");
+			}
+		});
 
 		AbstractType t_put_get_name = f.contextAware("Name", (dc) -> {
 			if (getParentPropertyNode("resource", models, dc)!=null) {

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

@@ -2491,6 +2491,80 @@ public class ConcourseEditorTest {
 		editor.assertCompletions(/*NONE*/);
 	}
 
+	@Test public void resourceInEmbeddedTaskConfigNotRequiredIfSpecifiedInTask() throws Exception {
+		Editor editor;
+
+		editor = harness.newEditor(
+				"resources:\n" +
+				"- name: docker-image\n" +
+				"  type: docker-image\n" +
+				"  source:\n" +
+				"    username: {{docker_hub_username}}\n" +
+				"    password: {{docker_hub_password}}\n" +
+				"    repository: kdvolder/sts3-build-env\n" +
+				"jobs:\n" +
+				"- name: build-commons-update-site\n" +
+				"  plan:\n" +
+				"  - task: hello-world\n" +
+				"    image: docker-image\n" + //Given here! So not required in config!
+				"    config:\n" +
+				"      inputs:\n" +
+				"      - name: commons-git\n" +
+				"      platform: linux\n" +
+				"      run:\n" +
+				"        path: which\n" +
+				"        args:\n" +
+				"        - mvn"
+		);
+		editor.assertProblems(/*none*/);
+
+		editor = harness.newEditor(
+				"resources:\n" +
+				"- name: docker-image\n" +
+				"  type: docker-image\n" +
+				"  source:\n" +
+				"    username: {{docker_hub_username}}\n" +
+				"    password: {{docker_hub_password}}\n" +
+				"    repository: kdvolder/sts3-build-env\n" +
+				"jobs:\n" +
+				"- name: build-commons-update-site\n" +
+				"  plan:\n" +
+				"  - task: hello-world\n" +
+				"    config:\n" +
+				"      inputs:\n" +
+				"      - name: commons-git\n" +
+				"      platform: linux\n" +
+				"      run:\n" +
+				"        path: which\n" +
+				"        args:\n" +
+				"        - mvn"
+		);
+
+		Diagnostic problem = editor.assertProblem(
+				"inputs:\n" +
+				"      - name: commons-git\n" +
+				"      platform: linux\n" +
+				"      run:\n" +
+				"        path: which\n" +
+				"        args:\n" +
+				"        - mvn"
+		);
+		assertContains("One of [image_resource, image] is required", problem.getMessage());
+	}
+
+	@Test public void resourceInTaskConfigFileNotRequired() throws Exception {
+		Editor editor = harness.newEditor(LanguageIds.CONCOURSE_TASK,
+				"inputs:\n" +
+				"- name: commons-git\n" +
+				"platform: linux\n" +
+				"run:\n" +
+				"  path: which\n" +
+				"  args:\n" +
+				"  - mvn"
+		);
+		editor.assertProblems(/*NONE*/);
+	}
+
 	@Ignore @Test public void relaxedIndentContextMoreSpaces() throws Exception {
 		Editor editor;