From 3267d1de7cb7aeecffe0eddf23bace786c6c203d Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Wed, 1 Mar 2017 16:49:18 -0800 Subject: [PATCH] Property constraint violations in concourse editor are warnings. --- .../util/SimpleLanguageServer.java | 28 ++++++------ .../SchemaBasedYamlASTReconciler.java | 6 +-- .../yaml/reconcile/YamlSchemaProblems.java | 10 +++++ .../languageserver/testharness/Editor.java | 6 ++- .../concourse/ConcourseLanguageServer.java | 16 +++++-- .../vscode/concourse/ConcourseEditorTest.java | 45 ++++++++++++++++++- 6 files changed, 88 insertions(+), 23 deletions(-) diff --git a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java index 858c5a050..9687c1df3 100644 --- a/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java +++ b/vscode-extensions/commons/commons-language-server/src/main/java/org/springframework/ide/vscode/commons/languageserver/util/SimpleLanguageServer.java @@ -193,20 +193,6 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl LOG.log(Level.WARNING, "Invalid reconcile problem ignored", e); } } - - private DiagnosticSeverity getDiagnosticSeverity(ReconcileProblem problem) { - ProblemSeverity severity = problem.getType().getDefaultSeverity(); - switch (severity) { - case ERROR: - return DiagnosticSeverity.Error; - case WARNING: - return DiagnosticSeverity.Warning; - case IGNORE: - return null; - default: - throw new IllegalStateException("Bug! Missing switch case?"); - } - } }; // Avoid running in the same thread as lsp4j as it can result @@ -221,6 +207,20 @@ public abstract class SimpleLanguageServer implements LanguageServer, LanguageCl .subscribe(); } + protected DiagnosticSeverity getDiagnosticSeverity(ReconcileProblem problem) { + ProblemSeverity severity = problem.getType().getDefaultSeverity(); + switch (severity) { + case ERROR: + return DiagnosticSeverity.Error; + case WARNING: + return DiagnosticSeverity.Warning; + case IGNORE: + return null; + default: + throw new IllegalStateException("Bug! Missing switch case?"); + } + } + public void waitForReconcile() throws Exception { while (!this.busyReconcile.isDone()) { this.busyReconcile.get(); diff --git a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java index 6ff139519..e6b77d777 100644 --- a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java +++ b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/SchemaBasedYamlASTReconciler.java @@ -235,7 +235,7 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { } else { message = "Properties "+missingProps+" are required for '"+type+"'"; } - problem(map, message); + problem(map, message, YamlSchemaProblems.MISSING_PROPERTY); } //Check for missing/extra 'one-of' constrained properties @@ -245,13 +245,13 @@ public class SchemaBasedYamlASTReconciler implements YamlASTReconciler { .filter(foundProps::contains) .count(); if (foundPropsCount==0) { - problem(map, "One of "+requiredProps+" is required for '"+type+"'"); + 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+"'"); + problem(entry.getKeyNode(), "Only one of "+requiredProps+" should be defined for '"+type+"'", YamlSchemaProblems.EXTRA_PROPERTY); } } } diff --git a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java index 32f307a4c..1ed254951 100644 --- a/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java +++ b/vscode-extensions/commons/commons-yaml/src/main/java/org/springframework/ide/vscode/commons/yaml/reconcile/YamlSchemaProblems.java @@ -10,6 +10,8 @@ *******************************************************************************/ package org.springframework.ide.vscode.commons.yaml.reconcile; +import java.util.Set; + import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemSeverity; import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemType; import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblem; @@ -19,6 +21,8 @@ import org.springframework.ide.vscode.commons.yaml.schema.YType; import org.springframework.ide.vscode.commons.yaml.schema.YTypedProperty; import org.yaml.snakeyaml.nodes.Node; +import com.google.common.collect.ImmutableSet; + /** * Methods for creating reconciler problems for Schema based reconciler implementation. * @@ -29,6 +33,12 @@ public class YamlSchemaProblems { public static final ProblemType SYNTAX_PROBLEM = problemType("YamlSyntaxProblem"); public static final ProblemType SCHEMA_PROBLEM = problemType("YamlSchemaProblem"); public static final ProblemType DEPRECATED_PROPERTY = problemType("DeprecatedProperty", ProblemSeverity.WARNING); + public static final ProblemType MISSING_PROPERTY = problemType("MissingProperty", ProblemSeverity.ERROR); + public static final ProblemType EXTRA_PROPERTY = problemType("ExtraProperty", ProblemSeverity.ERROR); + + public static final Set PROPERTY_CONSTRAINT = ImmutableSet.of( + MISSING_PROPERTY, EXTRA_PROPERTY + ); public static ProblemType problemType(final String typeName, ProblemSeverity defaultSeverity) { diff --git a/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java b/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java index 7561fd567..03e8bced7 100644 --- a/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java +++ b/vscode-extensions/commons/language-server-test-harness/src/main/java/org/springframework/ide/vscode/languageserver/testharness/Editor.java @@ -42,6 +42,9 @@ import org.eclipse.lsp4j.TextEdit; import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.junit.Assert; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; + import reactor.core.publisher.Flux; public class Editor { @@ -120,7 +123,7 @@ public class Editor { * @param expectedProblems * @throws BadLocationException */ - public void assertProblems(String... expectedProblems) throws Exception { + public List assertProblems(String... expectedProblems) throws Exception { Editor editor = this; List actualProblems = new ArrayList<>(editor.reconcile().stream().filter(d -> { return !ignoredTypes.contains(d.getCode()); @@ -140,6 +143,7 @@ public class Editor { if (bad!=null) { fail(bad+problemSumary(editor, actualProblems)); } + return ImmutableList.copyOf(actualProblems); } private String problemSumary(Editor editor, List actualProblems) throws Exception { diff --git a/vscode-extensions/vscode-concourse/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java b/vscode-extensions/vscode-concourse/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java index 96ce4eb91..00379f134 100644 --- a/vscode-extensions/vscode-concourse/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java +++ b/vscode-extensions/vscode-concourse/src/main/java/org/springframework/ide/vscode/concourse/ConcourseLanguageServer.java @@ -14,24 +14,26 @@ import java.util.concurrent.CompletableFuture; import org.eclipse.lsp4j.CompletionList; import org.eclipse.lsp4j.CompletionOptions; +import org.eclipse.lsp4j.DiagnosticSeverity; import org.eclipse.lsp4j.ServerCapabilities; import org.eclipse.lsp4j.TextDocumentSyncKind; import org.springframework.ide.vscode.commons.languageserver.LanguageIds; import org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngine; import org.springframework.ide.vscode.commons.languageserver.completion.VscodeCompletionEngineAdapter; import org.springframework.ide.vscode.commons.languageserver.hover.HoverInfoProvider; -import org.springframework.ide.vscode.commons.languageserver.hover.VscodeHoverEngine; import org.springframework.ide.vscode.commons.languageserver.hover.VscodeHoverEngineAdapter; import org.springframework.ide.vscode.commons.languageserver.reconcile.IReconcileEngine; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ProblemType; +import org.springframework.ide.vscode.commons.languageserver.reconcile.ReconcileProblem; import org.springframework.ide.vscode.commons.languageserver.util.SimpleLanguageServer; import org.springframework.ide.vscode.commons.languageserver.util.SimpleTextDocumentService; import org.springframework.ide.vscode.commons.util.text.TextDocument; import org.springframework.ide.vscode.commons.yaml.ast.YamlASTProvider; import org.springframework.ide.vscode.commons.yaml.completion.SchemaBasedYamlAssistContextProvider; -import org.springframework.ide.vscode.commons.yaml.completion.YamlAssistContextProvider; import org.springframework.ide.vscode.commons.yaml.completion.YamlCompletionEngine; import org.springframework.ide.vscode.commons.yaml.hover.YamlHoverInfoProvider; import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaBasedReconcileEngine; +import org.springframework.ide.vscode.commons.yaml.reconcile.YamlSchemaProblems; import org.springframework.ide.vscode.commons.yaml.schema.YamlSchema; import org.springframework.ide.vscode.commons.yaml.structure.YamlStructureProvider; @@ -46,13 +48,11 @@ public class ConcourseLanguageServer extends SimpleLanguageServer { private class SchemaSpecificPieces { - final YamlSchema schema; final VscodeCompletionEngine completionEngine; final VscodeHoverEngineAdapter hoverEngine; final YamlSchemaBasedReconcileEngine reconcileEngine; SchemaSpecificPieces(YamlSchema schema) { - this.schema = schema; SchemaBasedYamlAssistContextProvider contextProvider = new SchemaBasedYamlAssistContextProvider(schema); YamlCompletionEngine yamlCompletionEngine = new YamlCompletionEngine(structureProvider, contextProvider); this.completionEngine = new VscodeCompletionEngineAdapter(ConcourseLanguageServer.this, yamlCompletionEngine); @@ -126,6 +126,14 @@ public class ConcourseLanguageServer extends SimpleLanguageServer { documents.onDefinition(definitionFinder); } + @Override + protected DiagnosticSeverity getDiagnosticSeverity(ReconcileProblem problem) { + ProblemType type = problem.getType(); + if (YamlSchemaProblems.PROPERTY_CONSTRAINT.contains(type)) { + return DiagnosticSeverity.Warning; + } + return super.getDiagnosticSeverity(problem); + } @Override protected ServerCapabilities getServerCapabilities() { diff --git a/vscode-extensions/vscode-concourse/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java b/vscode-extensions/vscode-concourse/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java index 76f3ec11d..e7b13212c 100644 --- a/vscode-extensions/vscode-concourse/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java +++ b/vscode-extensions/vscode-concourse/src/test/java/org/springframework/ide/vscode/concourse/ConcourseEditorTest.java @@ -11,13 +11,14 @@ package org.springframework.ide.vscode.concourse; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; import static org.springframework.ide.vscode.languageserver.testharness.TestAsserts.assertContains; import java.io.InputStream; import java.util.Arrays; +import java.util.List; import java.util.stream.Collectors; +import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.DiagnosticSeverity; import org.junit.Before; import org.junit.Test; @@ -536,6 +537,48 @@ public class ConcourseEditorTest { ); } + @Test + public void violatedPropertyConstraintsAreWarnings() throws Exception { + Editor editor; + + editor = harness.newEditor( + "jobs:\n" + + "- name: blah" + ); + editor.assertProblems("name: blah|'plan' is required"); + assertEquals(DiagnosticSeverity.Warning, editor.assertProblem("name: blah").getSeverity()); + + editor = harness.newEditor( + "jobs:\n" + + "- name: do-stuff\n" + + " plan:\n" + + " - task: foo" + ); + editor.assertProblems("task: foo|One of [config, file] is required"); + assertEquals(DiagnosticSeverity.Warning, editor.assertProblem("task: foo").getSeverity()); + + editor = harness.newEditor( + "jobs:\n" + + "- name: do-stuff\n" + + " plan:\n" + + " - task: foo\n" + + " config: {}\n" + + " file: path/to/file" + ); + { + List problems = editor.assertProblems( + "config|Only one of [config, file]", + "{}|[inputs, platform, run] are required", + "{}|One of [image_resource, image]", + "file|Only one of [config, file]" + ); + //All of the problems in this example are property contraint violations! So all should be warnings. + for (Diagnostic diagnostic : problems) { + assertEquals(DiagnosticSeverity.Warning, diagnostic.getSeverity()); + } + } + } + @Test public void reconcileDuplicateJobNames() throws Exception { Editor editor = harness.newEditor(