#104: proper evaluation of variables (#147)

cli/src/main/java/com/devonfw/tools/ide/environment/AbstractEnvironmentVariables.java

@@ -28,6 +28,8 @@ public abstract class AbstractEnvironmentVariables implements EnvironmentVariabl
   // Variable surrounded with "${" and "}" such as "${JAVA_HOME}" 1......2........
   private static final Pattern VARIABLE_SYNTAX = Pattern.compile("(\\$\\{([^}]+)})");
 
+  private static final String SELF_REFERENCING_NOT_FOUND = "";
+
   private static final int MAX_RECURSION = 9;
 
   private static final String VARIABLE_PREFIX = "${";
@@ -161,36 +163,88 @@ public EnvironmentVariables resolved() {
   @Override
   public String resolve(String string, Object src) {
 
-    return resolve(string, src, 0, src, string);
+    return resolve(string, src, 0, src, string, this);
   }
 
-  private String resolve(String value, Object src, int recursion, Object rootSrc, String rootValue) {
+  /**
+   * This method is called recursively. This allows you to resolve variables that are defined by other variables.
+   *
+   * @param value the {@link String} that potentially contains variables in the syntax "${«variable«}". Those will be
+   *        resolved by this method and replaced with their {@link #get(String) value}.
+   * @param src the source where the {@link String} to resolve originates from. Should have a reasonable
+   *        {@link Object#toString() string representation} that will be used in error or log messages if a variable
+   *        could not be resolved.
+   * @param recursion the current recursion level. This is used to interrupt endless recursion.
+   * @param rootSrc the root source where the {@link String} to resolve originates from.
+   * @param rootValue the root value to resolve.
+   * @param resolvedVars this is a reference to an object of {@link EnvironmentVariablesResolved} being the lowest level
+   *        in the {@link EnvironmentVariablesType hierarchy} of variables. In case of a self-referencing variable
+   *        {@code x} the resolving has to continue one level higher in the {@link EnvironmentVariablesType hierarchy}
+   *        to avoid endless recursion. The {@link EnvironmentVariablesResolved} is then used if another variable
+   *        {@code y} must be resolved, since resolving this variable has to again start at the lowest level. For
+   *        example: For levels {@code l1, l2} with {@code l1 < l2} and {@code x=${x} foo} and {@code y=bar} defined at
+   *        level {@code l1} and {@code x=test ${y}} defined at level {@code l2}, {@code x} is first resolved at level
+   *        {@code l1} and then up the {@link EnvironmentVariablesType hierarchy} at {@code l2} to avoid endless
+   *        recursion. However, {@code y} must be resolved starting from the lowest level in the
+   *        {@link EnvironmentVariablesType hierarchy} and therefore {@link EnvironmentVariablesResolved} is used.
+   * @return the given {@link String} with the variables resolved.
+   */
+  private String resolve(String value, Object src, int recursion, Object rootSrc, String rootValue,
+      AbstractEnvironmentVariables resolvedVars) {
 
     if (value == null) {
       return null;
     }
     if (recursion > MAX_RECURSION) {
-      throw new IllegalStateException("Reached maximum recursion resolving " + value + " for root valiable " + rootSrc
+      throw new IllegalStateException("Reached maximum recursion resolving " + value + " for root variable " + rootSrc
           + " with value '" + rootValue + "'.");
     }
     recursion++;
+
     Matcher matcher = VARIABLE_SYNTAX.matcher(value);
     if (!matcher.find()) {
       return value;
     }
     StringBuilder sb = new StringBuilder(value.length() + EXTRA_CAPACITY);
     do {
       String variableName = matcher.group(2);
-      String variableValue = getValue(variableName);
+      String variableValue = resolvedVars.getValue(variableName);
       if (variableValue == null) {
         this.context.warning("Undefined variable {} in '{}={}' for root '{}={}'", variableName, src, value, rootSrc,
             rootValue);
-      } else {
-        String replacement = resolve(variableValue, variableName, recursion, rootSrc, rootValue);
+        continue;
+      }
+      EnvironmentVariables lowestFound = findVariable(variableName);
+      boolean isNotSelfReferencing = lowestFound == null || !lowestFound.getFlat(variableName).equals(value);
+
+      if (isNotSelfReferencing) {
+        // looking for "variableName" starting from resolved upwards the hierarchy
+        String replacement = resolvedVars.resolve(variableValue, variableName, recursion, rootSrc, rootValue,
+            resolvedVars);
+        matcher.appendReplacement(sb, Matcher.quoteReplacement(replacement));
+      } else { // is self referencing
+        // finding next occurrence of "variableName" up the hierarchy of EnvironmentVariablesType
+        EnvironmentVariables next = lowestFound.getParent();
+        while (next != null) {
+          if (next.getFlat(variableName) != null) {
+            break;
+          }
+          next = next.getParent();
+        }
+        if (next == null) {
+          matcher.appendReplacement(sb, Matcher.quoteReplacement(SELF_REFERENCING_NOT_FOUND));
+          continue;
+        }
+        // resolving a self referencing variable one level up the hierarchy of EnvironmentVariablesType, i.e. at "next",
+        // to avoid endless recursion
+        String replacement = ((AbstractEnvironmentVariables) next).resolve(next.getFlat(variableName), variableName,
+            recursion, rootSrc, rootValue, resolvedVars);
         matcher.appendReplacement(sb, Matcher.quoteReplacement(replacement));
+
       }
     } while (matcher.find());
     matcher.appendTail(sb);
+
     String resolved = sb.toString();
     return resolved;
   }

cli/src/main/java/com/devonfw/tools/ide/environment/EnvironmentVariables.java

@@ -186,7 +186,7 @@ default EnvironmentVariables findVariable(String name) {
    * @param source the source where the {@link String} to resolve originates from. Should have a reasonable
    *        {@link Object#toString() string representation} that will be used in error or log messages if a variable
    *        could not be resolved.
-   * @return the the given {@link String} with the variables resolved.
+   * @return the given {@link String} with the variables resolved.
    * @see com.devonfw.tools.ide.tool.ide.IdeToolCommandlet
    */
   String resolve(String string, Object source);

cli/src/main/java/com/devonfw/tools/ide/environment/EnvironmentVariablesPropertiesFile.java

@@ -222,9 +222,9 @@ public String set(String name, String value, boolean export) {
     String oldValue = this.variables.put(name, value);
     boolean flagChanged = export != this.exportedVariables.contains(name);
     if (Objects.equals(value, oldValue) && !flagChanged) {
-      this.context.trace("Set valiable '{}={}' caused no change in {}", name, value, this.propertiesFilePath);
+      this.context.trace("Set variable '{}={}' caused no change in {}", name, value, this.propertiesFilePath);
     } else {
-      this.context.debug("Set valiable '{}={}' in {}", name, value, this.propertiesFilePath);
+      this.context.debug("Set variable '{}={}' in {}", name, value, this.propertiesFilePath);
       this.modifiedVariables.add(name);
       if (export && (value != null)) {
         this.exportedVariables.add(name);

cli/src/test/java/com/devonfw/tools/ide/commandlet/VersionSetCommandletTest.java

@@ -44,6 +44,15 @@ public void testVersionSetCommandletRun() throws IOException {
         IDE_TOOLS=mvn,eclipse
 
         BAR=bar-${SOME}
-                """);
+        
+        TEST_ARGS1=${TEST_ARGS1} settings1
+        TEST_ARGS4=${TEST_ARGS4} settings4
+        TEST_ARGS5=${TEST_ARGS5} settings5
+        TEST_ARGS6=${TEST_ARGS6} settings6
+        TEST_ARGS7=${TEST_ARGS7} settings7
+        TEST_ARGS8=settings8
+        TEST_ARGS9=settings9
+        TEST_ARGSb=${TEST_ARGS10} settingsb ${TEST_ARGSa} ${TEST_ARGSb}
+        TEST_ARGSc=${TEST_ARGSc} settingsc""");
   }
 }

cli/src/test/java/com/devonfw/tools/ide/environment/EnvironmentVariablesTest.java

@@ -0,0 +1,60 @@
+package com.devonfw.tools.ide.environment;
+
+import com.devonfw.tools.ide.context.AbstractIdeContextTest;
+import com.devonfw.tools.ide.context.IdeTestContext;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test of {@link EnvironmentVariables}.
+ */
+public class EnvironmentVariablesTest extends AbstractIdeContextTest {
+
+  /**
+   * Test of {@link EnvironmentVariables#resolve(String, Object)} with self referencing variables.
+   */
+  @Test
+  public void testProperEvaluationOfVariables() {
+
+    // arrange
+    String path = "workspaces/foo-test/my-git-repo";
+    IdeTestContext context = newContext(PROJECT_BASIC, path, false);
+    EnvironmentVariables variables = context.getVariables();
+
+    // act
+    String TEST_ARGS1 = variables.get("TEST_ARGS1");
+    String TEST_ARGS2 = variables.get("TEST_ARGS2");
+    String TEST_ARGS3 = variables.get("TEST_ARGS3");
+    String TEST_ARGS4 = variables.get("TEST_ARGS4");
+    String TEST_ARGS5 = variables.get("TEST_ARGS5");
+    String TEST_ARGS6 = variables.get("TEST_ARGS6");
+    String TEST_ARGS7 = variables.get("TEST_ARGS7");
+    String TEST_ARGS8 = variables.get("TEST_ARGS8");
+    String TEST_ARGS9 = variables.get("TEST_ARGS9");
+    String TEST_ARGS10 = variables.get("TEST_ARGS10");
+    // some more advanced cases
+    String TEST_ARGSa = variables.get("TEST_ARGSa");
+    String TEST_ARGSb = variables.get("TEST_ARGSb");
+    String TEST_ARGSc = variables.get("TEST_ARGSc");
+    String TEST_ARGSd = variables.get("TEST_ARGSd");
+
+    // assert
+    assertThat(TEST_ARGS1).isEqualTo(" user1 settings1 workspace1 conf1");
+    assertThat(TEST_ARGS2).isEqualTo(" user2 conf2");
+    assertThat(TEST_ARGS3).isEqualTo(" user3 workspace3");
+    assertThat(TEST_ARGS4).isEqualTo(" settings4");
+    assertThat(TEST_ARGS5).isEqualTo(" settings5 conf5");
+    assertThat(TEST_ARGS6).isEqualTo(" settings6 workspace6 conf6");
+
+    assertThat(TEST_ARGS7).isEqualTo("user7 settings7 workspace7 conf7");
+    assertThat(TEST_ARGS8).isEqualTo("settings8 workspace8 conf8");
+    assertThat(TEST_ARGS9).isEqualTo("settings9 workspace9");
+    assertThat(TEST_ARGS10).isEqualTo("user10 workspace10");
+
+    assertThat(TEST_ARGSa).isEqualTo(" user1 settings1 workspace1 conf1  user3 workspace3 confa");
+    assertThat(TEST_ARGSb)
+        .isEqualTo("user10 workspace10 settingsb  user1 settings1 workspace1 conf1  user3 workspace3 confa userb");
+
+    assertThat(TEST_ARGSc).isEqualTo(" user1 settings1 workspace1 conf1 userc settingsc confc");
+    assertThat(TEST_ARGSd).isEqualTo(" user1 settings1 workspace1 conf1 userd workspaced");
+  }
+}

cli/src/test/resources/ide-projects/basic/conf/ide.properties

@@ -4,4 +4,13 @@
 
 M2_REPO=~/.m2/repository
 
-SOME=some-${UNDEFINED}
+SOME=some-${UNDEFINED}
+
+TEST_ARGS1=${TEST_ARGS1} conf1
+TEST_ARGS2=${TEST_ARGS2} conf2
+TEST_ARGS5=${TEST_ARGS5} conf5
+TEST_ARGS6=${TEST_ARGS6} conf6
+TEST_ARGS7=${TEST_ARGS7} conf7
+TEST_ARGS8=${TEST_ARGS8} conf8
+TEST_ARGSa=${TEST_ARGS1} ${TEST_ARGS3} confa
+TEST_ARGSc=${TEST_ARGSc} confc

cli/src/test/resources/ide-projects/basic/home/.ide/ide.properties

@@ -3,4 +3,13 @@
 #********************************************************************************
 
 DOCKER_EDITION=docker
-FOO=foo-${BAR}
+FOO=foo-${BAR}
+
+TEST_ARGS1=${TEST_ARGS1} user1
+TEST_ARGS2=${TEST_ARGS2} user2
+TEST_ARGS3=${TEST_ARGS3} user3
+TEST_ARGS7=user7
+TEST_ARGS10=user10
+TEST_ARGSb=userb
+TEST_ARGSc=${TEST_ARGS1} userc
+TEST_ARGSd=${TEST_ARGS1} userd

cli/src/test/resources/ide-projects/basic/settings/ide.properties

@@ -9,4 +9,14 @@ INTELLIJ_EDITION=ultimate
 
 IDE_TOOLS=mvn,eclipse
 
-BAR=bar-${SOME}
+BAR=bar-${SOME}
+
+TEST_ARGS1=${TEST_ARGS1} settings1
+TEST_ARGS4=${TEST_ARGS4} settings4
+TEST_ARGS5=${TEST_ARGS5} settings5
+TEST_ARGS6=${TEST_ARGS6} settings6
+TEST_ARGS7=${TEST_ARGS7} settings7
+TEST_ARGS8=settings8
+TEST_ARGS9=settings9
+TEST_ARGSb=${TEST_ARGS10} settingsb ${TEST_ARGSa} ${TEST_ARGSb}
+TEST_ARGSc=${TEST_ARGSc} settingsc

cli/src/test/resources/ide-projects/basic/workspaces/foo-test/ide.properties

@@ -0,0 +1,12 @@
+#********************************************************************************
+# Type of {@link EnvironmentVariables} from the
+# {@link com.devonfw.tools.ide.context.IdeContext#getWorkspacePath() workspace directory}.
+#********************************************************************************
+TEST_ARGS1=${TEST_ARGS1} workspace1
+TEST_ARGS3=${TEST_ARGS3} workspace3
+TEST_ARGS6=${TEST_ARGS6} workspace6
+TEST_ARGS7=${TEST_ARGS7} workspace7
+TEST_ARGS8=${TEST_ARGS8} workspace8
+TEST_ARGS9=${TEST_ARGS9} workspace9
+TEST_ARGS10=${TEST_ARGS10} workspace10
+TEST_ARGSd=${TEST_ARGSd} workspaced