Disable 'buildpack' deprecated warning

Also fixed some other small oddities in the code.
This commit is contained in:
Kris De Volder
2018-12-18 16:18:47 -08:00
parent d89baf2146
commit 797eb6717d
4 changed files with 24 additions and 40 deletions

View File

@@ -888,9 +888,10 @@ public class YTypeFactory {
public void isDeprecated(boolean isDeprecated) {
this.isDeprecated = isDeprecated;
}
public void isDeprecated(String deprecationMessage) {
public YTypedPropertyImpl isDeprecated(String deprecationMessage) {
this.isDeprecated = deprecationMessage!=null;
this.deprecationMessage = deprecationMessage;
return this;
}
@Override

View File

@@ -126,25 +126,6 @@ public class Constraints {
};
}
public static Constraint deprecateProperty(Function<String, String> messageFormatter, String deprecatedProperty) {
return (DynamicSchemaContext dc, Node parent, Node node, YType type, IProblemCollector problems) -> {
// the `node` is the VALUE off the property, not the property itself. We don't want to deprecate this.
// Instead we want to deprecate the associated keynode, which represents the property.
// Find the corresponding tuple in the parent, as we want to deprecate
// the keyNode (so the actual property)
if (parent instanceof MappingNode) {
MappingNode map = (MappingNode) parent;
for (NodeTuple prop : map.getValue()) {
Node keyNode = prop.getKeyNode();
String name = NodeUtil.asScalar(keyNode);
if (deprecatedProperty.equals(name)) {
problems.accept(YamlSchemaProblems.deprecatedProperty(messageFormatter.apply(name), keyNode));
}
}
}
};
}
/**
* Deprecated because you shouldn't need to use this method to create a {@link SchemaContextAware} Constraint.
* A Constraint itself is already implicitly aware of the {@link DynamicSchemaContext} (i.e. it already receives

View File

@@ -121,18 +121,11 @@ public final class ManifestYmlSchema implements YamlSchema {
YAtomicType t_path = f.yatomic("Path");
YAtomicType t_buildpack_entry = f.yatomic("Buildpack Entry");
if (buildpackProvider != null) {
t_buildpack_entry.setHintProvider(buildpackProvider);
// t_buildpack_entry.parseWith(ManifestYmlValueParsers.fromHints(t_buildpack_entry.toString(), buildpackProvider));
}
// Deprecated. See: https://www.pivotaltracker.com/story/show/162499688
YAtomicType t_buildpack = f.yatomic("Buildpack");
if (t_buildpack != null) {
//t_buildpack.require(Constraints.deprecateProperty((name) ->
// "Deprecated: Use `buildpacks` instead.", "buildpack"));
if (buildpackProvider != null) {
t_buildpack.setHintProvider(buildpackProvider);
t_buildpack.require(Constraints.deprecateProperty((name) ->
"Deprecated: Use `buildpacks` instead.", "buildpack"));
}
YAtomicType t_stack = f.yatomic("Stack");
@@ -198,7 +191,10 @@ public final class ManifestYmlSchema implements YamlSchema {
YTypedPropertyImpl[] props = {
f.yprop("buildpack", t_buildpack),
f.yprop("buildpacks", f.yseq(t_buildpack_entry)),
//TODO: replace the above with the below to make 'buildpack' deprecated once we have proper support for `buildpacks` in cf push.
//f.yprop("buildpack", t_buildpack).isDeprecated("Deprecated: Use `buildpacks` instead."),
//Note: don't forget to also re-enable the test case called 'reconcileDeprecatedBuildpackWarning'.
f.yprop("buildpacks", f.yseq(t_buildpack)),
f.yprop("command", t_string),
f.yprop("disk_quota", t_memory),
f.yprop("domain", t_domain),

View File

@@ -25,6 +25,7 @@ import org.eclipse.lsp4j.CompletionItem;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.DiagnosticSeverity;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
@@ -42,6 +43,8 @@ import org.springframework.ide.vscode.languageserver.testharness.LanguageServerH
import org.springframework.ide.vscode.manifest.yaml.bootiful.ManifestLanguageServerTest;
import org.springframework.test.context.junit4.SpringRunner;
import static org.springframework.ide.vscode.languageserver.testharness.Editor.*;
import com.google.common.collect.ImmutableList;
@RunWith(SpringRunner.class)
@@ -346,7 +349,7 @@ public class ManifestYamlEditorTest {
"- name: foo\n" +
" <*>"
);
editor.assertCompletions((c) -> !(c.getLabel().startsWith("- ") || c.getLabel().startsWith(Unicodes.LEFT_ARROW+" ")),
editor.assertCompletions(PLAIN_COMPLETION,
// ---------------
"applications:\n" +
"- name: foo\n" +
@@ -827,14 +830,17 @@ public class ManifestYamlEditorTest {
editor = harness.newEditor(
"applications:\n" +
"- name: my-app\n" +
" buildpack: java_buildpack");
// NOTE: should the warning appear over the value `java_buildpack` or the property `buildpack`?
// For now testing on the value, but if warning is instead switched to `buildpack`, this test should
// fail (expected) and updated to reflect this change
problem = editor.assertProblems(
"buildpack|Deprecated: Use `buildpacks` instead.")
.get(0);
assertEquals(DiagnosticSeverity.Warning, problem.getSeverity());
" buildpack: java_buildpack\n" +
" buildpacks:\n"+
" - java_buidpack\n"
);
editor.assertProblems(/*NONE*/); // no problems right now. But in the future...
// TODO: this is what it should really do:
// problem = editor.assertProblems(
// "buildpack|Deprecated: Use `buildpacks` instead.")
// .get(0);
// assertEquals(DiagnosticSeverity.Warning, problem.getSeverity());
}
@Test