Better handling of union types in Yaml completions

Schema-based completion engine provides the
completions for every subtype of a union type
(unless more specific subtype can be inferred).

See: https://github.com/spring-projects/sts4/issues/345#issuecomment-523069469

Signed-off-by: Kris De Volder <kdevolder@pivotal.io>

headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java

@@ -99,29 +99,42 @@ public class YTypeAssistContext extends AbstractYamlAssistContext {
 				return customContentAssistant.getCompletions(completionFactory(), region, region.toRelative(offset));
 			}
 		}
-		String query = getPrefix(doc, node, offset);
-		List<ICompletionProposal> completions = getValueCompletions(doc, node, offset, query);
-		if (completions.isEmpty()) {
-			TypeBasedSnippetProvider snippetProvider = typeUtil.getSnippetProvider();
-			if (snippetProvider!=null) {
-				Collection<Snippet> snippets = snippetProvider.getSnippets(type);
-				YamlIndentUtil indenter = new YamlIndentUtil(doc);
-				for (Snippet snippet : snippets) {
-					String snippetName = snippet.getName();
-					double score = FuzzyMatcher.matchScore(query, snippetName);
-					if (score!=0.0 && snippet.isApplicable(getSchemaContext())) {
-						DocumentEdits edits = createEditFromSnippet(doc, node, offset, query, indenter, snippet);
-						completions.add(completionFactory().valueProposal(snippetName, query, snippetName, type, null, score, edits, typeUtil));
+		if (typeUtil.isTrueUnion(type)) {
+			Collection<YType> unionSubTypes = typeUtil.getUnionSubTypes(type);
+			//When a union type was not inferred to one of its subtypes...
+			//Then suggest completions for all of its subtypes since, presumably they are
+			//all valid in this context at the moment.
+			List<ICompletionProposal> completions = new ArrayList<>();
+			for (YType unionSubType : unionSubTypes) {
+				YTypeAssistContext unionContext = new YTypeAssistContext(this, unionSubType);
+				completions.addAll(unionContext.getCompletions(doc, node, offset));
+			}
+			return completions;
+		} else {
+			String query = getPrefix(doc, node, offset);
+			List<ICompletionProposal> completions = getValueCompletions(doc, node, offset, query);
+			if (completions.isEmpty()) {
+				TypeBasedSnippetProvider snippetProvider = typeUtil.getSnippetProvider();
+				if (snippetProvider!=null) {
+					Collection<Snippet> snippets = snippetProvider.getSnippets(type);
+					YamlIndentUtil indenter = new YamlIndentUtil(doc);
+					for (Snippet snippet : snippets) {
+						String snippetName = snippet.getName();
+						double score = FuzzyMatcher.matchScore(query, snippetName);
+						if (score!=0.0 && snippet.isApplicable(getSchemaContext())) {
+							DocumentEdits edits = createEditFromSnippet(doc, node, offset, query, indenter, snippet);
+							completions.add(completionFactory().valueProposal(snippetName, query, snippetName, type, null, score, edits, typeUtil));
+						}
 					}
 				}
+				completions.addAll(getKeyCompletions(doc, node, offset, query));
 			}
-			completions.addAll(getKeyCompletions(doc, node, offset, query));
+			if (typeUtil.isSequencable(type)) {
+				completions = new ArrayList<>(completions);
+				completions.addAll(getDashedCompletions(doc, node, offset));
+			}
+			return completions;
 		}
-		if (typeUtil.isSequencable(type)) {
-			completions = new ArrayList<>(completions);
-			completions.addAll(getDashedCompletions(doc, node, offset));
-		}
-		return completions;
 	}
 
 	private DocumentEdits createEditFromSnippet(YamlDocument doc, SNode node, int offset, String query, YamlIndentUtil indenter,

headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/SNodeDynamicSchemaContext.java

@@ -15,13 +15,15 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.springframework.ide.vscode.commons.util.CollectionUtil;
-import org.springframework.ide.vscode.commons.util.Log;
 import org.springframework.ide.vscode.commons.util.text.IDocument;
 import org.springframework.ide.vscode.commons.yaml.path.YamlPath;
 import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SChildBearingNode;
 import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SKeyNode;
 import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SNode;
+import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser.SSeqNode;
 
 /**
  * Adapts an SNode so it can be used by a YamlSchema as a {@link DynamicSchemaContext}
@@ -30,6 +32,9 @@ import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureParser
  */
 public class SNodeDynamicSchemaContext extends CachingSchemaContext {
 
+	final static Logger log = LoggerFactory.getLogger(SNodeDynamicSchemaContext.class);
+
+	
 	private SNode contextNode;
 	private YamlPath contextPath;
 
@@ -54,7 +59,7 @@ public class SNodeDynamicSchemaContext extends CachingSchemaContext {
 				}
 			}
 		} catch (Exception e) {
-			Log.log(e);
+			log.error("", e);
 		}
 		return Collections.emptySet();
 	}
@@ -86,6 +91,20 @@ public class SNodeDynamicSchemaContext extends CachingSchemaContext {
 
 	@Override
 	public boolean isSequence() {
+		try {
+			if (contextNode instanceof SChildBearingNode) {
+				List<SNode> children = ((SChildBearingNode)contextNode).getChildren();
+				if (CollectionUtil.hasElements(children)) {
+					for (SNode c : children) {
+						if (c instanceof SSeqNode) {
+							return true;
+						}
+					}
+				}
+			}
+		} catch (Exception e) {
+			log.error("", e);
+		}
 		return false;
 	}
 

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

@@ -38,6 +38,7 @@ import org.springframework.ide.vscode.commons.util.Renderables;
 import org.springframework.ide.vscode.commons.util.ValueParser;
 import org.springframework.ide.vscode.commons.yaml.ast.YamlFileAST;
 import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems;
+import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.AbstractUnionType;
 import org.springframework.ide.vscode.commons.yaml.schema.YTypeFactory.YBeanAndSequenceUnion;
 import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraint;
 import org.springframework.ide.vscode.commons.yaml.schema.constraints.Constraints;
@@ -290,6 +291,14 @@ public class YTypeFactory {
 		public boolean suggestDeprecatedProperties() {
 			return suggestDeprecatedProperties;
 		}
+
+		@Override
+		public Collection<YType> getUnionSubTypes(YType type) {
+			if (type instanceof AbstractUnionType) {
+				return ((AbstractUnionType) type).getUnionSubTypes();
+			}
+			return ImmutableList.of(type);
+		}
 	};
 
 	/////////////////////////////////////////////////////////////////////////////////////
@@ -801,23 +810,49 @@ public class YTypeFactory {
 		}
 	}
 
-	public class YAtomAndMapUnion extends AbstractType {
+	public class AbstractUnionType extends AbstractType {
+		protected final String name;
+		protected final YType[] subtypes;
+		
+		public AbstractUnionType(String name, YType... subTypes) {
+			this.name = name;
+			this.subtypes = subTypes;
+		}
+		@Override
+		public final String toString() {
+			if (name!=null) {
+				return name;
+			} else {
+				StringBuilder b = new StringBuilder("(");
+				boolean first = true;
+				for (YType t : subtypes) {
+					if (!first) {
+						b.append(" | ");
+					}
+					b.append(t);
+					first = false;
+				}
+				b.append(")");
+				return b.toString();
+			}
+		}
+		
+		public Collection<YType> getUnionSubTypes() {
+			return ImmutableList.copyOf(subtypes);
+		}
+	}
+
+	public class YAtomAndMapUnion extends AbstractUnionType {
 
-		private String name;
 		private YAtomicType atom;
 		private YMapType map;
 
 		public YAtomAndMapUnion(String name, YAtomicType atom, YMapType map) {
-			this.name = name;
+			super(name, atom, map);
 			this.atom = atom;
 			this.map = map;
 		}
 
-		@Override
-		public String toString() {
-			return name;
-		}
-
 		@Override
 		public boolean isAtomic() {
 			return true;
@@ -845,14 +880,13 @@ public class YTypeFactory {
 
 	}
 	
-	public class YBeanAndSequenceUnion extends AbstractType {
+	public class YBeanAndSequenceUnion extends AbstractUnionType {
 
-		private String name;
-		private YBeanType bean;
-		private YSeqType seq;
+		private final YBeanType bean;
+		private final YSeqType seq;
 
 		public YBeanAndSequenceUnion(String name, YBeanType yBeanType, YSeqType ySeqType) {
-			this.name = name;
+			super(name, yBeanType, ySeqType);
 			this.bean = yBeanType;
 			this.seq = ySeqType;
 		}
@@ -867,15 +901,6 @@ public class YTypeFactory {
 			return super.inferMoreSpecificType(dc);
 		}
 
-		@Override
-		public String toString() {
-			if (name!=null) {
-				return name;
-			} else {
-				return "(" + bean +" | " + seq + ")";
-			}
-		}
-
 		@Override
 		public boolean isBean() {
 			return true;
@@ -885,7 +910,6 @@ public class YTypeFactory {
 		public boolean isSequenceable() {
 			return true;
 		}
-
 	}
 
 	public static class YTypedPropertyImpl implements YTypedProperty, Cloneable {

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

@@ -74,4 +74,32 @@ public interface YTypeUtil {
 	 * or suppress them (false).
 	 */
 	boolean suggestDeprecatedProperties();
+	
+	/**
+	 * If a type can be considered to be the union of several other types, then 
+	 * this method optionally can be implemented to return a collection of these
+	 * types. 
+	 * <p>
+	 * The default implementation returns a singleton colllection containing the type 
+	 * itself because every type can at least be considered a union of itself with nothing else.
+	 */
+	Collection<YType> getUnionSubTypes(YType type);
+	
+	/**
+	 * Determines whether this type is a 'true' union type. This means that 'getUnionSubTypes'
+	 * does not simply return a collection of the type itself.
+	 */
+	default boolean isTrueUnion(YType type) {
+		Collection<YType> subtypes = getUnionSubTypes(type);
+		if (subtypes!=null) {
+			for (YType subType : subtypes) {
+				if (subType.equals(type)) {
+					return false;
+				}
+			}
+			return true;
+		}
+		return false;
+	}
+	
 }

headless-services/concourse-language-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java

@@ -612,7 +612,84 @@ public class ConcourseEditorTest {
 				"      <*>"
 		);
 	}
+
+	@Test
+	public void inParallelStepCompletionInList() throws Exception {
+		Editor editor = harness.newEditor(
+			"jobs:\n" +
+			"- name: some-job\n" +
+			"  plan:\n" +
+			"  - in_parallel:\n" +
+			"    - <*>"
+		);
+		editor.assertCompletionWithLabel("get", 
+			"jobs:\n" +
+			"- name: some-job\n" +
+			"  plan:\n" +
+			"  - in_parallel:\n" +
+			"    - get: <*>"
+		);
+	}
+
+	@Test
+	public void inParallelStepCompletionOptions() throws Exception {
+		assertContextualCompletions(PLAIN_COMPLETION,
+				"jobs:\n" +
+				"- name: some-job\n" +
+				"  plan:\n" +
+				"  - in_parallel:\n" +
+				"      <*>"
+				, // ------------
+				"<*>"
+				, // ==>
+				"fail_fast: <*>"
+				,
+				"limit: <*>"
+				,
+				"steps:\n"+
+				"      - <*>"
+		);
 		
+		assertContextualCompletions(c -> {
+					String l = c.getLabel();
+					boolean isDedentedStep = l.startsWith(Unicodes.LEFT_ARROW+" -");
+					return isDedentedStep && (l.contains("get") || l.contains("put"));
+				},
+				"jobs:\n" +
+				"- name: some-job\n" +
+				"  plan:\n" +
+				"  - in_parallel:\n" +
+				"    <*>"
+				, // ------------
+				"  <*>"
+				, // ==>
+				"- get: <*>"
+				,
+				"- put: <*>"
+		);
+
+	}
+
+	@Test
+	public void inParallelStepCompletionInObject() throws Exception {
+		Editor editor = harness.newEditor(
+			"jobs:\n" +
+			"- name: some-job\n" +
+			"  plan:\n" +
+			"  - in_parallel:\n" +
+			"      steps:\n" +
+			"      - <*>"
+		);
+		editor.assertCompletionWithLabel("get", 
+			"jobs:\n" +
+			"- name: some-job\n" +
+			"  plan:\n" +
+			"  - in_parallel:\n" +
+			"      steps:\n" +
+			"      - get: <*>"
+		);
+	}
+
 	@Test
 	public void reconcileSimpleTypes() throws Exception {
 		Editor editor;