Skip to content

Commit

Permalink
Better handling of union types in Yaml completions
Browse files Browse the repository at this point in the history 
Schema-based completion engine provides the
completions for every subtype of a union type
(unless more specific subtype can be inferred).

See: #345 (comment)

Signed-off-by: Kris De Volder <[email protected]>
  • Loading branch information
@kdvolder
kdvolder committed Aug 20, 2019
1 parent 577901c commit 01c0498
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 44 deletions.
51 changes: 32 additions & 19 deletions .../main/java/org/springframework/ide/vscode/commons/yaml/completion/YTypeAssistContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,29 +99,42 @@ public Collection<ICompletionProposal> getCompletions(YamlDocument doc, SNode no
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));
if (typeUtil.isSequencable(type)) {
completions = new ArrayList<>(completions);
completions.addAll(getDashedCompletions(doc, node, offset));
}
return completions;
}
return completions;
}

private DocumentEdits createEditFromSnippet(YamlDocument doc, SNode node, int offset, String query, YamlIndentUtil indenter,
Expand Down
23 changes: 21 additions & 2 deletions ...in/java/org/springframework/ide/vscode/commons/yaml/schema/SNodeDynamicSchemaContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,15 @@
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}
Expand All @@ -30,6 +32,9 @@
*/
public class SNodeDynamicSchemaContext extends CachingSchemaContext {

final static Logger log = LoggerFactory.getLogger(SNodeDynamicSchemaContext.class);


private SNode contextNode;
private YamlPath contextPath;

Expand All @@ -54,7 +59,7 @@ protected Set<String> computeDefinedProperties() {
}
}
} catch (Exception e) {
Log.log(e);
log.error("", e);
}
return Collections.emptySet();
}
Expand Down Expand Up @@ -86,6 +91,20 @@ public boolean isMap() {

@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;
}

Expand Down
70 changes: 47 additions & 23 deletions ...s-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
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;
Expand Down Expand Up @@ -290,6 +291,14 @@ public boolean tieredOptionalPropertyProposals() {
public boolean suggestDeprecatedProperties() {
return suggestDeprecatedProperties;
}

@Override
public Collection<YType> getUnionSubTypes(YType type) {
if (type instanceof AbstractUnionType) {
return ((AbstractUnionType) type).getUnionSubTypes();
}
return ImmutableList.of(type);
}
};

/////////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -801,23 +810,49 @@ public YType inferMoreSpecificType(DynamicSchemaContext dc) {
}
}

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;
Expand Down Expand Up @@ -845,14 +880,13 @@ public PartialCollection<YValueHint> getHintValues(DynamicSchemaContext dc) {

}

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;
}
Expand All @@ -867,15 +901,6 @@ public YType inferMoreSpecificType(DynamicSchemaContext dc) {
return super.inferMoreSpecificType(dc);
}

@Override
public String toString() {
if (name!=null) {
return name;
} else {
return "(" + bean +" | " + seq + ")";
}
}

@Override
public boolean isBean() {
return true;
Expand All @@ -885,7 +910,6 @@ public boolean isBean() {
public boolean isSequenceable() {
return true;
}

}

public static class YTypedPropertyImpl implements YTypedProperty, Cloneable {
Expand Down
28 changes: 28 additions & 0 deletions ...mons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

}
77 changes: 77 additions & 0 deletions ...ge-server/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,84 @@ public void inParallelStepCompletion() throws Exception {
" <*>"
);
}

@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;
Expand Down

0 comments on commit 01c0498

Please sign in to comment.