Skip to content

Commit

Permalink
Fix for platform showing invalid quick fix (#311)
Browse files Browse the repository at this point in the history
* Fix for platform showing invalid quick fix

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

* added filters in platform quick fix to remove all existing platforms and conflicting platforms

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

* removed code to show all available platforms if entered key is not matching

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

* checked for lowercase issues and re organized lowercase code. added tests for microprofile

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

* correcting copyrights

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

* adding more code based on review comments

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

* adding more comments on tests

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

* adding more comments on tests

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>

---------

Signed-off-by: Arun Venmany <Arun.Kumar.V.N@ibm.com>
  • Loading branch information
arunvenmany-ibm authored Oct 22, 2024
1 parent 08ee52a commit d0c888e
Show file tree
Hide file tree
Showing 5 changed files with 294 additions and 12 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2022, 2023 IBM Corporation and others.
* Copyright (c) 2022, 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
Expand All @@ -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,126 @@
/*******************************************************************************
* 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(
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 allPlatforms all platforms
* @return replacement platforms
*/
private static List<String> getReplacementPlatforms(Set<String> allPlatforms, List<String> existingPlatforms) {
List<String> existingPlatformsWithoutVersionLowerCase = existingPlatforms.stream().map(p->LibertyUtils.stripVersion(p).toLowerCase()).toList();
return allPlatforms.stream().filter(
p -> !existingPlatformsWithoutVersionLowerCase.contains(LibertyUtils.stripVersion(p.toLowerCase()))
).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.toLowerCase());

Optional<String> conflictingPlatform = existingPlatforms.stream().filter(
existingPlatform -> {
String conflictingPlatformName = LibertyConstants.conflictingPlatforms.get(pWithoutVersion);
return conflictingPlatformName != null && existingPlatform.toLowerCase().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;
}
}
Loading

0 comments on commit d0c888e

Please sign in to comment.