diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java index 2c628f4d7..9301c4ce0 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/YTypeFactory.java @@ -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 diff --git a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java index be19b6694..b17153928 100644 --- a/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java +++ b/headless-services/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/schema/constraints/Constraints.java @@ -126,25 +126,6 @@ public class Constraints { }; } - public static Constraint deprecateProperty(Function 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 diff --git a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java index 8143d86f1..1a90d7cbc 100644 --- a/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java +++ b/headless-services/manifest-yaml-language-server/src/main/java/org/springframework/ide/vscode/manifest/yaml/ManifestYmlSchema.java @@ -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), diff --git a/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java b/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java index 7b82371fd..e2d54dc7f 100644 --- a/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java +++ b/headless-services/manifest-yaml-language-server/src/test/java/org/springframework/ide/vscode/manifest/yaml/ManifestYamlEditorTest.java @@ -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