Add more feature hover info (#268)

* Add info to hover for feature

* Refactor FeatureListGraph

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
-* Copyright (c) 2020, 2023 IBM Corporation and others.
+* Copyright (c) 2020, 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
@@ -20,15 +20,22 @@
 import org.eclipse.lsp4j.MarkupContent;
 import org.eclipse.lsp4j.jsonrpc.CancelChecker;
 
+import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph;
+import io.openliberty.tools.langserver.lemminx.data.FeatureListNode;
 import io.openliberty.tools.langserver.lemminx.data.LibertyRuntime;
 import io.openliberty.tools.langserver.lemminx.models.feature.*;
 import io.openliberty.tools.langserver.lemminx.services.FeatureService;
 import io.openliberty.tools.langserver.lemminx.services.SettingsService;
 import io.openliberty.tools.langserver.lemminx.util.*;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Optional;
+import java.util.Set;
+import java.util.logging.Logger;
 
 public class LibertyHoverParticipant implements IHoverParticipant {
+    private static final Logger LOGGER = Logger.getLogger(LibertyHoverParticipant.class.getName());
 
     @Override
     public Hover onAttributeName(IHoverRequest request, CancelChecker cancelChecker) {
@@ -63,7 +70,7 @@ public Hover onText(IHoverRequest request, CancelChecker cancelChecker) {
         return null;
     }
 
-    private Hover getHoverFeatureDescription(String featureName, DOMDocument domDocument) {
+    private Hover getFeatureDescription(String featureName, DOMDocument domDocument) {
         LibertyRuntime runtimeInfo = LibertyUtils.getLibertyRuntimeInfo(domDocument);
         String libertyVersion =  runtimeInfo == null ? null : runtimeInfo.getRuntimeVersion();
         String libertyRuntime =  runtimeInfo == null ? null : runtimeInfo.getRuntimeType();
@@ -76,4 +83,55 @@ private Hover getHoverFeatureDescription(String featureName, DOMDocument domDocu
 
         return null;
     }
+
+    private Hover getHoverFeatureDescription(String featureName, DOMDocument document) {
+        // Choosing to use the default feature list to get the full enables/enabled by information to display, as quite often the generated
+        // feature list will only be a subset of the default one. If the feature is not found in the default feature list, this code will 
+        // default to the original description only which is available from the downloaded features.json file.
+        FeatureListGraph featureGraph = FeatureService.getInstance().getDefaultFeatureList();
+        FeatureListNode flNode = featureGraph.getFeatureListNode(featureName);
+        if (flNode == null) {
+            LOGGER.warning("Could not get full description for feature: "+featureName+"  from cached feature list. Using description from features.json file.");
+            return getFeatureDescription(featureName, document);
+        }
+
+        StringBuilder sb = new StringBuilder();
+        String description = flNode.getDescription();
+        sb.append("Description: ");
+        sb.append(description);
+        sb.append(System.lineSeparator());
+
+        // get features that directly enable this feature
+        Set<String> featureEnabledBy = flNode.getEnabledBy();
+        if (!featureEnabledBy.isEmpty()) {
+            sb.append("Enabled by: ");
+            // Need to sort the collection of features so that they are in a reliable order for tests.
+            ArrayList<String> sortedFeatures = new ArrayList<String>();
+            sortedFeatures.addAll(featureEnabledBy);
+            Collections.sort(sortedFeatures);
+            for (String nextFeature : sortedFeatures) {
+                sb.append(nextFeature);
+                sb.append(", ");
+            }
+            sb.setLength(sb.length() - 2);
+            sb.append(System.lineSeparator());
+        }
+
+        // get features that this feature directly enables
+        Set<String> featureEnables = flNode.getEnablesFeatures();
+        if (!featureEnables.isEmpty()) {
+            sb.append("Enables: ");
+            // Need to sort the collection of features so that they are in a reliable order for tests.
+            ArrayList<String> sortedFeatures = new ArrayList<String>();
+            sortedFeatures.addAll(featureEnables);
+            Collections.sort(sortedFeatures);
+            for (String nextFeature : sortedFeatures) {
+                sb.append(nextFeature);
+                sb.append(", ");
+            }
+            sb.setLength(sb.length() - 2);
+        }
+
+       return new Hover(new MarkupContent("plaintext", sb.toString()));
+    }
 }

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

@@ -32,8 +32,8 @@
 import org.eclipse.lsp4j.Range;
 import org.eclipse.lsp4j.jsonrpc.CancelChecker;
 
+import io.openliberty.tools.langserver.lemminx.data.ConfigElementNode;
 import io.openliberty.tools.langserver.lemminx.data.FeatureListGraph;
-import io.openliberty.tools.langserver.lemminx.data.FeatureListNode;
 import io.openliberty.tools.langserver.lemminx.services.FeatureService;
 import io.openliberty.tools.langserver.lemminx.services.LibertyProjectsManager;
 import io.openliberty.tools.langserver.lemminx.services.LibertyWorkspace;
@@ -73,14 +73,15 @@ public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeAction
             featureGraph = ws.getFeatureListGraph();
         }
 
-        FeatureListNode flNode = featureGraph.get(diagnostic.getSource());
-        if (flNode == null) {
+        ConfigElementNode configElement = featureGraph.getConfigElementNode(diagnostic.getSource());
+        if (configElement == null) {
             LOGGER.warning("Could not add quick fix for missing feature for config element due to missing information in the feature list: "+diagnostic.getSource());
             return;
         }
 
-        // getAllEnabledBy would return all transitive features but typically offers too much
-        Set<String> featureCandidates = flNode.getEnabledBy();
+        // This is getting features that directly enable the config element and not all transitive features 
+        // as that would typically be too much to display/offer in a quick fix.
+        Set<String> featureCandidates = configElement.getEnabledBy();
         if (featureCandidates.isEmpty()) {
             return;
         }

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

@@ -0,0 +1,21 @@
+/*******************************************************************************
+* 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.data;
+
+// Class to represent a config element in a feature list xml
+public class ConfigElementNode extends Node {
+
+    public ConfigElementNode(String nodeName) {
+        super(nodeName);
+    }
+}

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

@@ -1,5 +1,5 @@
 /*******************************************************************************
-* Copyright (c) 2023 IBM Corporation and others.
+* Copyright (c) 2023, 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
@@ -21,47 +21,68 @@
 
 public class FeatureListGraph {
     private String runtime = "";
-    private Map<String, FeatureListNode> nodes;
-    private Map<String, Set<String>> enabledByCache; // storing in lower case to enable diagnostics with configured features
-    private Map<String, Set<String>> enablesCache;
-
+    private Map<String, FeatureListNode> featureNodes;
+    private Map<String, ConfigElementNode> configElementNodes;
+    private Map<String, Node> nodes;
+    private Map<String, Set<String>> enabledByCache; 
+    private Map<String, Set<String>> enabledByCacheLowerCase; // storing in lower case to enable diagnostics with configured features
+
     public FeatureListGraph() {
-        nodes = new HashMap<String, FeatureListNode>();
+        nodes = new HashMap<String, Node>();
+        featureNodes = new HashMap<String, FeatureListNode>();
+        configElementNodes = new HashMap<String, ConfigElementNode>();
+        enabledByCacheLowerCase = new HashMap<String, Set<String>>();
         enabledByCache = new HashMap<String, Set<String>>();
-        enablesCache = new HashMap<String, Set<String>>();
     }
 
     public FeatureListNode addFeature(String nodeName) {
-        if (nodes.containsKey(nodeName)) {
-            return nodes.get(nodeName);
+        if (featureNodes.containsKey(nodeName)) {
+            return featureNodes.get(nodeName);
         }
         FeatureListNode node = new FeatureListNode(nodeName);
+        featureNodes.put(nodeName, node);
         nodes.put(nodeName, node);
         return node;
     }
 
-    public FeatureListNode addConfigElement(String nodeName) {
-        if (nodes.containsKey(nodeName)) {
-            return nodes.get(nodeName);
+    public FeatureListNode addFeature(String nodeName, String description) {
+        if (featureNodes.containsKey(nodeName)) {
+            FeatureListNode node = featureNodes.get(nodeName);
+            if (node.getDescription().isEmpty()) {
+                node.setDescription(description);
+            }
+            return node;
         }
-        FeatureListNode node = new FeatureListNode(nodeName);
+        FeatureListNode node = new FeatureListNode(nodeName, description);
+        featureNodes.put(nodeName, node);
         nodes.put(nodeName, node);
         return node;
     }
+
+    public ConfigElementNode addConfigElement(String nodeName) {
+        if (configElementNodes.containsKey(nodeName)) {
+            return configElementNodes.get(nodeName);
+        }
+        ConfigElementNode node = new ConfigElementNode(nodeName);
+        configElementNodes.put(nodeName, node);
+        nodes.put(nodeName, node);
+        return node;
+    }
+
+    public FeatureListNode getFeatureListNode(String nodeName) {
+        return featureNodes.get(nodeName);
+    }
 
-    public FeatureListNode get(String nodeName) {
-        return nodes.get(nodeName);
+    public ConfigElementNode getConfigElementNode(String nodeName) {
+        return configElementNodes.get(nodeName);
     }
 
     public boolean isEmpty() {
-        return nodes.isEmpty();
+        return configElementNodes.isEmpty() && featureNodes.isEmpty();
     }
 
-    public boolean isConfigElement(String featureListNode) {
-        if (!nodes.containsKey(featureListNode)) {
-            return false;
-        }
-        return nodes.get(featureListNode).isConfigElement();
+    public boolean isConfigElement(String name) {
+        return configElementNodes.containsKey(name);
     }
 
     public void setRuntime(String runtime) {
@@ -79,14 +100,32 @@ public String getRuntime() {
      * @return
      */
     public Set<String> getAllEnabledBy(String elementName) {
+        return getAllEnabledBy(elementName, true);
+    }
+
+    /**
+     * Returns a superset of 'owning' features that enable a given config element or feature.
+     * The features are returned in lower case if the 'lowerCase' boolean is true. Otherwise,
+     * the features are returned in their original case.
+     * @param elementName
+     * @return
+     */
+    public Set<String> getAllEnabledBy(String elementName, boolean lowerCase) {
+
+        if (lowerCase && enabledByCacheLowerCase.containsKey(elementName)) {
+            return enabledByCacheLowerCase.get(elementName);
+        }
+
         if (enabledByCache.containsKey(elementName)) {
             return enabledByCache.get(elementName);
         }
+
         if (!nodes.containsKey(elementName)) {
             return null;
         }
+
         // Implements a breadth-first-search on parent nodes
-        Set<String> allEnabledBy = new HashSet<String>(nodes.get(elementName).getEnabledBy());
+        Set<String> allEnabledBy = new HashSet<String>(nodes.get(elementName).getEnabledBy());;
         Deque<String> queue = new ArrayDeque<String>(allEnabledBy);
         Set<String> visited = new HashSet<String>();
         while (!queue.isEmpty()) {
@@ -100,68 +139,22 @@ public Set<String> getAllEnabledBy(String elementName) {
             allEnabledBy.addAll(enablers);
             queue.addAll(enablers);
         }
-        return addToEnabledByCacheInLowerCase(elementName, allEnabledBy);
+        return addToEnabledByCache(elementName, allEnabledBy, lowerCase);
     }
 
-    private Set<String> addToEnabledByCacheInLowerCase(String configElement, Set<String> allEnabledBy) {
+    private Set<String> addToEnabledByCache(String configElement, Set<String> allEnabledBy, boolean lowerCase) {
         Set<String> lowercaseEnabledBy = new HashSet<String>();
+        Set<String> originalcaseEnabledBy = new HashSet<String>();
+        originalcaseEnabledBy.addAll(allEnabledBy);
+
         for (String nextFeature: allEnabledBy) {
             lowercaseEnabledBy.add(nextFeature.toLowerCase());
         }
-        enabledByCache.put(configElement, lowercaseEnabledBy);
-        return lowercaseEnabledBy;
-    }
 
-    /**
-     * Returns the set of supported features or config elements for a given feature.
-     * @param feature
-     * @return
-     */
-    public Set<String> getAllEnables(String feature) {
-        if (enablesCache.containsKey(feature)) {
-            return enablesCache.get(feature);
-        }
-        if (!nodes.containsKey(feature)) {
-            return null;
-        }
-        // Implements a breadth-first-search on child nodes
-        Set<String> allEnables = new HashSet<String>(nodes.get(feature).getEnables());
-        Deque<String> queue = new ArrayDeque<String>(allEnables);
-        Set<String> visited = new HashSet<String>();
-        while (!queue.isEmpty()) {
-            String node = queue.getFirst();
-            queue.removeFirst();
-            if (visited.contains(node)) {
-                continue;
-            }
-            Set<String> enablers = nodes.get(node).getEnables();
-            visited.add(node);
-            allEnables.addAll(enablers);
-            queue.addAll(enablers);
-        }
-        enablesCache.put(feature, allEnables);
-        return allEnables;
-    }
+        enabledByCacheLowerCase.put(configElement, lowercaseEnabledBy);
+        enabledByCache.put(configElement, originalcaseEnabledBy);
 
-    /** Will be useful for future features **/
-
-    // public Set<String> getAllConfigElements(String feature) {
-    //     Set<String> configElements = new HashSet<String>();
-    //     for (String node : getAllEnables(feature)) {
-    //         if (isConfigElement(node)) {
-    //             configElements.add(node);
-    //         }
-    //     }
-    //     return configElements;
-    // }
-
-    // public Set<String> getAllEnabledFeatures(String feature) {
-    //     Set<String> enabledFeatures = new HashSet<String>();
-    //     for (String node : getAllEnables(feature)) {
-    //         if (!isConfigElement(node)) {
-    //             enabledFeatures.add(node);
-    //         }
-    //     }
-    //     return enabledFeatures;
-    // }
+        return lowerCase ? lowercaseEnabledBy : originalcaseEnabledBy;
+    }
+
 }