Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for platform showing invalid quick fix #311

Merged
merged 8 commits into from
Oct 22, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import java.util.List;
import java.util.Map;

import io.openliberty.tools.langserver.lemminx.codeactions.ReplacePlatform;
import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant;
import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest;
import org.eclipse.lsp4j.CodeAction;
Expand Down Expand Up @@ -58,6 +59,7 @@ private void registerCodeActions() {
codeActionParticipants.put(LibertyDiagnosticParticipant.NOT_OPTIONAL_CODE, new EditAttribute());
codeActionParticipants.put(LibertyDiagnosticParticipant.IMPLICIT_NOT_OPTIONAL_CODE, new AddAttribute());
codeActionParticipants.put(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE, new ReplaceFeature());
codeActionParticipants.put(LibertyDiagnosticParticipant.INCORRECT_PLATFORM_CODE, new ReplacePlatform());
codeActionParticipants.put(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_CODE, new AddFeature());
codeActionParticipants.put(LibertyDiagnosticParticipant.IS_FILE_NOT_DIR_CODE, new RemoveTrailingSlash());
codeActionParticipants.put(LibertyDiagnosticParticipant.Is_DIR_NOT_FILE_CODE, new AddTrailingSlash());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant {
public static final String Is_DIR_NOT_FILE_CODE = "is_dir_not_file";

public static final String INCORRECT_FEATURE_CODE = "incorrect_feature";

public static final String INCORRECT_PLATFORM_CODE = "incorrect_platform";

@Override
public void doDiagnostics(DOMDocument domDocument, List<Diagnostic> diagnostics,
XMLValidationSettings validationSettings, CancelChecker cancelChecker) {
Expand Down Expand Up @@ -386,7 +387,7 @@ private void validatePlatform(DOMDocument domDocument, List<Diagnostic> list, DO
Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), featureTextNode.getEnd(),
domDocument);
String message = "ERROR: The platform \"" + platformName + "\" does not exist.";
list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_FEATURE_CODE));
list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_PLATFORM_CODE));
}
// if this exact platform already exists, or another version of this feature already exists, then show a diagnostic
else {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*******************************************************************************
* Copyright (c) 2024 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
* http://www.eclipse.org/legal/epl-2.0.
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* IBM Corporation - initial API and implementation
*******************************************************************************/

package io.openliberty.tools.langserver.lemminx.codeactions;

import io.openliberty.tools.langserver.lemminx.LibertyExtension;
import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime;
import io.openliberty.tools.langserver.lemminx.services.FeatureService;
import io.openliberty.tools.langserver.lemminx.services.SettingsService;
import io.openliberty.tools.langserver.lemminx.util.LibertyConstants;
import io.openliberty.tools.langserver.lemminx.util.LibertyUtils;
import org.eclipse.lemminx.commons.CodeActionFactory;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant;
import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest;
import org.eclipse.lsp4j.CodeAction;
import org.eclipse.lsp4j.Diagnostic;
import org.eclipse.lsp4j.jsonrpc.CancelChecker;

import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.logging.Logger;

public class ReplacePlatform implements ICodeActionParticipant {
private static final Logger LOGGER = Logger.getLogger(LibertyExtension.class.getName());

@Override
public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeActions, CancelChecker cancelChecker) {
Diagnostic diagnostic = request.getDiagnostic();
DOMDocument document = request.getDocument();
try {
// Get a list of platforms that partially match the specified invalid platform.
// Create a code action to replace the invalid platform with each possible valid platform.
// First, get the invalid platform.
String invalidPlatform = document.findNodeAt(document.offsetAt(diagnostic.getRange().getEnd())).getTextContent();

final boolean replacePlatformName = invalidPlatform != null && !invalidPlatform.isBlank();
// strip off version number after the - so that we can provide all possible valid versions of a platform for completion
final String platformNameToReplace = replacePlatformName && invalidPlatform.contains("-") ? invalidPlatform.substring(0, invalidPlatform.lastIndexOf("-")) : invalidPlatform;

if (replacePlatformName) {
LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(document);
Set<String> allPlatforms = getAllPlatforms(runtimeInfo, document);
List<String> existingPlatforms = FeatureService.getInstance().collectExistingPlatforms(document, platformNameToReplace);
List<String> replacementPlatforms = getReplacementPlatforms(document,
platformNameToReplace, allPlatforms, existingPlatforms);
// check for conflicting platforms for already existing platforms. remove them from quick fix items
List<String> replacementPlatformsWithoutConflicts = getReplacementPlatformsWithoutConflicts(replacementPlatforms, existingPlatforms);
// filter with entered word
List<String> filteredPlatforms = replacementPlatformsWithoutConflicts.stream().
filter(it -> it.toLowerCase().contains(platformNameToReplace.toLowerCase()))
.toList();
for (String nextPlatform : filteredPlatforms) {
if (!nextPlatform.equals(platformNameToReplace)) {
String title = "Replace platform with " + nextPlatform;
codeActions.add(CodeActionFactory.replace(title, diagnostic.getRange(), nextPlatform, document.getTextDocument(), diagnostic));
}
}
}
} catch (Exception e) {
// BadLocationException not expected
LOGGER.warning("Could not generate code action for replace platform: " + e);
}
}

/**
* get list of existing platforms to exclude from list of possible replacements
* also exclude any platform with a different version that matches an existing platform
* @param document DOM document
* @param platformNameToReplace platform for replacing
* @param allPlatforms all platforms
* @return replacement platforms
*/
private static List<String> getReplacementPlatforms(DOMDocument document, String platformNameToReplace, Set<String> allPlatforms, List<String> existingPlatforms) {
List<String> existingPlatformsWithoutVersion = existingPlatforms.stream().map(p->LibertyUtils.stripVersion(p).toLowerCase()).toList();
return allPlatforms.stream().filter(
p -> !existingPlatformsWithoutVersion.contains(LibertyUtils.stripVersion(p))
).sorted(Comparator.naturalOrder()).toList();
}

/**
* find and remove conflicting platforms
* @param replacementPlatforms replacement platform list
* @param existingPlatforms existing platforms in doc
* @return non-conflicting platforms
*/
private static List<String> getReplacementPlatformsWithoutConflicts(List<String> replacementPlatforms, List<String> existingPlatforms) {
List<String> replacementPlatformsWithoutConflicts=new ArrayList<>();
replacementPlatforms.forEach(
p->{
String pWithoutVersion = LibertyUtils.stripVersion(p);

Optional<String> conflictingPlatform = existingPlatforms.stream().filter(
existingPlatform -> {
String conflictingPlatformName = LibertyConstants.conflictingPlatforms.get(pWithoutVersion);
return conflictingPlatformName != null && existingPlatform.contains(conflictingPlatformName);
}
).findFirst();
if(conflictingPlatform.isEmpty()){
replacementPlatformsWithoutConflicts.add(p);
}
}
);
return replacementPlatformsWithoutConflicts;
}

private static Set<String> getAllPlatforms(LibertyRuntime runtimeInfo, DOMDocument document) {
String libertyVersion = runtimeInfo == null ? null : runtimeInfo.getRuntimeVersion();
String libertyRuntime = runtimeInfo == null ? null : runtimeInfo.getRuntimeType();

final int requestDelay = SettingsService.getInstance().getRequestDelay();
return FeatureService.getInstance().getAllPlatforms(libertyVersion, libertyRuntime, requestDelay,
document.getDocumentURI());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.logging.Logger;

import io.openliberty.tools.langserver.lemminx.models.feature.FeatureTolerate;
import org.eclipse.lemminx.dom.DOMDocument;
import org.eclipse.lemminx.dom.DOMNode;
import org.eclipse.lemminx.uriresolver.CacheResourcesManager;
import org.eclipse.lemminx.uriresolver.CacheResourcesManager.ResourceToDeploy;
Expand Down Expand Up @@ -683,4 +684,40 @@ private void addRequiredFeatureNames(Feature feature, Set<String> requiredFeatur
requiredFeatureNames.add(extractedFeatureName);
}
}
/**
* Returns the platform names specified in the featureManager element in lower case,
* excluding the currentPlatformName if specified.
* @param document DOM document
* @param currentPlatformName current platform name
* @return all platforms in document
*/
public List<String> collectExistingPlatforms(DOMDocument document, String currentPlatformName) {
List<String> includedPlatforms = new ArrayList<>();
List<DOMNode> nodes = document.getDocumentElement().getChildren();
DOMNode featureManager = null;

for (DOMNode node : nodes) {
if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) {
featureManager = node;
break;
}
}
if (featureManager == null) {
return includedPlatforms;
}

List<DOMNode> platforms = featureManager.getChildren();
for (DOMNode platformNode : platforms) {
DOMNode platformTextNode = (DOMNode) platformNode.getChildNodes().item(0);
// skip nodes that do not have any text value (ie. comments)
if (platformNode.getNodeName().equals(LibertyConstants.PLATFORM_ELEMENT) && platformTextNode != null) {
String platformName = platformTextNode.getTextContent();
String platformNameLowerCase = platformName.toLowerCase();
if ((!platformNameLowerCase.equalsIgnoreCase(currentPlatformName))) {
includedPlatforms.add(platformNameLowerCase);
}
}
}
return includedPlatforms;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ public void testDiagnosticsForInclude() throws IOException, BadLocationException
" <include location=\"/testDir.xml\"/>", //
"</server>"
);

// Diagnostic location1 = new Diagnostic();
File serverXMLFile = new File("src/test/resources/server.xml");
assertFalse(serverXMLFile.exists());
Expand Down Expand Up @@ -231,7 +231,7 @@ public void testDiagnosticsForInclude() throws IOException, BadLocationException
fileIsDir.setCode("is_dir_not_file");
fileIsDir.setMessage("Path specified a file, but resource exists as a directory. Please add a trailing slash.");

XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLFile.toURI().toString(),
XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLFile.toURI().toString(),
not_xml, multi_liner, not_optional, missing_xml, optional_not_defined, missing_xml2,
dirIsFile, fileIsDir);

Expand All @@ -247,7 +247,7 @@ public void testDiagnosticsForInclude() throws IOException, BadLocationException
CodeAction fileIsDirCodeAction = ca(fileIsDir, fileIsDirTextEdit);


XMLAssert.testCodeActionsFor(serverXML, dirIsFile, dirIsFileCodeAction);
XMLAssert.testCodeActionsFor(serverXML, dirIsFile, dirIsFileCodeAction);

XMLAssert.testCodeActionsFor(serverXML, fileIsDir, fileIsDirCodeAction);
}
Expand All @@ -268,7 +268,7 @@ public void testDiagnosticsForIncludeWindows() throws BadLocationException {
" <include location=\"\\testDir.xml\"/>", //
"</server>"
);

// Diagnostic location1 = new Diagnostic();
File serverXMLFile = new File("src/test/resources/server.xml");
assertFalse(serverXMLFile.exists());
Expand All @@ -285,7 +285,7 @@ public void testDiagnosticsForIncludeWindows() throws BadLocationException {
fileIsDir.setCode("is_dir_not_file");
fileIsDir.setMessage("Path specified a file, but resource exists as a directory. Please add a trailing slash.");

XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLFile.toURI().toString(),
XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLFile.toURI().toString(),
dirIsFile, fileIsDir);

String fixedFilePath = "location=\"\\empty_server.xml\"";
Expand All @@ -298,7 +298,7 @@ public void testDiagnosticsForIncludeWindows() throws BadLocationException {
fileIsDir.getRange().getEnd().getLine(), fileIsDir.getRange().getEnd().getCharacter(), fixedDirPath);
CodeAction fileIsDirCodeAction = ca(fileIsDir, fileIsDirTextEdit);

XMLAssert.testCodeActionsFor(serverXML, dirIsFile, dirIsFileCodeAction);
XMLAssert.testCodeActionsFor(serverXML, dirIsFile, dirIsFileCodeAction);

XMLAssert.testCodeActionsFor(serverXML, fileIsDir, fileIsDirCodeAction);
}
Expand All @@ -307,7 +307,7 @@ public void testDiagnosticsForIncludeWindows() throws BadLocationException {
public void testConfigElementMissingFeatureManager() throws JAXBException {
assertTrue(featureList.exists());
FeatureService.getInstance().readFeaturesFromFeatureListFile(new ArrayList<Feature>(), libWorkspace, featureList);

String serverXml = "<server><ssl id=\"\"/></server>";
// Temporarily disabling config element diagnostics if featureManager element is missing (until issue 230 is addressed)
// Diagnostic config_for_missing_feature = new Diagnostic();
Expand Down Expand Up @@ -364,7 +364,7 @@ public void testConfigElementMissingFeatureUsingCachedFeaturelist() throws JAXBE

TextDocumentEdit textDoc = tde(sampleserverXMLURI, 0, texted);
WorkspaceEdit workspaceEdit = new WorkspaceEdit(Collections.singletonList(Either.forLeft(textDoc)));

invalidCodeAction.setEdit(workspaceEdit);
codeActions.add(invalidCodeAction);
}
Expand Down Expand Up @@ -445,7 +445,7 @@ public void testInvalidPlatformDiagnostic() throws BadLocationException {
);
Diagnostic invalid1 = new Diagnostic();
invalid1.setRange(r(3, 25, 3, 28));
invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE);
invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_PLATFORM_CODE);
invalid1.setMessage("ERROR: The platform \"jaX\" does not exist.");

Diagnostic invalid2 = new Diagnostic();
Expand Down Expand Up @@ -750,4 +750,63 @@ public void testUniquenessForNameChangedFeatureDiagnostic() throws BadLocationEx
XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI,
invalid);
}

@Test
public void testInvalidPlatformDiagnosticWithCodeCompletion() throws BadLocationException {

String serverXML = String.join(newLine, //
"<server description=\"Sample Liberty server\">", //
" <featureManager>", //
" <platform>javaEE</platform>", //
" </featureManager>", //
"</server>" //
);
Diagnostic invalid1 = new Diagnostic();
invalid1.setRange(r(2, 25, 2, 31));
invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_PLATFORM_CODE);
invalid1.setMessage("ERROR: The platform \"javaEE\" does not exist.");

XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI,
invalid1);
//expecting code action to show all javaee platforms ignoring input case
List<String> featuresStartWithJavaEE = new ArrayList<>();
featuresStartWithJavaEE.add("javaee-6.0");
featuresStartWithJavaEE.add("javaee-7.0");
featuresStartWithJavaEE.add("javaee-8.0");
Collections.sort(featuresStartWithJavaEE);

List<CodeAction> codeActions = new ArrayList<>();
for (String nextFeature : featuresStartWithJavaEE) {
TextEdit texted = te(invalid1.getRange().getStart().getLine(), invalid1.getRange().getStart().getCharacter(),
invalid1.getRange().getEnd().getLine(), invalid1.getRange().getEnd().getCharacter(), nextFeature);
CodeAction invalidCodeAction = ca(invalid1, texted);

codeActions.add(invalidCodeAction);
}

XMLAssert.testCodeActionsFor(serverXML, invalid1, codeActions.get(0), codeActions.get(1),
codeActions.get(2));

serverXML = String.join(newLine, //
"<server description=\"Sample Liberty server\">", //
" <featureManager>", //
" <feature>batch-1.0</feature>", //
" <platform>ja</platform>", //
" <platform>javaee-7.0</platform>", //
" </featureManager>", //
"</server>" //
);
invalid1 = new Diagnostic();
invalid1.setRange(r(3, 25, 3, 27));
invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_PLATFORM_CODE);
invalid1.setMessage("ERROR: The platform \"ja\" does not exist.");

XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI,
invalid1);
// expecting code action to show no platforms since
// 1. javaee is already included
// 2. jakartaee is conflicting with javaee platforms

XMLAssert.testCodeActionsFor(serverXML, invalid1);
}
}
Loading