adding more file filters and changing completion logic for multiple vars

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

lemminx-liberty/src/it/variable-processing-ol-it/src/test/java/LibertyWorkspaceIT.java

@@ -181,15 +181,6 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws IOException, Ba
                     .get(0).getLeft().getTextDocument()
                     .setUri(serverXmlFile.toURI().toString());
         }
-        TextEdit texted = te(8, 9,
-                8, 9, " ");
-
-        CodeAction invalidCodeAction = ca(invalid1, texted);
-        invalidCodeAction.getEdit()
-                .getDocumentChanges()
-                .get(0).getLeft().getTextDocument()
-                .setUri(serverXmlFile.toURI().toString());
-        codeActions.add(invalidCodeAction);
-        XMLAssert.testCodeActionsFor(serverXML, serverXmlFile.toURI().toString(), invalid1, codeActions.get(0),codeActions.get(1));
+        XMLAssert.testCodeActionsFor(serverXML, serverXmlFile.toURI().toString(), invalid1, codeActions.get(0));
     }
 }

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

@@ -56,32 +56,42 @@ public void onAttributeValue(String valuePrefix, ICompletionRequest request, ICo
                         .getDocumentURI());
         //getting all existing variables in current completion prefix string
         List<VariableLoc> variables = LibertyUtils.getVariablesFromTextContent(valuePrefix);
-        // value prefix will contain all existing attribute value, till the cursor
-        String variableName = valuePrefix.replace("${", "")
-                .replace("}", "");
-        StringBuilder prefixVariables = new StringBuilder();
-        String replacePrefix = "";
-        for (VariableLoc variableLoc : variables) {
-            prefixVariables.append(String.format("${%s}", variableLoc.getValue()));
-        }
+        String variablePrefix = "";
+        String completionPrefix;
+
         if (!variables.isEmpty()) {
+            // for multiple variables
             // find last variable last character and consider new variable completion from that location
             VariableLoc lastVar = variables.get(variables.size() - 1);
-            variableName = valuePrefix.substring(lastVar.getEndLoc() + 1);
-            // if last variable is started with ${
-            if (variableName.contains("${")) {
-                variableName = variableName.substring(variableName.lastIndexOf("${")).replace("${", "")
-                        .replace("}", "");
-                // add char between last variable and start of completion variable to prefix
-                replacePrefix = valuePrefix.substring(lastVar.getEndLoc() + 1, valuePrefix.lastIndexOf("${"));
+            // if last index of an existing variable is less than lastIndex of ${,
+            // this means that completion variable is started with ${
+            if (valuePrefix.lastIndexOf("${") > lastVar.getEndLoc()) {
+                // get whatever is after last ${ as variablePrefix for Completion
+                variablePrefix = valuePrefix.substring(valuePrefix.lastIndexOf("${") + 2);
+                completionPrefix = valuePrefix.substring(0, valuePrefix.lastIndexOf("${"));
+            } else {
+                variablePrefix = valuePrefix.substring(lastVar.getEndLoc() + 1);
+                // add char between last variable and start of completion variable to completionPrefix
+                completionPrefix = valuePrefix.substring(0,lastVar.getEndLoc()+1);
+            }
+        } else {
+            // for single variable,check ${ is specified
+            if (valuePrefix.contains("${")) {
+                // extract variable name with whatever is after ${
+                variablePrefix = valuePrefix.substring(valuePrefix.lastIndexOf("${") + 2);
+                // extract completionPrefix with whatever is before ${
+                completionPrefix = valuePrefix.substring(0, valuePrefix.lastIndexOf("${"));
+            } else {
+                // if no ${ specified, take all data as completion variable
+                variablePrefix = valuePrefix;
+                completionPrefix = "";
             }
         }
-        prefixVariables.append(replacePrefix);
-        String finalVariableName = variableName;
-        variableProps.entrySet().stream().filter(it -> it.getKey().toString().toLowerCase().contains(finalVariableName.toLowerCase()))
+        String finalVariableName = variablePrefix.replace("${","").replace("}","");
+        variableProps.entrySet().stream().filter(it -> it.getKey().toString().toLowerCase()
+                        .contains(finalVariableName.toLowerCase()))
                 .forEach(variableProp -> {
-                    String varValue = String.format("%s${%s}", prefixVariables, variableProp.getKey());
-
+                    String varValue = String.format("%s${%s}", completionPrefix, variableProp.getKey());
                     Either<TextEdit, InsertReplaceEdit> edit = Either.forLeft(new
                             TextEdit(request.getReplaceRange(), varValue));
                     CompletionItem completionItem = new CompletionItem();

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

@@ -129,7 +129,9 @@ private void validateVariables(DOMDocument domDocument, List<Diagnostic> diagnos
         for (VariableLoc variable : variables) {
             if (!variablesMap.containsKey(variable.getValue())) {
                 String variableInDoc = String.format("${%s}", variable.getValue());
-                Range range = XMLPositionUtility.createRange(variable.getStartLoc()-2,variable.getEndLoc()+1,
+                //range is used in ReplaceVariable to provide quick fix.
+                // we just need the variable value range here as ${} is added in replace variable message
+                Range range = XMLPositionUtility.createRange(variable.getStartLoc() - 2, variable.getEndLoc() + 1,
                         domDocument);
                 String message = "ERROR: The variable \"" + variable.getValue() + "\" does not exist.";
 

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

@@ -66,12 +66,6 @@ public void doCodeAction(ICodeActionRequest request, List<CodeAction> codeAction
                     String variableInDoc = String.format("${%s}", nextVariable.getKey().toString());
                     codeActions.add(CodeActionFactory.replace(title, diagnostic.getRange(), variableInDoc, document.getTextDocument(), diagnostic));
                 }
-                // use special code action to reload variable diagnostics
-                Range range = XMLPositionUtility.createRange(document.getDocumentElement().getEndTagCloseOffset(), document.getDocumentElement().getEndTagCloseOffset()+1,
-                        document);
-                String title = "Reload Diagnostics";
-                codeActions.add(CodeActionFactory.insert(title, range.getEnd(), " ",
-                        document.getTextDocument(), diagnostic));
             }
         } catch (Exception e) {
             // BadLocationException not expected

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

@@ -72,7 +72,7 @@ private FileAlterationObserver getFileAlterationObserver(final String parentPath
                 new SuffixFileFilter(Arrays.asList(".class", ".lst", ".txt", ".log", ".manager", ".libertyls",
                         ".sLock", ".jar", ".war", ".ear",".mf"),
                         IOCase.INSENSITIVE)
-                        .or(new NameFileFilter(Arrays.asList("workarea", "plugin-cfg.xml", "libs", "tmp", "classes","bin",
+                        .or(new NameFileFilter(Arrays.asList("plugin-cfg.xml", "libs", "tmp", "classes",
                                 "generated-sources", "generated-test-sources", "invoker-reports",
                                 "it", "maven-status", "surefire-reports", "test-classes"), IOCase.INSENSITIVE)));
         FileAlterationObserver observer = new FileAlterationObserver(parentPath, notFileFilter);
@@ -84,14 +84,17 @@ private void addFileAlterationListener(FileAlterationObserver observer, LibertyW
         observer.addListener(new FileAlterationListenerAdaptor() {
             @Override
             public void onDirectoryCreate(File file) {
+                // not required
             }
 
             @Override
             public void onDirectoryDelete(File file) {
+                // not required
             }
 
             @Override
             public void onDirectoryChange(File file) {
+                // not required
             }
 
             @Override

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

@@ -54,9 +54,9 @@ public class LibertyUtils {
     private static Thread thread;
 
     //considering ${var} pattern for variable.  do we have other representation for variable?
-    private static final String regex = "\\$\\{(.*?)\\}";
+    private static final String VAR_PATTERN_REGEX = "\\$\\{(.*?)\\}";
     // Compile the Regex.
-    private static final Pattern p = Pattern.compile(regex);
+    private static final Pattern VAR_PATTERN = Pattern.compile(VAR_PATTERN_REGEX);
 
     private LibertyUtils() {
     }
@@ -606,15 +606,17 @@ public static File getFileFromLibertyPluginXml(Path pluginConfigFilePath, String
 
     /**
      * read variables from text content
+     * if the text content is ${testVar1}/${testVar2}/app ,response will contain 2 variable values
+     *      [{testVar1,startIndex,endIndex},{testVar2,startIndex,endIndex}]
      * @param docContent text content
-     * @return list of variables
+     * @return list of variables.
      */
     public static List<VariableLoc> getVariablesFromTextContent(String docContent) {
         List<VariableLoc> variables = new ArrayList<>();
         // Find match between given string
         // and regular expression
         // using Pattern.matcher()
-        Matcher m = p.matcher(docContent);
+        Matcher m = VAR_PATTERN.matcher(docContent);
         // Get the subsequence
         // using find() method
         while (m.find()) {

lemminx-liberty/src/test/java/io/openliberty/LibertyCompletionTest.java

@@ -389,6 +389,8 @@ public void testVariableCompletionItemWithVariablePatterns() throws BadLocationE
                 propsMap.put("default.https.port","9443");
                 propsMap.put("testVar","false");
                 propsMap.put("testVar2","true");
+                propsMap.put("appName","app-name.war");
+                propsMap.put("appName2","app-name-new.war");
                 Properties props = new Properties();
                 props.putAll(propsMap);
 
@@ -437,6 +439,25 @@ public void testVariableCompletionItemWithVariablePatterns() throws BadLocationE
                         "</server>" //
                 );
                 XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, 0);
+
+                // single variable completion with prefix
+                serverXML = String.join(newLine,
+                        "<server description=\"Sample Liberty server\">",
+                        "   <featureManager>",
+                        "       <feature>servlet</feature>",
+                        "       <platform>jakartaee-9.1</platform>",
+                        "   </featureManager>",
+                        "   <webApplication contextRoot=\"/app-name\" location=\"apps/${app|\" />",
+                        "   <httpEndpoint id=\"defaultHttpEndpoint\" httpPort=\"9080\" httpsPort=\"9443\"/>",
+                        "   <ssl id=\"defaultSSLConfig\" trustDefaultCerts=\"true\" />",
+                        "</server>"
+                );
+
+                CompletionItem testVarCompletion = c("apps/${appName}", "apps/${appName}");
+                CompletionItem testVar2Completion = c("apps/${appName2}", "apps/${appName2}");
+
+                XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, testVarCompletion,
+                        testVar2Completion);
         }
 
         // Tests the
@@ -446,6 +467,7 @@ public void testVariableCompletionItemWithMultipleVars() throws BadLocationExcep
                 Map<String,String> propsMap=new HashMap<>();
                 propsMap.put("default.http.port","9080");
                 propsMap.put("default.https.port","9443");
+                propsMap.put("appLocation","root");
                 propsMap.put("testVar","app-name.war");
                 propsMap.put("testVar2","app-name2.war");
                 Properties props = new Properties();
@@ -479,19 +501,19 @@ public void testVariableCompletionItemWithMultipleVars() throws BadLocationExcep
                         "       <feature>servlet</feature>",
                         "       <platform>jakartaee-9.1</platform>",
                         "   </featureManager>",
-                        "   <webApplication contextRoot=\"/app-name\" location=\"${testVar2}/apps/${tes|\" />",
+                        "   <webApplication contextRoot=\"/app-name\" location=\"${testVar2}/apps/${appLocation}/${tes|\" />",
                         "   <httpEndpoint id=\"defaultHttpEndpoint\" httpPort=\"9080\" httpsPort=\"9443\"/>",
                         "   <ssl id=\"defaultSSLConfig\" trustDefaultCerts=\"true\" />",
                         "</server>"
                 );
 
-                testVarCompletion = c("${testVar2}/apps/${testVar}", "${testVar2}/apps/${testVar}");
-                testVar2Completion = c("${testVar2}/apps/${testVar2}", "${testVar2}/apps/${testVar2}");
+                testVarCompletion = c("${testVar2}/apps/${appLocation}/${testVar}", "${testVar2}/apps/${appLocation}/${testVar}");
+                testVar2Completion = c("${testVar2}/apps/${appLocation}/${testVar2}", "${testVar2}/apps/${appLocation}/${testVar2}");
 
                 XMLAssert.testCompletionFor(serverXML, null, serverXMLURI, TOTAL_ITEMS, testVarCompletion,
                         testVar2Completion);
 
-                /* for showing completion of second variable, variable should always prefix with ${ . Hence show no completion here*/
+                // for showing completion of second variable, variable should always prefix with ${ . Hence show no completion here
                 serverXML = String.join(newLine,
                         "<server description=\"Sample Liberty server\">",
                         "   <featureManager>",

lemminx-liberty/src/test/java/io/openliberty/LibertyDiagnosticTest.java

@@ -966,16 +966,7 @@ public void testInvalidVariableDiagnosticWithCodeAction() throws BadLocationExce
                     .get(0).getLeft().getTextDocument()
                     .setUri(serverXMLURI);
         }
-        TextEdit texted = te(8, 9,
-                8, 9, " ");
-
-        CodeAction invalidCodeAction = ca(invalid1, texted);
-        invalidCodeAction.getEdit()
-                .getDocumentChanges()
-                .get(0).getLeft().getTextDocument()
-                .setUri(serverXMLURI);
-        codeActions.add(invalidCodeAction);
-        XMLAssert.testCodeActionsFor(serverXML, serverXMLURI, invalid1, codeActions.get(0),codeActions.get(1));
+        XMLAssert.testCodeActionsFor(serverXML, serverXMLURI, invalid1, codeActions.get(0));
     }