fix: update variables usage which is fully qualified to a single loca… (

#1724)

* fix: update variables usage which is fully qualified to a single location

Qualification rules
The rules for qualifying a name are:
• A name can be qualified even though it does not need qualification except in a REDEFINES clause, in
which case it must not be qualified.

• Each qualifier must be of a higher level than the name it qualifies and must be within the same
hierarchy.
• If there is more than one combination of qualifiers that ensures uniqueness, any of those combinations
can be used.
• If compiler option QUALIFY(EXTEND) is in effect, and if there is only one fully qualified name that
matches your combination of qualifiers, that reference will be considered unique, even if the set of
qualifiers also matches a partial qualification for a different data item. Fully qualified means every
qualifier is specified.

Signed-off-by: Aman Prashant <aman.prashant@broadcom.com>

server/common/src/main/java/org/eclipse/lsp/cobol/common/model/tree/variable/VariableNode.java

@@ -100,8 +100,10 @@ public SyntaxError getError(MessageTemplate messageTemplate, ErrorSeverity sever
    * @param usageNode a variable usage node
    */
   public void addUsage(VariableUsageNode usageNode) {
-    usages.add(usageNode.getLocality().toLocation());
-    usageNode.addDefinition(this);
+    if (!usages.contains(usageNode.getLocality().toLocation())) {
+      usages.add(usageNode.getLocality().toLocation());
+      usageNode.addDefinition(this);
+    }
   }
 
   public List<Location> getDefinitions() {

server/engine/src/main/java/org/eclipse/lsp/cobol/core/model/VariableUsageUtils.java

@@ -14,6 +14,7 @@
  */
 package org.eclipse.lsp.cobol.core.model;
 
+import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
 import org.eclipse.lsp.cobol.common.model.NodeType;
 import org.eclipse.lsp.cobol.common.model.tree.variable.VariableNode;
@@ -23,9 +24,7 @@
 import java.util.*;
 import java.util.stream.Collectors;
 
-/**
- * The class take all defined variables and search through them by partial qualifier
- */
+/** The class take all defined variables and search through them by partial qualifier */
 @UtilityClass
 public class VariableUsageUtils {
   /**
@@ -37,23 +36,52 @@ public class VariableUsageUtils {
    */
   public static List<VariableNode> findVariablesForUsage(
       Multimap<String, VariableNode> definedVariables, List<VariableUsageNode> usageNodes) {
-    return definedVariables.get(usageNodes.get(0).getName()).stream()
-        .distinct()
-        .filter(it -> checkParents(it, usageNodes.subList(1, usageNodes.size())))
-        .collect(Collectors.toList());
+    Map<VariableNode, Integer> variableToStepCountsToMatchParentsMap =
+        definedVariables.get(usageNodes.get(0).getName()).stream()
+            .distinct()
+            .map(
+                it ->
+                    mapVariableToStepCountsToMatchParents(
+                        it, usageNodes.subList(1, usageNodes.size())))
+            .reduce(
+                (firstMap, secondMap) -> {
+                  firstMap.putAll(secondMap);
+                  return firstMap;
+                })
+            .orElse(Collections.emptyMap());
+
+    List<VariableNode> exactHierarchyMatchedVariables =
+        variableToStepCountsToMatchParentsMap.entrySet().stream()
+            .filter(entry -> entry.getValue().equals(usageNodes.size() - 1))
+            .map(Map.Entry::getKey)
+            .collect(Collectors.toList());
+
+    return exactHierarchyMatchedVariables.isEmpty()
+        ? new ArrayList<>(variableToStepCountsToMatchParentsMap.keySet())
+        : exactHierarchyMatchedVariables;
   }
 
-  private static boolean checkParents(VariableNode variable, List<VariableUsageNode> parents) {
+  private static Map<VariableNode, Integer> mapVariableToStepCountsToMatchParents(
+      VariableNode variable, List<VariableUsageNode> parents) {
+    VariableNode referredVariable = variable;
+    int count = 0;
     for (VariableUsageNode parent : parents) {
       String parentName = parent.getName();
       do {
-        variable = variable.getNearestParentByType(NodeType.VARIABLE)
-            .map(VariableNode.class::cast)
-            .orElse(null);
-        if (variable == null)
-          return false;
+
+        variable = getNearestParentVariable(variable);
+        if (variable == null) {
+          return new HashMap<>();
+        }
+        count++;
       } while (!variable.getName().equals(parentName));
     }
-    return true;
+    return Maps.newHashMap(Collections.singletonMap(referredVariable, count));
+  }
+
+  private static VariableNode getNearestParentVariable(VariableNode variable) {
+    return variable.getNearestParentByType(NodeType.VARIABLE)
+            .map(VariableNode.class::cast)
+            .orElse(null);
   }
 }

server/engine/src/test/java/org/eclipse/lsp/cobol/usecases/TestQualifiedVariableUsage.java

@@ -0,0 +1,118 @@
+/*
+ * Copyright (c) 2020 Broadcom.
+ * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries.
+ *
+ * This program and the accompanying materials are made
+ * available under the terms of the Eclipse Public License 2.0
+ * which is available at https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ *    Broadcom, Inc. - initial API and implementation
+ *
+ */
+
+package org.eclipse.lsp.cobol.usecases;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.eclipse.lsp.cobol.common.error.ErrorSource;
+import org.eclipse.lsp.cobol.test.engine.UseCaseEngine;
+import org.eclipse.lsp4j.Diagnostic;
+import org.eclipse.lsp4j.DiagnosticSeverity;
+import org.eclipse.lsp4j.Range;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.stream.Stream;
+
+/** Test different combinations of qualified variable usage */
+public class TestQualifiedVariableUsage {
+  public static final String HEADER_TEXT =
+      "PROCESS PGMN(LM),DYNAM,QUALIFY(EXTEND),SSRANGE(ZLEN) \n"
+          + "       IDENTIFICATION DIVISION.\n"
+          + "       PROGRAM-ID. 'TEST' RECURSIVE.\n"
+          + "       ENVIRONMENT DIVISION.\n"
+          + "       INPUT-OUTPUT SECTION.\n"
+          + "       FILE-CONTROL.\n"
+          + "       DATA DIVISION.\n"
+          + "       FILE SECTION.\n"
+          + "       WORKING-STORAGE SECTION.\n"
+          + "       LOCAL-STORAGE SECTION.\n"
+          + "       LINKAGE SECTION.\n"
+          + "       1 {$*value1} USAGE FUNCTION-POINTER.\n";
+  public static final String CASE1 =
+      HEADER_TEXT
+          + "       1 {$*zl_return}.\n"
+          + "         3 {$*key1}.\n"
+          + "           5 {$*data1}.\n"
+          + "             7 {$*value1} USAGE POINTER.\n"
+          + "         3 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "       PROCEDURE DIVISION USING {$zl_return} {$value1}.\n"
+          + "           SET {$value1} IN {$zl_return} TO {$value1}.\n"
+          + "       END PROGRAM 'TEST'.";
+  public static final String CASE2 =
+      HEADER_TEXT
+          + "       1 {$*zl_return}.\n"
+          + "         3 {$*key1}.\n"
+          + "           5 {$*data1}.\n"
+          + "             7 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "         3 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "       PROCEDURE DIVISION USING {$zl_return} {$value1}.\n"
+          + "           SET {$value1} IN {$data1} in {$key1} IN {$zl_return} TO {$value1}.\n"
+          + "       END PROGRAM 'TEST'.";
+
+  public static final String CASE3 =
+      HEADER_TEXT
+          + "       1 {$*zl_return}.\n"
+          + "         3 {$*key1}.\n"
+          + "           5 {$*data1}.\n"
+          + "             7 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "             7 {$*size1} PIC S9(9) COMP-5.\n"
+          + "             7 {$*capacity} PIC S9(9) COMP-5.\n"
+          + "         3 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "       PROCEDURE DIVISION USING {$zl_return} {$value1}.\n"
+          + "           SET {$value1} IN {$data1} IN {$zl_return} TO {$value1}.\n"
+          + "       END PROGRAM 'TEST'.";
+
+  public static final String CASE4 =
+      HEADER_TEXT
+          + "       1 {$*zl_return}.\n"
+          + "         3 {$*key1}.\n"
+          + "           5 {$*data1}.\n"
+          + "             7 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "           5 {$*data2}.\n"
+          + "             7 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "         3 {$*value1} USAGE FUNCTION-POINTER.\n"
+          + "       PROCEDURE DIVISION USING {$zl_return} {$value1}.\n"
+          + "           SET {_{$value1} IN {$key1} IN {$zl_return}|1_} TO {$value1}.\n"
+          + "       END PROGRAM 'TEST'.";
+
+  private static Stream<String> textsToTest() {
+    return Stream.of(CASE1, CASE2, CASE3);
+  }
+
+  @ParameterizedTest
+  @MethodSource("textsToTest")
+  @DisplayName("Parameterized - testing qualified variable usage")
+  void positiveTest(String text) {
+    UseCaseEngine.runTest(text, ImmutableList.of(), ImmutableMap.of());
+  }
+
+  @Test
+  void negativeTest() {
+    UseCaseEngine.runTest(
+        CASE4,
+        ImmutableList.of(),
+        ImmutableMap.of(
+            "1",
+            new Diagnostic(
+                new Range(),
+                "Ambiguous reference for VALUE1",
+                DiagnosticSeverity.Error,
+                ErrorSource.PARSING.getText())));
+  }
+}