From ed448287404816676a458286953b6d7e5354cd3a Mon Sep 17 00:00:00 2001 From: Arun Venmany Date: Tue, 27 Aug 2024 18:21:50 +0530 Subject: [PATCH] Versionless feature changes -more review comments Signed-off-by: Arun Venmany --- .../lemminx/LibertyCompletionParticipant.java | 12 +++---- .../lemminx/LibertyDiagnosticParticipant.java | 31 ++++++++----------- .../lemminx/LibertyHoverParticipant.java | 11 +++---- .../lemminx/codeactions/ReplaceFeature.java | 7 +++-- .../lemminx/services/FeatureService.java | 4 +-- .../lemminx/util/LibertyConstants.java | 10 +++--- .../langserver/lemminx/util/LibertyUtils.java | 16 ++++++++++ .../io/openliberty/LibertyDiagnosticTest.java | 25 +++++++++++++-- 8 files changed, 69 insertions(+), 47 deletions(-) diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java index eb3a6ecd..506dc6da 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCompletionParticipant.java @@ -120,18 +120,14 @@ private void buildPlatformCompletionItems(ICompletionRequest request, ICompletio request.getXMLDocument().getDocumentURI()); platforms.stream().filter(it->(Objects.isNull(platformName) || it.contains(platformName))) .filter(p -> !existingPlatforms.contains(LibertyUtils.stripVersion(p).toLowerCase())) - .forEach(item -> { + .forEach(platformItem -> { Range range = XMLPositionUtility.createRange(parentElement.getStartTagCloseOffset() + 1, parentElement.getEndTagOpenOffset(), request.getXMLDocument()); - Either edit = Either.forLeft(new TextEdit(range, item)); + Either edit = Either.forLeft(new TextEdit(range, platformItem)); CompletionItem completionItem = new CompletionItem(); - completionItem.setLabel(item); + completionItem.setLabel(platformItem); completionItem.setTextEdit(edit); - String platformNameNoVersion = LibertyUtils.stripVersion(item).toLowerCase(); - if (LibertyConstants.platformDescriptionMap.containsKey(platformNameNoVersion)) { - String version = LibertyUtils.getVersion(item); - completionItem.setDocumentation(String.format(LibertyConstants.platformDescriptionMap.get(platformNameNoVersion),version)); - } + completionItem.setDocumentation(LibertyUtils.getPlatformDescription(platformItem)); response.addCompletionItem(completionItem); }); } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java index ba7bacb8..6f728dd1 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java @@ -456,7 +456,7 @@ private void validateVersionLessFeatures(DOMDocument domDocument, List commonPlatforms = FeatureService.getInstance() @@ -475,38 +475,33 @@ else if(commonPlatforms.size()>1) { list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_FEATURE_CODE)); } else{ - checkForVersionlessPlatforms(domDocument, list, commonPlatforms, versionLessFeatureTextNode, featureName); + checkForVersionlessPlatforms(domDocument, list, commonPlatforms, versionLessFeatureTextNode, featureName, false); } } if (!preferredPlatforms.isEmpty()) { - checkForVersionlessPlatforms(domDocument, list, preferredPlatforms, versionLessFeatureTextNode, featureName); + checkForVersionlessPlatforms(domDocument, list, preferredPlatforms, versionLessFeatureTextNode, featureName,true); } } - /** - * check all platforms for versionless features - * @param domDocument - * @param list - * @param preferredPlatforms - * @param versionLessFeatureTextNode - * @param featureName - * @param libertyVersion - * @param libertyRuntime - * @param requestDelay - */ + private void checkForVersionlessPlatforms(DOMDocument domDocument, - List list, Set preferredPlatforms, - DOMNode versionLessFeatureTextNode, String featureName) { + List list, Set platformsToCompare, + DOMNode versionLessFeatureTextNode, String featureName, boolean isPlatformFromXml) { LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(domDocument); String libertyVersion = runtimeInfo == null ? null : runtimeInfo.getRuntimeVersion(); String libertyRuntime = runtimeInfo == null ? null : runtimeInfo.getRuntimeType(); final int requestDelay = SettingsService.getInstance().getRequestDelay(); Set allPlatforrmsForVersionLess = FeatureService.getInstance() .getAllPlatformsForVersionLessFeature(featureName, libertyVersion, libertyRuntime, requestDelay, domDocument.getDocumentURI()); - if (Sets.intersection(allPlatforrmsForVersionLess, preferredPlatforms).isEmpty() ){ + if (Sets.intersection(allPlatforrmsForVersionLess, platformsToCompare).isEmpty()) { + String message; + if (isPlatformFromXml) { + message = "ERROR: The \"" + featureName + "\" versionless feature does not have a configured platform."; + } else { + message = "ERROR: The \"" + featureName + "\" versionless feature configured platforms cannot be resolved with selected versioned features. Specify a platform or a feature with a version to enable resolution."; + } Range range = XMLPositionUtility.createRange(versionLessFeatureTextNode.getStart(), versionLessFeatureTextNode.getEnd(), domDocument); - String message = "ERROR: The \"" + featureName + "\" versionless feature cannot be resolved. Specify a platform or a feature with a version to enable resolution."; list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_FEATURE_CODE)); } } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java index e70bb489..1e0bdf0d 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyHoverParticipant.java @@ -80,17 +80,14 @@ public Hover onText(IHoverRequest request, CancelChecker cancelChecker) { /** * get description for platform from the feature json * there will be a feature with same shortname as platform. + * * @param platformName platform name * @return hover */ private Hover getHoverPlatformDescription(String platformName) { - String platformNameLowerCase = platformName.toLowerCase(); - // strip off version number after the - so that we can provide all possible valid versions of a platform for completion - String platformNameToCompare = platformNameLowerCase.contains("-") ? platformNameLowerCase.substring(0, platformNameLowerCase.lastIndexOf("-")) : platformNameLowerCase; - String version = platformNameLowerCase.contains("-") ? platformNameLowerCase.substring(platformNameLowerCase.lastIndexOf("-") + 1) : ""; - if (LibertyConstants.platformDescriptionMap.containsKey(platformNameToCompare)) { - return new Hover(new MarkupContent("plaintext", - String.format(LibertyConstants.platformDescriptionMap.get(platformNameToCompare), version))); + String description = LibertyUtils.getPlatformDescription(platformName); + if (description != null) { + return new Hover(new MarkupContent("plaintext", description)); } return null; } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceFeature.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceFeature.java index eebc4218..b5f67510 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceFeature.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/ReplaceFeature.java @@ -73,9 +73,10 @@ public void doCodeAction(ICodeActionRequest request, List codeAction Collections.sort(replacementFeatureNames); // sort these so they appear in alphabetical order in quick fixes - also helps the test case pass reliably for (String nextFeature : replacementFeatureNames) { - String title = "Replace feature with "+nextFeature; - - codeActions.add(CodeActionFactory.replace(title, diagnostic.getRange(), nextFeature, document.getTextDocument(), diagnostic)); + if (!nextFeature.equals(featureNameToReplace)) { + String title = "Replace feature with " + nextFeature; + codeActions.add(CodeActionFactory.replace(title, diagnostic.getRange(), nextFeature, document.getTextDocument(), diagnostic)); + } } } } catch (Exception e) { 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 578cc75d..353fdb24 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 @@ -271,7 +271,7 @@ public List getFeatureReplacements(String featureName, DOMNode featureM if (existingFeatures.contains(nextFeatureName)) { // collect feature name minus version number to know which other features to exclude String featureNameMinusVersion = nextFeatureName.contains("-") ? - nextFeatureName.substring(0, nextFeatureName.lastIndexOf("-") + 1) : nextFeatureName; + nextFeatureName.substring(0, nextFeatureName.lastIndexOf("-") + 1) : nextFeatureName + "-"; featuresWithoutVersionsToExclude.add(featureNameMinusVersion); } } @@ -283,7 +283,7 @@ public List getFeatureReplacements(String featureName, DOMNode featureM String nextFeatureName = featureNamesLowerCase.get(i); if (nextFeatureName.contains(featureNameLowerCase)) { String comparingFeatureName = nextFeatureName.contains("-") ? - nextFeatureName.substring(0, nextFeatureName.lastIndexOf("-") + 1) : nextFeatureName+"-"; + nextFeatureName.substring(0, nextFeatureName.lastIndexOf("-") + 1) : nextFeatureName + "-"; if (!featuresWithoutVersionsToExclude.contains(comparingFeatureName)) { replacementFeatures.add(features.get(i)); } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java index 9ee0ba3a..7585e759 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyConstants.java @@ -48,12 +48,11 @@ private LibertyConstants() { }}); public static final Map changedFeatureNameMap = Collections.unmodifiableMap(new HashMap<>() {{ - put("jsp-", "pages-"); put("jaspic-", "appAuthentication-"); put("jacc-", "appAuthorization-"); put("jca-", "connectors-"); put("jcaInboundSecurity-", "connectorsInboundSecurity-"); - put("ejb-", "enterprisebeans-"); + put("ejb-", "enterpriseBeans-"); put("ejbHome-", "enterpriseBeansHome-"); put("ejbLite-", "enterpriseBeansLite-"); put("ejbPersistentTimer-", "enterpriseBeansPersistentTimer-"); @@ -62,15 +61,14 @@ private LibertyConstants() { put("jsf-", "faces-"); put("javaMail-", "mail-"); put("jms-", "messaging-"); - put("jpa-", "enterprisebeans-"); - put("ejb-", "persistence-"); + put("jpa-", "persistence-"); put("jaxrs-", "restfulWS-"); put("jaxrsClient-", "restfulWSClient-"); + put("jsp-", "pages-"); put("jaxb-", "xmlBinding-"); put("jaxws-", "xmlWS-"); put("wasJmsServer-", "messagingServer-"); - put("wasJmsClient-", "enterprisebeans-"); - put("ejb-", "messagingClient-"); + put("wasJmsClient-", "messagingClient-"); put("wasJmsSecurity-", "messagingSecurity-"); }}); } diff --git a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java index fd8a456c..078379f0 100644 --- a/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java +++ b/lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/util/LibertyUtils.java @@ -40,6 +40,7 @@ import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager; import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace; import io.openliberty.tools.langserver.lemminx.services.SettingsService; +import org.eclipse.lsp4j.CompletionItem; public class LibertyUtils { @@ -558,4 +559,19 @@ public static String stripVersion(String stringWithVersion) { return stringWithVersion.contains("-") ? stringWithVersion.substring(0, stringWithVersion.lastIndexOf("-")) : stringWithVersion; } + + + /** + * get platform description + * @param platformItem platform item + * @return description + */ + public static String getPlatformDescription(String platformItem) { + String platformNameNoVersion = LibertyUtils.stripVersion(platformItem).toLowerCase(); + if (LibertyConstants.platformDescriptionMap.containsKey(platformNameNoVersion)) { + String version = getVersion(platformItem); + return String.format(LibertyConstants.platformDescriptionMap.get(platformNameNoVersion), version); + } + return null; + } } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index f0a0e5a1..05ba409e 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -478,7 +478,7 @@ public void testInvalidPlatformForVersionlessFeatureDiagnostic() throws BadLocat Diagnostic invalid1 = new Diagnostic(); invalid1.setRange(r(2, 24, 2, 31)); invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE); - invalid1.setMessage("ERROR: Need to specify any platform or at least one versioned feature."); + invalid1.setMessage("ERROR: The servlet versionless feature cannot be resolved. Specify a platform or a feature with a version to enable resolution."); XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, invalid1); @@ -546,7 +546,7 @@ public void testUnresolvedPlatformForVersionlessFeatureDiagnostic() throws BadLo Diagnostic invalid1 = new Diagnostic(); invalid1.setRange(r(2, 24, 2, 31)); invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE); - invalid1.setMessage("ERROR: The \"servlet\" versionless feature cannot be resolved. Specify a platform or a feature with a version to enable resolution."); + invalid1.setMessage("ERROR: The \"servlet\" versionless feature configured platforms cannot be resolved with selected versioned features. Specify a platform or a feature with a version to enable resolution."); XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, invalid1); @@ -563,7 +563,7 @@ public void testUnresolvedPlatformForVersionlessFeatureDiagnostic() throws BadLo invalid1 = new Diagnostic(); invalid1.setRange(r(2, 24, 2, 27)); invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE); - invalid1.setMessage("ERROR: The \"jsp\" versionless feature cannot be resolved. Specify a platform or a feature with a version to enable resolution."); + invalid1.setMessage("ERROR: The \"jsp\" versionless feature does not have a configured platform."); XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, invalid1); @@ -581,6 +581,25 @@ public void testUnresolvedPlatformForVersionlessFeatureDiagnostic() throws BadLo invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE); invalid1.setMessage("ERROR: The \"servlet\" versionless feature cannot be resolved since there are more than one common platform. Specify a platform or a feature with a version to enable resolution."); + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, + invalid1); + + // mpConfig-3.0 and mpJwt-2.0 have a single common platform(microProfile-5.0) but servlet feature does not support that + serverXML = String.join(newLine, // + "", // + " ", // + " servlet", // + " mpConfig-3.0", // + " mpJwt-2.0", // + " ", // + "" // + ); + + invalid1 = new Diagnostic(); + invalid1.setRange(r(2, 24, 2, 31)); + invalid1.setCode(LibertyDiagnosticParticipant.INCORRECT_FEATURE_CODE); + invalid1.setMessage("ERROR: The \"servlet\" versionless feature configured platforms cannot be resolved with selected versioned features. Specify a platform or a feature with a version to enable resolution."); + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, invalid1); }