diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java index d401e083..b0ab9951 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java @@ -18,6 +18,9 @@ import java.util.Set; import java.util.logging.Logger; +import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime; +import io.openliberty.tools.langserver.lemminx.services.SettingsService; +import io.openliberty.tools.langserver.lemminx.util.LibertyUtils; import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lemminx.commons.CodeActionFactory; import org.eclipse.lemminx.commons.TextDocument; @@ -63,7 +66,10 @@ public void doCodeAction(ICodeActionRequest request, List codeAction Diagnostic diagnostic = request.getDiagnostic(); DOMDocument document = request.getDocument(); TextDocument textDocument = document.getTextDocument(); - + LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(document); + String libertyVersion = runtimeInfo == null ? null : runtimeInfo.getRuntimeVersion(); + String libertyRuntime = runtimeInfo == null ? null : runtimeInfo.getRuntimeType(); + final int requestDelay = SettingsService.getInstance().getRequestDelay(); LibertyWorkspace ws = LibertyProjectsManager.getInstance().getWorkspaceFolder(document.getDocumentURI()); FeatureListGraph featureGraph = null; if (ws == null) { @@ -90,6 +96,12 @@ public void doCodeAction(ICodeActionRequest request, List codeAction ArrayList sortedFeatures = new ArrayList(); sortedFeatures.addAll(featureCandidates); Collections.sort(sortedFeatures); + // get existing platforms from the document + List existingPlatforms = FeatureService.getInstance() + .collectExistingPlatforms(document, ""); + // find version less features for all versioned features in the sorted features + Set possibleVersionlessFeatures = FeatureService.getInstance() + .getVersionLessFeaturesForVersioned(sortedFeatures, libertyRuntime, libertyVersion, requestDelay, document.getDocumentURI()); String insertText = ""; int referenceRangeStart = 0; @@ -137,6 +149,16 @@ public void doCodeAction(ICodeActionRequest request, List codeAction } insertText = IndentUtil.formatText(insertText, indent, referenceRange.getStart().getCharacter()); + // for each versionless features that has at least one matching platform to a specified platform in the document + for (String feature : possibleVersionlessFeatures) { + Set allPlatforms = FeatureService.getInstance().getAllPlatformsForVersionLessFeature(feature, libertyVersion, libertyRuntime, requestDelay, document.getDocumentURI()); + if (allPlatforms.stream().anyMatch(existingPlatforms::contains)) { + String title = "Add feature " + feature; + codeActions.add(CodeActionFactory.insert( + title, referenceRange.getEnd(), + String.format(insertText, feature), textDocument, diagnostic)); + } + } for (String feature : sortedFeatures) { String title = "Add feature " + feature; codeActions.add(CodeActionFactory.insert( diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java index 26ba4b3e..28ac9bb0 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/services/FeatureService.java @@ -755,4 +755,26 @@ public List collectExistingPlatforms(DOMDocument document, String curren } return includedPlatforms; } + + /** + * get version less feature list for specified list of versioned features + * @param versionedFeatureNames list of versioned feature names + * @param libertyRuntime librty runtime + * @param libertyVersion runtime version + * @param requestDelay request delay + * @param documentURI server xml document uri + * @return version less feature list + */ + public Set getVersionLessFeaturesForVersioned(List versionedFeatureNames, String libertyRuntime, String libertyVersion,int requestDelay, String documentURI) { + FeaturesAndPlatforms featuresAndPlatforms = getFeaturesAndPlatforms( libertyVersion,libertyRuntime, requestDelay, documentURI); + Set featureNames = featuresAndPlatforms.getPublicFeatures().stream() + .filter(feature -> feature.getWlpInformation() != null) + .map(feature -> feature.getWlpInformation().getShortName().toLowerCase()) + .collect(Collectors.toSet()); + + return versionedFeatureNames.stream() + .map(LibertyUtils::stripVersion) + .filter(feature -> featureNames.contains(feature.toLowerCase())) + .collect(Collectors.toSet()); + } } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index 52421d2f..407583a0 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -1092,6 +1092,61 @@ public void testConfigElementSameNameAsVersionlessFeatureNoDiagnostics() throws @Test public void testConfigElementSameNameAsVersionlessFeatureWithDiagnosticsAndCodeAction() throws BadLocationException { + String configElement = ""; + String serverXML = String.join(newLine, + "", + " ", + " javaee-7.0", + " ", + configElement, + "" + ); + Diagnostic diagnostic = new Diagnostic(); + diagnostic.setRange(r(4, 0, 4, 46)); + diagnostic.setCode(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE); + diagnostic.setMessage(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_MESSAGE); + + XMLAssert.testDiagnosticsFor(serverXML, null, null, sampleserverXMLURI, diagnostic); + diagnostic.setSource("mpMetrics"); + + List featuresToAdd = new ArrayList<>(); + // here javaee-7.0 platform is specified , which is not valid for mpMetrics + // hence versionless feature is not shown + featuresToAdd.add("mpMetrics-1.0"); + featuresToAdd.add("mpMetrics-1.1"); + featuresToAdd.add("mpMetrics-2.0"); + featuresToAdd.add("mpMetrics-2.2"); + featuresToAdd.add("mpMetrics-2.3"); + featuresToAdd.add("mpMetrics-3.0"); + featuresToAdd.add("mpMetrics-4.0"); + featuresToAdd.add("mpMetrics-5.0"); + featuresToAdd.add("mpMetrics-5.1"); + + Collections.sort(featuresToAdd); + + List codeActions = new ArrayList<>(); + for (String nextFeature: featuresToAdd) { + String addFeature = String.format("%s%s",System.lineSeparator(),nextFeature); + TextEdit texted = te(2, 39, 2, 39, addFeature); + CodeAction invalidCodeAction = ca(diagnostic, texted); + + TextDocumentEdit textDoc = tde(sampleserverXMLURI, 0, texted); + WorkspaceEdit workspaceEdit = new WorkspaceEdit(Collections.singletonList(Either.forLeft(textDoc))); + + invalidCodeAction.setEdit(workspaceEdit); + codeActions.add(invalidCodeAction); + } + + // diagnostic with code action expected + XMLAssert.testCodeActionsFor(serverXML, sampleserverXMLURI, diagnostic, (String) null, + codeActions.get(0), codeActions.get(1), codeActions.get(2), + codeActions.get(3), codeActions.get(4), codeActions.get(5), + codeActions.get(6), codeActions.get(7), codeActions.get(8) + ); + } + + @Test + public void testConfigElementDiagnosticsAndCodeActionWithVersionless() throws BadLocationException { String configElement = ""; String serverXML = String.join(newLine, "", @@ -1106,10 +1161,12 @@ public void testConfigElementSameNameAsVersionlessFeatureWithDiagnosticsAndCodeA diagnostic.setCode(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE); diagnostic.setMessage(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_MESSAGE); - XMLAssert.testDiagnosticsFor(serverXML, null, null, sampleserverXMLURI,diagnostic); + XMLAssert.testDiagnosticsFor(serverXML, null, null, sampleserverXMLURI, diagnostic); diagnostic.setSource("mpMetrics"); List featuresToAdd = new ArrayList(); + // show version less feature name since microprofile platform is added + featuresToAdd.add("mpMetrics"); featuresToAdd.add("mpMetrics-1.0"); featuresToAdd.add("mpMetrics-1.1"); featuresToAdd.add("mpMetrics-2.0"); @@ -1139,7 +1196,7 @@ public void testConfigElementSameNameAsVersionlessFeatureWithDiagnosticsAndCodeA XMLAssert.testCodeActionsFor(serverXML, sampleserverXMLURI, diagnostic, (String) null, codeActions.get(0), codeActions.get(1), codeActions.get(2), codeActions.get(3), codeActions.get(4), codeActions.get(5), - codeActions.get(6), codeActions.get(7), codeActions.get(8) + codeActions.get(6), codeActions.get(7), codeActions.get(8), codeActions.get(9) ); } } \ No newline at end of file diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactory.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactory.java index c12ee4ee..6f41bfd3 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactory.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactory.java @@ -12,6 +12,7 @@ *******************************************************************************/ package io.openliberty.tools.langserver.codeactions; +import com.google.gson.JsonPrimitive; import io.openliberty.tools.langserver.LibertyLanguageServer; import io.openliberty.tools.langserver.LibertyTextDocumentService; import io.openliberty.tools.langserver.ls.LibertyTextDocument; @@ -32,6 +33,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; public abstract class CodeActionQuickfixFactory { @@ -46,6 +48,22 @@ protected CodeActionQuickfixFactory(LibertyTextDocumentService libertyTextDocume /** * returns list of code actions or commands. * called from CodeActionParticipant + * 1. In case of quick fix for single value property + * a. show all allowed values in code action + * 2. Multi value property, + * a. If field has multiple values specified + * b. If any of the value is valid, show code action + * "Replace with {validValues}" + * "Replace with {validValues},${nextAllowedValue1}" + * ... + * "Replace with {validValues},${nextAllowedValueN}" + * example, user entered,WLP_LOGGING_CONSOLE_SOURCE=abc,audit,message,kyc + * quickfix should contain something like + * Replace with "audit,message + * Replace with "audit,message,trace" + * Replace with "audit,message,ffdc" + * Replace with "audit,message,auditLog" + * * @param params code action params * @return codeaction */ @@ -57,9 +75,19 @@ public List> apply(CodeActionParams params) { if (diagnostic.getCode() != null && getErrorCode().equals(diagnostic.getCode().getLeft())) { String line = new ParserFileHelperUtil().getLine(new LibertyTextDocument(openedDocument), diagnostic.getRange().getStart().getLine()); if (line != null) { + String prefix = getUserEnteredValidValues(diagnostic); + if (!Objects.equals(prefix, "")) { + // add a code action to just replace with valid values + // present in current string + res.add(Either.forRight(createCodeAction(params, diagnostic, prefix))); + // append a comma so that completion will show all values + // for multi value + prefix += ","; + } // fetch all completion values and shows them as quick fix + // prefix will contain all valid values in current entered string, or else "" // completion returns empty list if no completion item is present - List possibleProperties = retrieveCompletionValues(openedDocument, diagnostic.getRange().getStart()); + List possibleProperties = retrieveCompletionValues(openedDocument, diagnostic.getRange().getStart(), prefix); for (String mostProbableProperty : possibleProperties) { // expected format for a code action is res.add(Either.forRight(createCodeAction(params, diagnostic, mostProbableProperty))); @@ -70,6 +98,26 @@ public List> apply(CodeActionParams params) { return res; } + /** + * get valid values entered by user in the current line + * in case of multi value property, user may have entered some valid and some invalid values + * @param diagnostic + * @return + */ + private static String getUserEnteredValidValues(Diagnostic diagnostic) { + String prefix = ""; + // user entered valid values are passed in diagnostic.setData() + if (diagnostic.getData() != null) { + if (diagnostic.getData() instanceof JsonPrimitive) { + prefix = ((JsonPrimitive) diagnostic.getData()).getAsString(); + } + if (diagnostic.getData() instanceof String) { + prefix = (String) diagnostic.getData(); + } + } + return prefix; + } + /** * used to create code action object for quickfix * @param params codeaction params @@ -88,7 +136,7 @@ protected CodeAction createCodeAction(CodeActionParams params, Diagnostic diagno return codeAction; } - protected abstract List retrieveCompletionValues(TextDocumentItem textDocumentItem, Position position); + protected abstract List retrieveCompletionValues(TextDocumentItem textDocumentItem, Position position, String prefix); protected abstract String getErrorCode(); } diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyValueQuickfix.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyValueQuickfix.java index eed67a70..52d36d7d 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyValueQuickfix.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyValueQuickfix.java @@ -43,20 +43,23 @@ protected String getErrorCode() { /** * retrieve list of completion items for a property key + * * @param textDocumentItem text document - * @param position current position, used to compute key + * @param position current position, used to compute key + * @param prefix prefix value to trigger completion with * @return list of string of completion item names */ @Override protected List retrieveCompletionValues(TextDocumentItem textDocumentItem, - Position position) { + Position position, String prefix) { List completionValues = new ArrayList<>(); try { LibertyTextDocument openedDocument = libertyLanguageServer.getTextDocumentService().getOpenedDocument(textDocumentItem.getUri()); String line = new ParserFileHelperUtil().getLine(openedDocument, position); PropertiesEntryInstance propertiesEntryInstance = new PropertiesEntryInstance(line, openedDocument); + CompletableFuture> completions; // get all completions for current property key - CompletableFuture> completions = propertiesEntryInstance.getPropertyValueInstance().getCompletions("", position); + completions = propertiesEntryInstance.getPropertyValueInstance().getCompletions(prefix, position); // map text values from completion items completionValues = completions.thenApply(completionItems -> completionItems.stream() .map(it -> it.getTextEdit().getLeft().getNewText()) diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/diagnostic/LibertyPropertiesDiagnosticService.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/diagnostic/LibertyPropertiesDiagnosticService.java index 33143a8d..a202928a 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/diagnostic/LibertyPropertiesDiagnosticService.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/diagnostic/LibertyPropertiesDiagnosticService.java @@ -85,9 +85,10 @@ private List computeInvalidValuesDiagnostic(PropertiesValidationResu String messageTemplate = DiagnosticMessages.getString(validationResult.getDiagnosticType()); // Currently the last arg (getIntegerRange) is only used for the Integer messages which use {2}. Otherwise null is passed and is ignored by the other messages. - String message = MessageFormat.format(messageTemplate, validationResult.getValue(), property, ServerPropertyValues.getIntegerRange(property)); + String message = MessageFormat.format(messageTemplate, validationResult.getCustomValue() != null ? validationResult.getCustomValue() : validationResult.getValue(), property, ServerPropertyValues.getIntegerRange(property)); Diagnostic diagnostic = new Diagnostic(computeRange(validationResult, lineContentInError), message, DiagnosticSeverity.Error, "Liberty Config Language Server"); diagnostic.setCode(ERROR_CODE_INVALID_PROPERTY_VALUE); + diagnostic.setData(validationResult.getMultiValuePrefix()); lspDiagnostics.add(diagnostic); } return lspDiagnostics; diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/model/propertiesfile/PropertiesKeyInstance.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/model/propertiesfile/PropertiesKeyInstance.java index 58d90da7..2d14c6a3 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/model/propertiesfile/PropertiesKeyInstance.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/model/propertiesfile/PropertiesKeyInstance.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2022, 2024 IBM Corporation and others. +* Copyright (c) 2022, 2025 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at @@ -9,9 +9,13 @@ *******************************************************************************/ package io.openliberty.tools.langserver.model.propertiesfile; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.concurrent.CompletableFuture; -import java.util.stream.Collectors; +import java.util.stream.Stream; import org.eclipse.lsp4j.CompletionItem; import org.eclipse.lsp4j.CompletionItemKind; @@ -108,28 +112,104 @@ protected void setDetailsOnCompletionItems(List items, String ke } } + /** + * this method will return valid properties for a specified key + * 1. In case of multi value property and user has entered multiple properties + * a. split input to prefix and filtervalue + * b. if all values in prefix are valid, use filtervalue and show completion + * of all allowed properties exlcuding already added valid properties + * c. if any of the values in entered text is invalid, do not show completion + * 2. in case of single value property or no comma is specified in multi value + * a. consider the entered value as filter and filter through all valid values + * to show completion + * @param enteredValue entered value + * @param position current position in document + * @return completion Items + */ public List getValidValues(String enteredValue, Position position) { - List values = ServerPropertyValues.getValidValues(textDocumentItem, propertyKey); - List results = values.stream() - .filter(s -> s.toLowerCase().contains(enteredValue.trim().toLowerCase())) - .map(s -> { - int line = position.getLine(); - Position rangeStart = new Position(line, getEndPosition() + 2); - Position rangeEnd = new Position(line, getEndPosition() + 2 + s.length()); - Range range = new Range(rangeStart, rangeEnd); - Either edit = Either.forLeft(new TextEdit(range, s)); - CompletionItem completionItem = new CompletionItem(); - completionItem.setTextEdit(edit); - completionItem.setLabel(s); - return completionItem; - }).toList(); + List validValues = ServerPropertyValues.getValidValues(textDocumentItem, propertyKey); + List enteredValuesLowerCase = new ArrayList<>(); + String filterValue; + String prefix; + int endPosition = getEndPosition() + 2; + if (ServerPropertyValues.multipleCommaSeparatedValuesAllowed(propertyKey)) { + if (enteredValue != null && enteredValue.contains(",")) { + // has comma separated values + prefix = enteredValue.substring(0, enteredValue.lastIndexOf(",") + 1); + filterValue = enteredValue.substring(enteredValue.lastIndexOf(",") + 1); + enteredValuesLowerCase = ServerPropertyValues.getCommaSeparatedValues(prefix).stream() + .map(String::toLowerCase).toList(); + // in case any of the entered value is invalid, do not show completion + Set invalidValuesEntered = new HashSet<>(enteredValuesLowerCase); + validValues.forEach(v -> invalidValuesEntered.remove(v.toLowerCase())); + if (!invalidValuesEntered.isEmpty()) { + return Collections.emptyList(); + } + } else { + // in case of no comma, set prefix as empty and use entered value to filter for property + filterValue = enteredValue; + prefix = ""; + } + } else { + // in case single value allowed, set prefix as empty and use entered value to filter for property + // this else is required to make variable effectively final + filterValue = enteredValue; + prefix = ""; + } + List results = generateCompletionItemsList(position, validValues, filterValue, enteredValuesLowerCase, prefix, endPosition); // Preselect the default. - // This uses the first item in the List as default. + // This uses the first item in the List as default. // (Check ServerPropertyValues.java) Currently sorted to have confirmed/sensible values as default. setDetailsOnCompletionItems(results, propertyKey, true); return results; } + /** + * 1. generate list of completion items based on filtering using filter value, + * and excluding all existing values + * 2. append prefix into completion string in case of multi value properties + * if user has user entered,WLP_LOGGING_CONSOLE_SOURCE=audit,message,| + * completion should contain something like + * "audit,message,trace" + * "audit,message,ffdc" + * "audit,message,auditLog" + * @param position current position + * @param validValues allowed values list + * @param filterValue filter through all valid allowed values + * @param enteredValuesLowerCase existing values in property, empty in case of single value + * @param prefix prefix in case of multi value + * @param endPosition last position + * @return completion items + */ + private List generateCompletionItemsList(Position position, List validValues, String filterValue, List enteredValuesLowerCase, String prefix, int endPosition) { + Stream filteredCompletion = validValues.stream() + .filter(s -> s.toLowerCase().contains(filterValue.trim().toLowerCase())); + if (!enteredValuesLowerCase.isEmpty()) { + // if there is already a value specified in case of comma separated field, filter out existing + filteredCompletion = filteredCompletion.filter(c -> !enteredValuesLowerCase.contains(c.toLowerCase())); + } + return filteredCompletion.map(s -> getCompletionItem(position, prefix + s, endPosition)).toList(); + } + + /** + * populate completion item object + * @param position current line position + * @param labelString string + * @param endPosition endposition + * @return + */ + private CompletionItem getCompletionItem(Position position, String labelString, int endPosition) { + int line = position.getLine(); + Position rangeStart = new Position(line, endPosition); + Position rangeEnd = new Position(line, endPosition + labelString.length()); + Range range = new Range(rangeStart, rangeEnd); + Either edit = Either.forLeft(new TextEdit(range, labelString)); + CompletionItem completionItem = new CompletionItem(); + completionItem.setTextEdit(edit); + completionItem.setLabel(labelString); + return completionItem; + } + @Override public String toString() { return this.propertyKey; diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/PropertiesValidationResult.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/PropertiesValidationResult.java index f6f960ab..6d9fa176 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/PropertiesValidationResult.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/PropertiesValidationResult.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2022, 2023 IBM Corporation and others. +* Copyright (c) 2022, 2025 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at @@ -13,6 +13,10 @@ package io.openliberty.tools.langserver.utils; import java.math.BigInteger; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashSet; import java.util.List; import java.util.logging.Logger; import java.util.regex.Matcher; @@ -30,6 +34,11 @@ public class PropertiesValidationResult { String diagnosticType; PropertiesEntryInstance entry; LibertyTextDocument textDocumentItem; + // being used for passing correct values for quickfix + // in case the current value has multiple values, and some of them is correct + String multiValuePrefix; + // used in case we need to send a custom diagnostic message + String customValue; private static final Logger LOGGER = Logger.getLogger(PropertiesValidationResult.class.getName()); @@ -37,6 +46,7 @@ private PropertiesValidationResult(PropertiesEntryInstance entry) { this.entry = entry; this.textDocumentItem = entry.getTextDocument(); this.hasErrors = false; + this.multiValuePrefix = ""; } /** @@ -79,8 +89,13 @@ public void validateServerProperty() { if (ServerPropertyValues.usesPredefinedValues(textDocumentItem, property)) { List validValues = ServerPropertyValues.getValidValues(textDocumentItem, property); if(ServerPropertyValues.isCaseSensitive(property)) { - // if case-sensitive, check if value is valid - hasErrors = !validValues.contains(value); + // currently all comma separated values properties are case-sensitive + if (ServerPropertyValues.multipleCommaSeparatedValuesAllowed(getKey()) && value != null) { + validateMultiValueProperty(value, validValues); + } else { + // case-sensitive single value field, check if value is valid + hasErrors = !validValues.contains(value); + } } else { // ignoring case, check if value is valid hasErrors = !validValues.stream().anyMatch(value::equalsIgnoreCase); @@ -130,6 +145,41 @@ public void validateServerProperty() { return; } + /** + * validate multi value property + * 1. set hasErrors to true if value starts or ends with comma + * 2. if multi value has some valid and invalid values + * a. send a custom message back to show only invalid values in error message + * b. send valid values along with diagnostic data, + * so that code action can show option to quickfix using valid values + * @param value + * @param validValues + */ + private void validateMultiValueProperty(String value, List validValues) { + if (value.startsWith(",") || value.endsWith(",")) { + hasErrors = true; + } else { + List enteredValues = ServerPropertyValues.getCommaSeparatedValues(value); + hasErrors = !new HashSet<>(validValues).containsAll(enteredValues); + HashSet invalidValues = new HashSet<>(enteredValues); + validValues.forEach(invalidValues::remove); + // setting a custom value for diagnostic message, to show only invalid values in the message + setCustomValue(String.join(",", invalidValues)); + // getting all valid prefix in case of multiple values + // example, user entered,WLP_LOGGING_CONSOLE_SOURCE=abc,audit,message,kyc + // quickfix should contain something like + // replace with "audit,message + // replace with "audit,message,trace" + // replace with "audit,message,ffdc" + // replace with "audit,message,auditLog" + List retainValues = new ArrayList<>(validValues); + retainValues.retainAll(enteredValues); + retainValues.sort(Comparator.comparingInt( + ServerPropertyValues.LOGGING_SOURCE_VALUES::indexOf)); + setMultiValuePrefix(String.join(",", retainValues)); + } + } + public void setLineNumber(Integer lineNumber) { this.lineNumber = lineNumber; } @@ -165,4 +215,20 @@ public String getValue() { public String getDiagnosticType() { return diagnosticType; } + + public String getMultiValuePrefix() { + return multiValuePrefix; + } + + public void setMultiValuePrefix(String multiValuePrefix) { + this.multiValuePrefix = multiValuePrefix; + } + + public String getCustomValue() { + return customValue; + } + + public void setCustomValue(String customValue) { + this.customValue = customValue; + } } diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/ServerPropertyValues.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/ServerPropertyValues.java index 2b94b887..dca8237d 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/ServerPropertyValues.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/utils/ServerPropertyValues.java @@ -1,5 +1,5 @@ /******************************************************************************* -* Copyright (c) 2022, 2023 IBM Corporation and others. +* Copyright (c) 2022, 2025 IBM Corporation and others. * * This program and the accompanying materials are made available under the * terms of the Eclipse Public License v. 2.0 which is available at @@ -9,6 +9,7 @@ *******************************************************************************/ package io.openliberty.tools.langserver.utils; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -74,6 +75,18 @@ public class ServerPropertyValues { ); }}; + private static Set commaSeparatedValuesAllowedProperties = new HashSet() {{ + add("WLP_LOGGING_CONSOLE_SOURCE"); + add("WLP_LOGGING_MESSAGE_SOURCE"); + EquivalentProperties.getServerVarKeys().forEach( + serverKey -> { + if(this.contains(serverKey)) { + this.add(EquivalentProperties.getEquivalentProperty(serverKey)); + } + } + ); + }}; + private static HashMap> integerRangeValues = new HashMap>() {{ put("WLP_DEBUG_ADDRESS", Range.between(1,65535)); put("default.http.port", Range.between(1,65535)); @@ -133,4 +146,12 @@ public static boolean isCaseSensitive(String key) { public static Range getIntegerRange(String key) { return integerRangeValues.get(key); } + + public static boolean multipleCommaSeparatedValuesAllowed(String key) { + return commaSeparatedValuesAllowedProperties.contains(key); + } + + public static List getCommaSeparatedValues(String valueString){ + return new ArrayList<>(Arrays.asList(valueString.split(","))); + } } diff --git a/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactoryTest.java b/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactoryTest.java index 5aaad5a0..869faa19 100644 --- a/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactoryTest.java +++ b/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/CodeActionQuickfixFactoryTest.java @@ -43,6 +43,11 @@ protected TextDocumentIdentifier initAndLaunchDiagnostic(String fileName) throws FileInputStream streamWithContentToTest = new FileInputStream(f); return initAndLaunchDiagnostic(f, streamWithContentToTest); } + protected TextDocumentIdentifier initAndLaunchDiagnostic(String path, String fileName) throws FileNotFoundException { + File f = new File(path + fileName); + FileInputStream streamWithContentToTest = new FileInputStream(f); + return initAndLaunchDiagnostic(f, streamWithContentToTest); + } protected TextDocumentIdentifier initAndLaunchDiagnostic(File f, InputStream streamWithContentToTest) { libertyLanguageServer = initializeLanguageServerWithFileUriString(streamWithContentToTest, f.toURI().toString()); diff --git a/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyQuickfixTest.java b/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyQuickfixTest.java index 506b4cf6..939e98ae 100644 --- a/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyQuickfixTest.java +++ b/liberty-ls/src/test/java/io/openliberty/tools/langserver/codeactions/InvalidPropertyQuickfixTest.java @@ -34,6 +34,7 @@ public void testReturnCodeActionForQuickfixForBootStrapProperties() throws FileN diagnostic1.setCode(ERROR_CODE_INVALID_PROPERTY_VALUE); diagnostic1.setSeverity(DiagnosticSeverity.Error); diagnostic1.setSource("Liberty Config Language Server"); + diagnostic1.setData(""); diagnostics.add(diagnostic1); CompletableFuture>> codeActionsCompletableFuture = retrieveCodeActions(textDocumentIdentifier, lastPublishedDiagnostics.getDiagnostics().get(0)); @@ -57,6 +58,7 @@ public void testReturnCodeActionForQuickfixForServerEnv() throws FileNotFoundExc diagnostic1.setSeverity(DiagnosticSeverity.Error); diagnostic1.setSource("Liberty Config Language Server"); diagnostics.add(diagnostic1); + diagnostic1.setData(""); CompletableFuture>> codeActionsCompletableFuture = retrieveCodeActions(textDocumentIdentifier, diagnostic); List expectedCodeActions=populateCodeActions(diagnostics, "Replace value with AUDIT","Replace value with INFO", @@ -64,6 +66,50 @@ public void testReturnCodeActionForQuickfixForServerEnv() throws FileNotFoundExc checkRetrievedCodeAction(textDocumentIdentifier, codeActionsCompletableFuture, expectedRange, expectedCodeActions); } + @Test + public void testReturnCodeActionForQuickfixForServerEnvWithMultiValues() throws FileNotFoundException, ExecutionException, InterruptedException { + TextDocumentIdentifier textDocumentIdentifier = initAndLaunchDiagnostic("src/test/resources/workspace/codeaction/src/main/liberty/config2/","server.env"); + + Diagnostic diagnostic = lastPublishedDiagnostics.getDiagnostics().get(0); + + Range expectedRange = new Range(new Position(0, 27), new Position(0, 43)); + List diagnostics = new ArrayList<>(); + Diagnostic diagnostic1 = new Diagnostic(expectedRange, "The value `au` is not valid for the variable `WLP_LOGGING_MESSAGE_SOURCE`."); + diagnostic1.setCode(ERROR_CODE_INVALID_PROPERTY_VALUE); + diagnostic1.setSeverity(DiagnosticSeverity.Error); + diagnostic1.setSource("Liberty Config Language Server"); + diagnostics.add(diagnostic1); + diagnostic1.setData("message,trace"); + CompletableFuture>> codeActionsCompletableFuture = retrieveCodeActions(textDocumentIdentifier, diagnostic); + List expectedCodeActions=populateCodeActions(diagnostics,"Replace value with message,trace", + "Replace value with message,trace,accessLog", + "Replace value with message,trace,ffdc","Replace value with message,trace,audit"); + checkRetrievedCodeAction(textDocumentIdentifier, codeActionsCompletableFuture, expectedRange, expectedCodeActions); + } + + + @Test + public void testReturnCodeActionForQuickfixForBootstrapWithMultiValues() throws FileNotFoundException, ExecutionException, InterruptedException { + TextDocumentIdentifier textDocumentIdentifier = initAndLaunchDiagnostic("src/test/resources/workspace/codeaction/src/main/liberty/config2/","bootstrap.properties"); + + Diagnostic diagnostic = lastPublishedDiagnostics.getDiagnostics().get(0); + + Range expectedRange = new Range(new Position(0, 34), new Position(0, 51)); + List diagnostics = new ArrayList<>(); + Diagnostic diagnostic1 = new Diagnostic(expectedRange, "The value `aud` is not valid for the property `com.ibm.ws.logging.console.source`."); + diagnostic1.setCode(ERROR_CODE_INVALID_PROPERTY_VALUE); + diagnostic1.setSeverity(DiagnosticSeverity.Error); + diagnostic1.setSource("Liberty Config Language Server"); + diagnostics.add(diagnostic1); + diagnostic1.setData("message,trace"); + CompletableFuture>> codeActionsCompletableFuture = retrieveCodeActions(textDocumentIdentifier, diagnostic); + List expectedCodeActions=populateCodeActions(diagnostics,"Replace value with message,trace", + "Replace value with message,trace,accessLog", + "Replace value with message,trace,ffdc","Replace value with message,trace,audit"); + checkRetrievedCodeAction(textDocumentIdentifier, codeActionsCompletableFuture, expectedRange, expectedCodeActions); + } + + private void checkRetrievedCodeAction(TextDocumentIdentifier textDocumentIdentifier, CompletableFuture>> codeActions, Range expectedRange, List expectedCodeActions) throws InterruptedException, ExecutionException { int index = 0; diff --git a/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/BootstrapPropertyCompletionTest.java b/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/BootstrapPropertyCompletionTest.java index 4b869e62..47f8e202 100644 --- a/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/BootstrapPropertyCompletionTest.java +++ b/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/BootstrapPropertyCompletionTest.java @@ -117,6 +117,38 @@ public void testValueCompletionLogProviderUserEnteredInvalidValue() throws Excep assertEquals(0, completionItems.size()); } + @Test + public void testValueCompletionMultiPropertyValue() throws Exception { + CompletableFuture, CompletionList>> completions = getCompletion("com.ibm.ws.logging.console.source=trace", new Position(0, 33)); + List completionItems = completions.get().getLeft(); + assertEquals(1, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace"); + + completions = getCompletion("com.ibm.ws.logging.console.source=trace,", new Position(0, 34)); + completionItems = completions.get().getLeft(); + assertEquals(4, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace,accessLog","trace,message","trace,ffdc","trace,audit"); + + completions = getCompletion("com.ibm.ws.logging.console.source=trace,a", new Position(0, 35)); + completionItems = completions.get().getLeft(); + assertEquals(3, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace,accessLog","trace,audit","trace,message"); + + completions = getCompletion("com.ibm.ws.logging.console.source=trace,message,", new Position(0, 42)); + completionItems = completions.get().getLeft(); + assertEquals(3, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace,message,accessLog","trace,message,ffdc","trace,message,audit"); + + completions = getCompletion("com.ibm.ws.logging.console.source=trace,message", new Position(0, 41)); + completionItems = completions.get().getLeft(); + assertEquals(1, completionItems.size()); + + // return no completion because messag is invalid + completions = getCompletion("com.ibm.ws.logging.message.source=trace,messag,", new Position(0, 40)); + completionItems = completions.get().getLeft(); + assertEquals(0, completionItems.size()); + } + protected CompletableFuture, CompletionList>> getCompletion(String enteredText, Position position) throws URISyntaxException, InterruptedException, ExecutionException { String filename = "bootstrap.properties"; File file = new File(resourcesDir, filename); diff --git a/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/ServerEnvCompletionTest.java b/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/ServerEnvCompletionTest.java index dd933818..06df2d2f 100644 --- a/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/ServerEnvCompletionTest.java +++ b/liberty-ls/src/test/java/io/openliberty/tools/langserver/completion/ServerEnvCompletionTest.java @@ -149,6 +149,39 @@ public void testValueCompletionForYesNoWhenUserEnteredY() throws Exception { } + + @Test + public void testValueCompletionMultiPropertyValue() throws Exception { + CompletableFuture, CompletionList>> completions = getCompletion("WLP_LOGGING_MESSAGE_SOURCE=trace", new Position(0, 33)); + List completionItems = completions.get().getLeft(); + assertEquals(1, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace"); + + completions = getCompletion("WLP_LOGGING_MESSAGE_SOURCE=trace,", new Position(0, 34)); + completionItems = completions.get().getLeft(); + assertEquals(4, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace,accessLog","trace,message","trace,ffdc","trace,audit"); + + completions = getCompletion("WLP_LOGGING_MESSAGE_SOURCE=trace,a", new Position(0, 35)); + completionItems = completions.get().getLeft(); + assertEquals(3, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace,accessLog","trace,audit","trace,message"); + + completions = getCompletion("WLP_LOGGING_MESSAGE_SOURCE=trace,message,", new Position(0, 42)); + completionItems = completions.get().getLeft(); + assertEquals(3, completionItems.size()); + checkCompletionsContainAllStrings(completionItems, "trace,message,accessLog","trace,message,ffdc","trace,message,audit"); + + completions = getCompletion("WLP_LOGGING_MESSAGE_SOURCE=trace,message", new Position(0, 41)); + completionItems = completions.get().getLeft(); + assertEquals(1, completionItems.size()); + + // no completion expected as there is an invalid value in the string + completions = getCompletion("WLP_LOGGING_CONSOLE_SOURCE=trace,abc,", new Position(0, 38)); + completionItems = completions.get().getLeft(); + assertEquals(0, completionItems.size()); + } + protected CompletableFuture, CompletionList>> getCompletion(String enteredText, Position position) throws URISyntaxException, InterruptedException, ExecutionException, IOException { String filename = "server.env"; File file = new File(resourcesDir, filename); diff --git a/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/BootstrapPropertyDiagnosticTest.java b/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/BootstrapPropertyDiagnosticTest.java index 6393bc95..9e0b7993 100644 --- a/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/BootstrapPropertyDiagnosticTest.java +++ b/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/BootstrapPropertyDiagnosticTest.java @@ -14,7 +14,7 @@ public class BootstrapPropertyDiagnosticTest extends AbstractDiagnosticTest { @Test public void testBootstrapProperties() throws Exception { - testDiagnostic("bootstrap.properties", 8); + testDiagnostic("bootstrap.properties", 9); checkDiagnosticsContainsAllRanges( // Checking invalid value: com.ibm.ws.logging.console.format=DEVd createRange(0, 34, 38), @@ -31,7 +31,9 @@ public void testBootstrapProperties() throws Exception { // Checking invalid package list: org.osgi.framework.bootdelegation=com.ibm.websphere,com. createRange(8, 34, 56), // Checking invalid value: com.ibm.ws.logging.console.format=DEVd - createRange(10, 34, 38) + createRange(10, 34, 38), + // Checking invalid value: com.ibm.ws.logging.console.source=trace,aud + createRange(11, 34, 43) ); checkDiagnosticsContainsMessages( "The value `DEVd` is not valid for the property `com.ibm.ws.logging.console.format`.", @@ -41,7 +43,8 @@ public void testBootstrapProperties() throws Exception { "The value `2147483648` is not within the valid range `[0..2147483647]` for the property `com.ibm.hpel.log.purgeMaxSize`.", "The value `yes` is not valid for the property `com.ibm.ws.logging.copy.system.streams`.", "This value must be a comma-delimited list of Java packages.", - "The value `DEVd` is not valid for the property `com.ibm.ws.logging.console.format`." + "The value `DEVd` is not valid for the property `com.ibm.ws.logging.console.format`.", + "The value `aud` is not valid for the property `com.ibm.ws.logging.console.source`." ); } } diff --git a/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/ServerEnvDiagnosticTest.java b/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/ServerEnvDiagnosticTest.java index 0c6df512..ee735c46 100644 --- a/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/ServerEnvDiagnosticTest.java +++ b/liberty-ls/src/test/java/io/openliberty/tools/langserver/diagnostic/ServerEnvDiagnosticTest.java @@ -16,7 +16,7 @@ public class ServerEnvDiagnosticTest extends AbstractDiagnosticTest { @Test public void testServerEnv() throws Exception { // has invalid, case-sensitive, case-insensitive, and negative port integer values. - testDiagnostic("server.env", 6); + testDiagnostic("server.env", 7); checkDiagnosticsContainsAllRanges( // Checking invalid value: WLP_LOGGING_CONSOLE_FORMAT=asdf createRange(0, 27, 31), @@ -29,7 +29,9 @@ public void testServerEnv() throws Exception { // Checking invalid whitespace after equal sign: WLP_LOGGING_MESSAGE_FORMAT= SIMPLE createRange(7,26,28), // Checking invalid case-sensitive property: WLP_LOGGING_CONSOLE_SOURCE=messagE - createRange(9, 27, 34) + createRange(9, 27, 34), + // Checking invalid case-sensitive property: WLP_LOGGING_MESSAGE_SOURCE=message,au + createRange(10, 27, 37) ); checkDiagnosticsContainsMessages( "The value `asdf` is not valid for the variable `WLP_LOGGING_CONSOLE_FORMAT`.", @@ -37,7 +39,8 @@ public void testServerEnv() throws Exception { "The value `-2` is not within the valid range `[1..65535]` for the variable `WLP_DEBUG_ADDRESS`.", "There should be no whitespace surrounding the equal sign (=).", "There should be no whitespace surrounding the equal sign (=).", - "The value `messagE` is not valid for the variable `WLP_LOGGING_CONSOLE_SOURCE`." + "The value `messagE` is not valid for the variable `WLP_LOGGING_CONSOLE_SOURCE`.", + "The value `au` is not valid for the variable `WLP_LOGGING_MESSAGE_SOURCE`." ); } } diff --git a/liberty-ls/src/test/resources/workspace/codeaction/src/main/liberty/config2/bootstrap.properties b/liberty-ls/src/test/resources/workspace/codeaction/src/main/liberty/config2/bootstrap.properties new file mode 100644 index 00000000..93aa54af --- /dev/null +++ b/liberty-ls/src/test/resources/workspace/codeaction/src/main/liberty/config2/bootstrap.properties @@ -0,0 +1 @@ +com.ibm.ws.logging.console.source=trace,message,aud \ No newline at end of file diff --git a/liberty-ls/src/test/resources/workspace/codeaction/src/main/liberty/config2/server.env b/liberty-ls/src/test/resources/workspace/codeaction/src/main/liberty/config2/server.env new file mode 100644 index 00000000..5c80ac1d --- /dev/null +++ b/liberty-ls/src/test/resources/workspace/codeaction/src/main/liberty/config2/server.env @@ -0,0 +1 @@ +WLP_LOGGING_MESSAGE_SOURCE=trace,message,au \ No newline at end of file diff --git a/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/bootstrap.properties b/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/bootstrap.properties index 64a7e2e3..00e120ac 100644 --- a/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/bootstrap.properties +++ b/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/bootstrap.properties @@ -8,4 +8,5 @@ com.ibm.hpel.log.purgeMaxSize=2147483648 com.ibm.ws.logging.copy.system.streams=yes org.osgi.framework.bootdelegation=com.ibm.websphere,com. #adding same line again to check diagnostic is coming on both lines -com.ibm.ws.logging.console.format=DEVd \ No newline at end of file +com.ibm.ws.logging.console.format=DEVd +com.ibm.ws.logging.console.source=trace,aud \ No newline at end of file diff --git a/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/server.env b/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/server.env index 6d22422e..abe2dca9 100644 --- a/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/server.env +++ b/liberty-ls/src/test/resources/workspace/diagnostic/src/main/liberty/config/server.env @@ -7,4 +7,5 @@ WLP_DEBUG_REMOTE =n WLP_LOGGING_MESSAGE_FORMAT= SIMPLE #adding same line again to check diagnostic is coming on both lines -WLP_LOGGING_CONSOLE_SOURCE=messagE \ No newline at end of file +WLP_LOGGING_CONSOLE_SOURCE=messagE +WLP_LOGGING_MESSAGE_SOURCE=message,au \ No newline at end of file