Diagnostics and Quick fixes for lost config elements

.github/workflows/build.yml

@@ -39,7 +39,7 @@ jobs:
           java-version: 17
       - name: Build Lemminx Liberty
         working-directory: ./lemminx-liberty
-        run: ./mvnw clean package -ntp
+        run: ./mvnw clean package -ntp -DskipTests
       - name: Test Lemminx Liberty
         working-directory: ./lemminx-liberty
         run: ./mvnw verify -ntp

lemminx-liberty/pom.xml

@@ -4,7 +4,7 @@
     <groupId>io.openliberty.tools</groupId>
     <artifactId>liberty-langserver-lemminx</artifactId>
     <packaging>jar</packaging>
-    <version>2.0.2-SNAPSHOT</version>
+    <version>2.1-SNAPSHOT</version>
 
     <name>lemminx-liberty</name>
     <url>https://github.com/OpenLiberty/liberty-language-server</url>
@@ -55,6 +55,15 @@
                     <target>17</target>
                 </configuration>
             </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <version>3.1.2</version>
+            </plugin>
+            <plugin>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <version>3.1.2</version>
+            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-shade-plugin</artifactId>

lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyCodeActionParticipant.java

@@ -24,6 +24,7 @@
 import org.eclipse.lsp4j.jsonrpc.CancelChecker;
 
 import io.openliberty.tools.langserver.lemminx.codeactions.AddAttribute;
+import io.openliberty.tools.langserver.lemminx.codeactions.AddFeature;
 import io.openliberty.tools.langserver.lemminx.codeactions.CreateFile;
 import io.openliberty.tools.langserver.lemminx.codeactions.EditAttribute;
 import io.openliberty.tools.langserver.lemminx.codeactions.ReplaceFeature;
@@ -55,6 +56,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.MISSING_CONFIGURED_FEATURE_CODE, new AddFeature());
         }
     }
 }

lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/LibertyDiagnosticParticipant.java

@@ -23,27 +23,41 @@
 import org.eclipse.lsp4j.Range;
 import org.eclipse.lsp4j.jsonrpc.CancelChecker;
 
+import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph;
 import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime;
 import io.openliberty.tools.langserver.lemminx.services.FeatureService;
+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 io.openliberty.tools.langserver.lemminx.util.*;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.logging.Logger;
 
 public class LibertyDiagnosticParticipant implements IDiagnosticsParticipant {
+    private static final Logger LOGGER = Logger.getLogger(LibertyDiagnosticParticipant.class.getName());
+
+    public static final String LIBERTY_LEMMINX_SOURCE = "liberty-lemminx";
+
     public static final String MISSING_FILE_MESSAGE = "The resource at the specified location could not be found.";
     public static final String MISSING_FILE_CODE = "missing_file";
 
+    public static final String MISSING_CONFIGURED_FEATURE_MESSAGE = "This config element does not relate to a feature configured in the featureManager. Remove this element or add a relevant feature.";
+    public static final String MISSING_CONFIGURED_FEATURE_CODE = "lost_config_element";
+
     public static final String NOT_OPTIONAL_MESSAGE = "The specified resource cannot be skipped. Check location value or set optional to true.";
     public static final String NOT_OPTIONAL_CODE = "not_optional";
     public static final String IMPLICIT_NOT_OPTIONAL_MESSAGE = "The specified resource cannot be skipped. Check location value or add optional attribute.";
     public static final String IMPLICIT_NOT_OPTIONAL_CODE = "implicit_not_optional";
 
     public static final String INCORRECT_FEATURE_CODE = "incorrect_feature";
+
+    private Set<String> includedFeatures;
 
     @Override
     public void doDiagnostics(DOMDocument domDocument, List<Diagnostic> diagnostics,
@@ -53,22 +67,29 @@ public void doDiagnostics(DOMDocument domDocument, List<Diagnostic> diagnostics,
         try {
             validateDom(domDocument, diagnostics);
         } catch (IOException e) {
-            System.err.println("Error validating document " + domDocument.getDocumentURI());
-            System.err.println(e.getMessage());
+            LOGGER.severe("Error validating document " + domDocument.getDocumentURI());
+            LOGGER.severe(e.getMessage());
         }
     }
 
-    private void validateDom(DOMDocument domDocument, List<Diagnostic> list) throws IOException {
+    private void validateDom(DOMDocument domDocument, List<Diagnostic> diagnosticsList) throws IOException {
         List<DOMNode> nodes = domDocument.getDocumentElement().getChildren();
-
+        List<Diagnostic> tempDiagnosticsList = new ArrayList<Diagnostic>();
+        includedFeatures = new HashSet<>();
+        LibertyWorkspace workspace = LibertyProjectsManager.getInstance().getWorkspaceFolder(domDocument.getDocumentURI());
+        // TODO: Consider adding a cached feature list onto repo to optimize
+        FeatureListGraph featureGraph = (workspace == null) ? new FeatureListGraph() : workspace.getFeatureListGraph();
         for (DOMNode node : nodes) {
-            if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) {
-                validateFeature(domDocument, list, node);
-            } else if (LibertyConstants.INCLUDE_ELEMENT.equals(node.getNodeName())) {
-                validateIncludeLocation(domDocument, list, node);
+            String nodeName = node.getNodeName();
+            if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(nodeName)) {
+                validateFeature(domDocument, diagnosticsList, node);
+            } else if (LibertyConstants.INCLUDE_ELEMENT.equals(nodeName)) {
+                validateIncludeLocation(domDocument, diagnosticsList, node);
+            } else if (featureGraph.isConfigElement(nodeName)) {    // defaults to false
+                holdConfigElement(domDocument, node, tempDiagnosticsList);
             }
         }
-
+        validateConfigElements(diagnosticsList, tempDiagnosticsList, featureGraph);
     }
 
     private void validateFeature(DOMDocument domDocument, List<Diagnostic> list, DOMNode featureManager) {
@@ -80,7 +101,6 @@ private void validateFeature(DOMDocument domDocument, List<Diagnostic> list, DOM
 
         // Search for duplicate features
         // or features that do not exist
-        Set<String> includedFeatures = new HashSet<>();
         List<DOMNode> features = featureManager.getChildren();
         for (DOMNode featureNode : features) {
             DOMNode featureTextNode = (DOMNode) featureNode.getChildNodes().item(0);
@@ -93,13 +113,13 @@ private void validateFeature(DOMDocument domDocument, List<Diagnostic> list, DOM
                     Range range = XMLPositionUtility.createRange(featureTextNode.getStart(), featureTextNode.getEnd(),
                             domDocument);
                     String message = "ERROR: The feature \"" + featureName + "\" does not exist.";
-                    list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, "liberty-lemminx", INCORRECT_FEATURE_CODE));
+                    list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, INCORRECT_FEATURE_CODE));
                 } else {
                     if (includedFeatures.contains(featureName)) {
                         Range range = XMLPositionUtility.createRange(featureTextNode.getStart(),
                                 featureTextNode.getEnd(), domDocument);
                         String message = "ERROR: " + featureName + " is already included.";
-                        list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, "liberty-lemminx"));
+                        list.add(new Diagnostic(range, message, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE));
                     } else {
                         includedFeatures.add(featureName);
                     }
@@ -117,7 +137,7 @@ private void validateFeature(DOMDocument domDocument, List<Diagnostic> list, DOM
      * 2) performed in isConfigXMLFile
      * 4) not yet implemented/determined
      */
-    private void validateIncludeLocation(DOMDocument domDocument, List<Diagnostic> list, DOMNode node) {
+    private void validateIncludeLocation(DOMDocument domDocument, List<Diagnostic> diagnosticsList, DOMNode node) {
         String locAttribute = node.getAttribute("location");
         if (locAttribute == null) {
             return;
@@ -131,7 +151,7 @@ private void validateIncludeLocation(DOMDocument domDocument, List<Diagnostic> l
         Range range = XMLPositionUtility.createRange(locNode.getStart(), locNode.getEnd(), domDocument);
         if (!locAttribute.endsWith(".xml")) {
             String message = "The specified resource is not an XML file.";
-            list.add(new Diagnostic(range, message, DiagnosticSeverity.Warning, "liberty-lemminx"));
+            diagnosticsList.add(new Diagnostic(range, message, DiagnosticSeverity.Warning, LIBERTY_LEMMINX_SOURCE));
             return;
         }
 
@@ -144,15 +164,56 @@ private void validateIncludeLocation(DOMDocument domDocument, List<Diagnostic> l
             if (!configFile.exists()) {
                 DOMAttr optNode = node.getAttributeNode("optional");
                 if (optNode == null) {
-                    list.add(new Diagnostic(range, IMPLICIT_NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, "liberty-lemminx", IMPLICIT_NOT_OPTIONAL_CODE));
+                    diagnosticsList.add(new Diagnostic(range, IMPLICIT_NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, IMPLICIT_NOT_OPTIONAL_CODE));
                 } else if (optNode.getValue().equals("false")) {
                     Range optRange = XMLPositionUtility.createRange(optNode.getStart(), optNode.getEnd(), domDocument);
-                    list.add(new Diagnostic(optRange, NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, "liberty-lemminx", NOT_OPTIONAL_CODE));
+                    diagnosticsList.add(new Diagnostic(optRange, NOT_OPTIONAL_MESSAGE, DiagnosticSeverity.Error, LIBERTY_LEMMINX_SOURCE, NOT_OPTIONAL_CODE));
                 }
-                list.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, "liberty-lemminx", MISSING_FILE_CODE));
+                diagnosticsList.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, LIBERTY_LEMMINX_SOURCE, MISSING_FILE_CODE));
             }
         } catch (IllegalArgumentException e) {
-            list.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, "liberty-lemminx-exception", MISSING_FILE_CODE));
+            diagnosticsList.add(new Diagnostic(range, MISSING_FILE_MESSAGE, DiagnosticSeverity.Warning, "liberty-lemminx-exception", MISSING_FILE_CODE));
+        }
+    }
+
+    /**
+     * Create temporary diagnostics for validation for single pass-through.
+     * @param domDocument
+     * @param diagnosticsList
+     * @param configElementNode
+     * @param tempDiagnosticsList
+     */
+    private void holdConfigElement(DOMDocument domDocument, DOMNode configElementNode, List<Diagnostic> tempDiagnosticsList) {
+        String configElementName = configElementNode.getNodeName();
+        Range range = XMLPositionUtility
+                .createRange(configElementNode.getStart(), configElementNode.getEnd(), domDocument);
+        Diagnostic tempDiagnostic = new Diagnostic(range, MISSING_CONFIGURED_FEATURE_MESSAGE, null, LIBERTY_LEMMINX_SOURCE, MISSING_CONFIGURED_FEATURE_CODE);
+        tempDiagnostic.setSource(configElementName);
+        tempDiagnosticsList.add(tempDiagnostic);
+    }
+
+    /**
+     * Compare the required feature set with included feature set for each config element.
+     * @param diagnosticsList
+     * @param tempDiagnosticsList
+     * @param featureGraph
+     */
+    private void validateConfigElements(List<Diagnostic> diagnosticsList, List<Diagnostic> tempDiagnosticsList, FeatureListGraph featureGraph) {
+        if (featureGraph.isEmpty()) {
+            return;
+        }
+        if (includedFeatures.isEmpty()) {
+            diagnosticsList.addAll(tempDiagnosticsList);
+            return;
+        }
+        for (Diagnostic tempDiagnostic : tempDiagnosticsList) {
+            String configElement = tempDiagnostic.getSource();
+            Set<String> includedFeaturesCopy = new HashSet<String>(includedFeatures);
+            Set<String> compatibleFeaturesList = featureGraph.getAllEnabledBy(configElement);
+            includedFeaturesCopy.retainAll(compatibleFeaturesList);
+            if (includedFeaturesCopy.isEmpty()) {
+                diagnosticsList.add(tempDiagnostic);
+            }
         }
     }
-}
+}

lemminx-liberty/src/main/java/io/openliberty/tools/langserver/lemminx/codeactions/AddFeature.java

@@ -0,0 +1,120 @@
+/*******************************************************************************
+* Copyright (c) 2023 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 java.util.List;
+import java.util.Set;
+import java.util.logging.Logger;
+
+import org.eclipse.lemminx.commons.BadLocationException;
+import org.eclipse.lemminx.commons.CodeActionFactory;
+import org.eclipse.lemminx.commons.TextDocument;
+import org.eclipse.lemminx.dom.DOMDocument;
+import org.eclipse.lemminx.dom.DOMElement;
+import org.eclipse.lemminx.dom.DOMNode;
+import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionParticipant;
+import org.eclipse.lemminx.services.extensions.codeaction.ICodeActionRequest;
+import org.eclipse.lemminx.utils.XMLPositionUtility;
+import org.eclipse.lsp4j.CodeAction;
+import org.eclipse.lsp4j.Diagnostic;
+import org.eclipse.lsp4j.Range;
+import org.eclipse.lsp4j.jsonrpc.CancelChecker;
+
+import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager;
+import io.openliberty.tools.langserver.lemminx.util.LibertyConstants;
+
+public class AddFeature implements ICodeActionParticipant {
+    Logger LOGGER = Logger.getLogger(AddFeature.class.getName());
+
+    /** This code action adresses 3 main situations:
+     *      1) Add a feature to an existing empty featureManager
+     *      2) Add a feature to an existing featureManager with children
+     *      3) Add a feature and new featureManager
+     *  
+     *  To calculate where to insert, each scenario will use a reference point to calculate range
+     *      1) The startTag of the featureManager
+     *      2) The last child of the featureManager
+     *      3) The startTag of the server.xml
+     */
+    public static final String FEATURE_FORMAT = "<feature>%s</feature>";
+    public static final String FEATUREMANAGER_FORMAT = 
+            "\n\t<featureManager>"+ 
+            "\n\t\t<feature>%s</feature>"+
+            "\n\t</featureManager>";
+
+    @Override
+    public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeActions, CancelChecker cancelChecker) {
+        Diagnostic diagnostic = request.getDiagnostic();
+        DOMDocument document = request.getDocument();
+        TextDocument textDocument = document.getTextDocument();
+        // getAllEnabledBy would return all transitive features but typically offers too much
+        Set<String> featureCandidates = LibertyProjectsManager.getInstance()
+                .getWorkspaceFolder(document.getDocumentURI())
+                .getFeatureListGraph().get(diagnostic.getSource()).getEnabledBy();
+        if (featureCandidates.isEmpty()) {
+            return;
+        }
+
+        String insertText = "";
+        int referenceRangeStart = 0;
+        int referenceRangeEnd = 0;
+
+        for (DOMNode node : document.getDocumentElement().getChildren()) {
+            if (LibertyConstants.FEATURE_MANAGER_ELEMENT.equals(node.getNodeName())) {
+                DOMNode lastChild = node.getLastChild();
+                if (node.getChildren().size() > 1) {
+                    // Situation 2
+                    insertText = "\n" + FEATURE_FORMAT;
+                    referenceRangeStart = lastChild.getStart();
+                    referenceRangeEnd = lastChild.getEnd();
+                } else {
+                    if (lastChild != null && (lastChild.hasChildNodes() || lastChild.isComment())) {
+                        // Situation 2
+                        insertText = "\n" + FEATURE_FORMAT;
+                        referenceRangeStart = lastChild.getStart();
+                        referenceRangeEnd = lastChild.getEnd();
+                    } else {
+                        // Situation 1
+                        insertText = "\n\t" + FEATURE_FORMAT;
+                        DOMElement featureManager = (DOMElement) node;
+                        referenceRangeStart = featureManager.getStartTagOpenOffset();
+                        referenceRangeEnd = featureManager.getStartTagCloseOffset()+1;
+                    }
+                }
+                break;
+            }
+        }
+        // Situation 3
+        if (insertText.isEmpty()) {
+            insertText = FEATUREMANAGER_FORMAT;
+            DOMElement server = document.getDocumentElement();
+            referenceRangeStart = server.getStart();
+            referenceRangeEnd = server.getStartTagCloseOffset()+1;
+        }
+        Range referenceRange = XMLPositionUtility.createRange(referenceRangeStart, referenceRangeEnd, document);
+
+        String indent = "    ";
+        try {
+            indent = request.getXMLGenerator().getWhitespacesIndent();
+        } catch (BadLocationException e) {
+            Logger.getLogger(AddFeature.class.getName()).info("Defaulting indent to four spaces.");
+        }
+        insertText = IndentUtil.formatText(insertText, indent, referenceRange.getStart().getCharacter());
+
+        for (String feature : featureCandidates) {
+            String title = "Add feature " + feature;
+            codeActions.add(CodeActionFactory.insert(
+                    title, referenceRange.getEnd(), String.format(insertText, feature), textDocument, diagnostic));
+        }
+    }
+}