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 a0da04b2..362de91b 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 @@ -38,10 +38,14 @@ import java.util.Collections; import java.util.HashSet; import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.logging.Logger; import java.util.stream.Collectors; +import static io.openliberty.tools.langserver.lemminx.util.LibertyConstants.changedFeatureNameDiagMessage; + public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant { private static final Logger LOGGER = Logger.getLogger(LibertyDiagnosticParticipant.class.getName()); @@ -117,6 +121,7 @@ private void validateFeaturesAndPlatforms(DOMDocument domDocument, List versionedFeatures = new HashSet(); Set preferredPlatforms = new HashSet(); Set preferredPlatformsWithoutVersion = new HashSet(); + Set featureList = new HashSet(); // Search for duplicate features // or features that do not exist List features = featureManager.getChildren(); @@ -126,13 +131,13 @@ private void validateFeaturesAndPlatforms(DOMDocument domDocument, List list, Set includedFeatures, DOMNode featureTextNode, String libertyVersion, String libertyRuntime, int requestDelay, Set versionedFeatures, Set versionlessFeatures, Set featuresWithoutVersions) { + private void validateFeature(DOMDocument domDocument, List list, Set includedFeatures, DOMNode featureTextNode, String libertyVersion, String libertyRuntime, int requestDelay, Set versionedFeatures, Set versionlessFeatures, Set featuresWithoutVersions, Set featureList) { // skip nodes that do not have any text value (ie. comments) if (featureTextNode != null && featureTextNode.getTextContent() != null) { String featureName = featureTextNode.getTextContent().trim(); @@ -144,7 +149,7 @@ private void validateFeature(DOMDocument domDocument, List list, Set String message = "ERROR: The feature \"" + featureName + "\" does not exist."; list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_FEATURE_CODE)); } else { - checkForFeatureUniqueness(domDocument, list, includedFeatures, featureTextNode, versionedFeatures, versionlessFeatures, featuresWithoutVersions, featureName); + checkForFeatureUniqueness(domDocument, list, includedFeatures, featureTextNode, versionedFeatures, versionlessFeatures, featuresWithoutVersions, featureName, featureList); } } } @@ -152,6 +157,7 @@ private void validateFeature(DOMDocument domDocument, List list, Set /** * check whether feature name is unique * throw error if another version of same feature exists as well + * * @param domDocument * @param list * @param includedFeatures @@ -160,17 +166,35 @@ private void validateFeature(DOMDocument domDocument, List list, Set * @param versionlessFeatures * @param featuresWithoutVersions * @param featureName + * @param featureList */ - private void checkForFeatureUniqueness(DOMDocument domDocument, List list, Set includedFeatures, DOMNode featureTextNode, Set versionedFeatures, Set versionlessFeatures, Set featuresWithoutVersions, String featureName) { + private void checkForFeatureUniqueness(DOMDocument domDocument, List list, Set includedFeatures, DOMNode featureTextNode, Set versionedFeatures, Set versionlessFeatures, Set featuresWithoutVersions, String featureName, Set featureList) { String featureNameLower = featureName.toLowerCase(); - String featureNameNoVersionLower=featureNameLower; + String featureNameNoVersionLower; if(featureNameLower.contains("-")){ featureNameNoVersionLower = featureNameLower.substring(0, featureNameLower.lastIndexOf("-")); versionedFeatures.add(featureName); }else{ + featureNameNoVersionLower = featureNameLower; versionlessFeatures.add(featureName); } + String featureNameNoVersion = LibertyUtils.stripVersion(featureName); + Map changedFeatureNameMapLower = LibertyConstants.changedFeatureNameMap.entrySet().parallelStream().collect( + Collectors.toMap(entry -> entry.getKey().toLowerCase(), + entry -> entry.getValue().toLowerCase())); + Map changedFeatureNameMapLowerReversed = + changedFeatureNameMapLower.entrySet() + .stream() + .collect(Collectors.toMap(Map.Entry::getValue, Map.Entry::getKey)); + Set featuresWithOldNames = featuresWithoutVersions.stream().map( + v -> changedFeatureNameMapLower.get(v + "-")) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + Set featuresWithChangedNames = featuresWithoutVersions.stream().map( + v -> changedFeatureNameMapLowerReversed.get(v + "-")) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); // if this exact feature already exists, or another version of this feature already exists, then show a diagnostic if (includedFeatures.contains(featureNameLower)) { Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), @@ -180,11 +204,28 @@ private void checkForFeatureUniqueness(DOMDocument domDocument, List } else if (featuresWithoutVersions.contains(featureNameNoVersionLower)) { Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), featureTextNode.getEnd(), domDocument); - String featureNameNoVersion = LibertyUtils.stripVersion(featureName); String message = "ERROR: More than one version of feature " + featureNameNoVersion + " is included. Only one version of a feature may be specified."; list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE)); + } else if (featuresWithOldNames.contains(featureNameNoVersionLower + "-")) { + String otherFeatureName = getOtherFeatureName(featureList, changedFeatureNameMapLowerReversed, featureNameNoVersionLower); + //check for features whose name is changed such as jsp is changed to pages + Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), + featureTextNode.getEnd(), domDocument); + String message = String.format(changedFeatureNameDiagMessage, featureName, otherFeatureName, + LibertyUtils.stripVersion(otherFeatureName), + LibertyUtils.stripVersion(featureName)); + list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE)); + } else if (featuresWithChangedNames.contains(featureNameNoVersionLower + "-")) { + String otherFeatureName = getOtherFeatureName(featureList, changedFeatureNameMapLower, featureNameNoVersionLower); + Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), + featureTextNode.getEnd(), domDocument); + String message = String.format(changedFeatureNameDiagMessage, featureName, otherFeatureName, + LibertyUtils.stripVersion(featureName), + LibertyUtils.stripVersion(otherFeatureName)); + list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE)); } includedFeatures.add(featureNameLower); + featureList.add(featureName); featuresWithoutVersions.add(featureNameNoVersionLower); } @@ -520,4 +561,20 @@ private void checkForVersionlessPlatforms(DOMDocument domDocument, list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_FEATURE_CODE)); } } + + /** + * Get conflicting featurename for any feature + * @param includedFeatures + * @param changedFeatureNameMap + * @param featureNameNoVersionLower + * @return + */ + private static String getOtherFeatureName(Set includedFeatures, Map changedFeatureNameMapLowerReversed, String featureNameNoVersionLower) { + String otherFeatureNameWithoutVersion = changedFeatureNameMapLowerReversed.get(featureNameNoVersionLower + "-"); + String otherFeatureName = includedFeatures + .stream() + .filter(f -> f.toLowerCase().contains(LibertyUtils.stripVersion(otherFeatureNameWithoutVersion))) + .findFirst().orElse(null); + return otherFeatureName; + } } 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 7585e759..dcec096a 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 @@ -71,4 +71,7 @@ private LibertyConstants() { put("wasJmsClient-", "messagingClient-"); put("wasJmsSecurity-", "messagingSecurity-"); }}); + + public static String changedFeatureNameDiagMessage="ERROR: The %s feature cannot be configured with the %s feature because they are two different versions of the same feature. " + + "The feature name changed from %s to %s for Jakarta EE. Remove one of the features."; } diff --git a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java index f785de0e..0bd3d8cb 100644 --- a/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java +++ b/lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java @@ -687,4 +687,67 @@ public void testConfigElementVersionLess() throws JAXBException { configForMissingFeature.setMessage(LibertyDiagnosticParticipant.MISSING_CONFIGURED_FEATURE_MESSAGE); XMLAssert.testDiagnosticsFor(serverXML1, null, null, serverXMLURI,configForMissingFeature); } + + @Test + public void testUniquenessForNameChangedFeatureDiagnostic() throws BadLocationException { + String serverXML = String.join(newLine, // + "", // + " ", // + " jms-2.0", // + " messaging-3.0", // + " ", // + "" // + ); + Diagnostic invalid = new Diagnostic(); + invalid.setRange(r(3, 24, 3, 37)); + invalid.setMessage("ERROR: The messaging-3.0 feature cannot be configured with the jms-2.0 feature because they are two different versions of the same feature. The feature name changed from jms to messaging for Jakarta EE. Remove one of the features."); + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, + invalid); + + serverXML = String.join(newLine, // + "", // + " ", // + " messaging-3.0", // + " jms-2.0", // + " ", // + "" // + ); + invalid = new Diagnostic(); + invalid.setRange(r(3, 24, 3, 31)); + invalid.setMessage("ERROR: The jms-2.0 feature cannot be configured with the messaging-3.0 feature because they are two different versions of the same feature. The feature name changed from jms to messaging for Jakarta EE. Remove one of the features."); + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, + invalid); + + serverXML = String.join(newLine, // + "", // + " ", // + " javaee-7.0", // + " enterpriseBeans-4.0", // + " ejb-3.2", // + " ", // + + "" // + ); + invalid = new Diagnostic(); + invalid.setRange(r(4, 24, 4, 31)); + invalid.setMessage("ERROR: The ejb-3.2 feature cannot be configured with the enterpriseBeans-4.0 feature because they are two different versions of the same feature. The feature name changed from ejb to enterpriseBeans for Jakarta EE. Remove one of the features."); + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, + invalid); + + serverXML = String.join(newLine, // + "", // + " ", // + " javaee-7.0", // + " enterpriseBeans", // + " ejb", // + " ", // + + "" // + ); + invalid = new Diagnostic(); + invalid.setRange(r(4, 24, 4, 27)); + invalid.setMessage("ERROR: The ejb feature cannot be configured with the enterpriseBeans feature because they are two different versions of the same feature. The feature name changed from ejb to enterpriseBeans for Jakarta EE. Remove one of the features."); + XMLAssert.testDiagnosticsFor(serverXML, null, null, serverXMLURI, + invalid); + } } \ No newline at end of file diff --git a/liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyTextDocumentService.java b/liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyTextDocumentService.java index 86aa76d6..f015fac4 100644 --- a/liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyTextDocumentService.java +++ b/liberty-ls/src/main/java/io/openliberty/tools/langserver/LibertyTextDocumentService.java @@ -29,35 +29,17 @@ import java.util.logging.Logger; import java.util.stream.Collectors; -import org.eclipse.lsp4j.CodeAction; -import org.eclipse.lsp4j.CodeActionParams; -import org.eclipse.lsp4j.CodeLens; -import org.eclipse.lsp4j.CodeLensParams; -import org.eclipse.lsp4j.Command; import org.eclipse.lsp4j.CompletionItem; import org.eclipse.lsp4j.CompletionList; import org.eclipse.lsp4j.CompletionParams; -import org.eclipse.lsp4j.DefinitionParams; import org.eclipse.lsp4j.Diagnostic; -import org.eclipse.lsp4j.DocumentFormattingParams; -import org.eclipse.lsp4j.DocumentHighlight; -import org.eclipse.lsp4j.DocumentHighlightParams; -import org.eclipse.lsp4j.DocumentRangeFormattingParams; -import org.eclipse.lsp4j.DocumentSymbol; -import org.eclipse.lsp4j.DocumentSymbolParams; import org.eclipse.lsp4j.Hover; import org.eclipse.lsp4j.HoverParams; -import org.eclipse.lsp4j.Location; -import org.eclipse.lsp4j.LocationLink; import org.eclipse.lsp4j.PublishDiagnosticsParams; -import org.eclipse.lsp4j.SymbolInformation; -import org.eclipse.lsp4j.TextDocumentItem; -import org.eclipse.lsp4j.TextEdit; import org.eclipse.lsp4j.jsonrpc.messages.Either; import io.openliberty.tools.langserver.completion.LibertyPropertiesCompletionProvider; import io.openliberty.tools.langserver.diagnostic.DiagnosticRunner; -import io.openliberty.tools.langserver.diagnostic.LibertyPropertiesDiagnosticService; import io.openliberty.tools.langserver.hover.LibertyPropertiesHoverProvider; public class LibertyTextDocumentService implements TextDocumentService { @@ -81,6 +63,9 @@ public LibertyTextDocument getOpenedDocument(String uri) { public void didOpen(DidOpenTextDocumentParams params) { LibertyTextDocument document = documents.onDidOpenTextDocument(params); String uri = document.getUri(); + if (uri == null) { + LOGGER.severe("Liberty text document URI is null for " + params); + } validate(Arrays.asList(uri)); new DiagnosticRunner(libertyLanguageServer).compute(params); } @@ -89,6 +74,9 @@ public void didOpen(DidOpenTextDocumentParams params) { public void didChange(DidChangeTextDocumentParams params) { LibertyTextDocument document = documents.onDidChangeTextDocument(params); String uri = document.getUri(); + if (uri == null) { + LOGGER.severe("Liberty text document URI is null for " + params); + } validate(Arrays.asList(uri)); new DiagnosticRunner(libertyLanguageServer).compute(params); } @@ -97,6 +85,9 @@ public void didChange(DidChangeTextDocumentParams params) { public void didClose(DidCloseTextDocumentParams params) { documents.onDidCloseTextDocument(params); String uri = params.getTextDocument().getUri(); + if (uri == null) { + LOGGER.severe("Liberty text document URI is null for " + params); + } libertyLanguageServer.getLanguageClient() .publishDiagnostics(new PublishDiagnosticsParams(uri, new ArrayList())); } @@ -117,8 +108,16 @@ private void validateAll() { @Override public CompletableFuture hover(HoverParams hoverParams) { String uri = hoverParams.getTextDocument().getUri(); + if (uri == null) { + LOGGER.severe("Liberty text document URI is null for " + hoverParams); + } LibertyTextDocument textDocumentItem = documents.get(uri); - return new LibertyPropertiesHoverProvider(textDocumentItem).getHover(hoverParams.getPosition()); + if (textDocumentItem != null) { + return new LibertyPropertiesHoverProvider(textDocumentItem).getHover(hoverParams.getPosition()); + } else { + LOGGER.severe("The document with uri " + uri + " has not been found in opened documents. Cannot provide hover."); + return CompletableFuture.completedFuture(new Hover()); + } } @Override