#330: fix CLI to prevent invalid shortoption split (#530)

cli/src/main/java/com/devonfw/tools/ide/context/AbstractIdeContext.java

@@ -987,21 +987,37 @@ public boolean apply(CliArguments arguments, Commandlet cmd, CompletionCandidate
     } else {
       propertyIterator = cmd.getValues().iterator();
     }
+    Property<?> property = null;
+    if (propertyIterator.hasNext()) {
+      property = propertyIterator.next();
+    }
     while (!currentArgument.isEnd()) {
       trace("Trying to match argument '{}'", currentArgument);
-      Property<?> property = null;
+      Property<?> currentProperty = property;
       if (!arguments.isEndOptions()) {
-        property = cmd.getOption(currentArgument.getKey());
+        Property<?> option = cmd.getOption(currentArgument.getKey());
+        if (option != null) {
+          currentProperty = option;
+        }
+      }
+      if (currentProperty == null) {
+        trace("No option or next value found");
+        return false;
       }
-      if (property == null) {
-        if (!propertyIterator.hasNext()) {
-          trace("No option or next value found");
-          return false;
+      trace("Next property candidate to match argument is {}", currentProperty);
+      if (currentProperty == property) {
+        if (!property.isMultiValued()) {
+          if (propertyIterator.hasNext()) {
+            property = propertyIterator.next();
+          } else {
+            property = null;
+          }
+        }
+        if ((property != null) && property.isValue() && property.isMultiValued()) {
+          arguments.endOptions();
         }
-        property = propertyIterator.next();
       }
-      trace("Next property candidate to match argument is {}", property);
-      boolean matches = property.apply(arguments, this, cmd, collector);
+      boolean matches = currentProperty.apply(arguments, this, cmd, collector);
       if (!matches || currentArgument.isCompletion()) {
         return false;
       }

cli/src/main/java/com/devonfw/tools/ide/property/KeywordProperty.java

@@ -1,5 +1,10 @@
 package com.devonfw.tools.ide.property;
 
+import com.devonfw.tools.ide.cli.CliArguments;
+import com.devonfw.tools.ide.commandlet.Commandlet;
+import com.devonfw.tools.ide.completion.CompletionCandidateCollector;
+import com.devonfw.tools.ide.context.IdeContext;
+
 /**
  * {@link BooleanProperty} for a keyword (e.g. "install" in "ide install mvn 3.9.4").
  */
@@ -24,4 +29,11 @@ public boolean isExpectValue() {
     return false;
   }
 
+  @Override
+  protected boolean applyValue(String argValue, boolean lookahead, CliArguments args, IdeContext context, Commandlet commandlet,
+      CompletionCandidateCollector collector) {
+
+    setValue(true);
+    return true;
+  }
 }

cli/src/test/java/com/devonfw/tools/ide/cli/CliAdvancedParsingTest.java

@@ -0,0 +1,30 @@
+package com.devonfw.tools.ide.cli;
+
+import org.junit.jupiter.api.Test;
+
+import com.devonfw.tools.ide.context.AbstractIdeContextTest;
+import com.devonfw.tools.ide.context.IdeTestContext;
+
+/**
+ * Integration test of {@link com.devonfw.tools.ide.context.AbstractIdeContext#run(CliArguments) CLI parsing}.
+ */
+public class CliAdvancedParsingTest extends AbstractIdeContextTest {
+
+  private static final String PROJECT_MVN = "mvn";
+
+  /**
+   * Test that implicit end-options is triggered for multi-valued arguments to prevent splitting odd-formatted short-options like "-version".
+   */
+  @Test
+  public void testPreventShortOptionsForMultivaluedArguments() {
+
+    // arrange
+    IdeTestContext context = newContext(PROJECT_MVN);
+    CliArguments args = new CliArguments("java", "-version");
+    args.next();
+    // act
+    context.run(args);
+    // assert
+    assertThat(context).logAtInfo().hasNoMessage("java -v -e -r -s -i -o -n").hasMessage("java -version");
+  }
+}

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

@@ -93,9 +93,36 @@ public void testRunInfoLogging() {
     env.run();
     // assert
     if (SystemInfoImpl.INSTANCE.isWindows()) {
-      assertThat(context).log().hasEntriesWithNothingElseInBetween( //
+      assertThat(context).log().hasEntries /*WithNothingElseInBetween*/( //
+          IdeLogEntry.ofInfo("BAR=bar-some-${UNDEFINED}"), //
+          IdeLogEntry.ofInfo("DOCKER_EDITION=docker"), //
+          IdeLogEntry.ofInfo("ECLIPSE_VERSION=2023-03"), //
+          IdeLogEntry.ofInfo("FOO=foo-bar-some-${UNDEFINED}"), //
+          IdeLogEntry.ofInfo("HOME=" + context.getUserHome() + ""), //
+          IdeLogEntry.ofInfo("IDE_HOME=" + context.getIdeHome() + ""), //
+          IdeLogEntry.ofInfo("IDE_TOOLS=mvn,eclipse"), //
+          IdeLogEntry.ofInfo("INTELLIJ_EDITION=ultimate"), //
+          IdeLogEntry.ofInfo("JAVA_VERSION=17*"), //
           IdeLogEntry.ofInfo("M2_REPO=" + context.getUserHome() + "/.m2/repository"), //
-          new IdeLogEntry(IdeLogLevel.INFO, "PATH=", true) //
+          IdeLogEntry.ofInfo("MVN_VERSION=3.9.1"), //
+          new IdeLogEntry(IdeLogLevel.INFO, "PATH=", true), //
+          IdeLogEntry.ofInfo("SOME=some-${UNDEFINED}"), //
+          IdeLogEntry.ofInfo("TEST_ARGS1= user1 settings1 workspace1 conf1"), //
+          IdeLogEntry.ofInfo("TEST_ARGS10=user10 workspace10"), //
+          IdeLogEntry.ofInfo("TEST_ARGS2= user2 conf2"), //
+          IdeLogEntry.ofInfo("TEST_ARGS3= user3 workspace3"), //
+          IdeLogEntry.ofInfo("TEST_ARGS4= settings4"), //
+          IdeLogEntry.ofInfo("TEST_ARGS5= settings5 conf5"), //
+          IdeLogEntry.ofInfo("TEST_ARGS6= settings6 workspace6 conf6"), //
+          IdeLogEntry.ofInfo("TEST_ARGS7=user7 settings7 workspace7 conf7"), //
+          IdeLogEntry.ofInfo("TEST_ARGS8=settings8 workspace8 conf8"), //
+          IdeLogEntry.ofInfo("TEST_ARGS9=settings9 workspace9"), //
+          IdeLogEntry.ofInfo("TEST_ARGSa= user1 settings1 workspace1 conf1  user3 workspace3 confa"), //
+          IdeLogEntry.ofInfo("TEST_ARGSb=user10 workspace10 settingsb  user1 settings1 workspace1 conf1  user3 workspace3 confa userb"), //
+          IdeLogEntry.ofInfo("TEST_ARGSc= user1 settings1 workspace1 conf1 userc settingsc confc"), //
+          IdeLogEntry.ofInfo("TEST_ARGSd= user1 settings1 workspace1 conf1 userd workspaced"), //
+          IdeLogEntry.ofInfo("WORKSPACE=foo-test"), //
+          IdeLogEntry.ofInfo("WORKSPACE_PATH=" + context.getWorkspacePath()) //
       );
     } else {
       assertThat(context).log().hasEntriesWithNothingElseInBetween( //

cli/src/test/java/com/devonfw/tools/ide/log/IdeTestLoggerAssertion.java

@@ -22,70 +22,77 @@ public IdeTestLoggerAssertion(List<IdeLogEntry> entries, IdeLogLevel level) {
 
   /**
    * @param message the expected {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message}.
+   * @return this assertion itself for fluent API calls.
    */
-  public void hasMessage(String message) {
+  public IdeTestLoggerAssertion hasMessage(String message) {
 
-    fulfillsPredicate(e -> e.message().equals(message), PredicateMode.MATCH_ONE, "Could not find log message equal to '" + message + "'");
+    return fulfillsPredicate(e -> e.message().equals(message), PredicateMode.MATCH_ONE, "Could not find log message equal to '" + message + "'");
   }
 
   /**
    * @param message the {@link String} expected to be {@link String#contains(CharSequence) contained} in a
-   * {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message}.
+   *     {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message}.
+   * @return this assertion itself for fluent API calls.
    */
-  public void hasMessageContaining(String message) {
+  public IdeTestLoggerAssertion hasMessageContaining(String message) {
 
-    fulfillsPredicate(e -> e.message().contains(message), PredicateMode.MATCH_ONE, "Could not find log message containing '" + message + "'");
+    return fulfillsPredicate(e -> e.message().contains(message), PredicateMode.MATCH_ONE, "Could not find log message containing '" + message + "'");
   }
 
   /**
    * @param message the {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message} that is not expected and should not have been logged.
+   * @return this assertion itself for fluent API calls.
    */
-  public void hasNoMessage(String message) {
+  public IdeTestLoggerAssertion hasNoMessage(String message) {
 
-    fulfillsPredicate(e -> !e.message().equals(message), PredicateMode.MATCH_ALL, "No log message should be equal to '" + message + "'");
+    return fulfillsPredicate(e -> !e.message().equals(message), PredicateMode.MATCH_ALL, "No log message should be equal to '" + message + "'");
   }
 
   /**
    * @param message the {@link String} expected not be {@link String#contains(CharSequence) contained} in any
-   * {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message}.
+   *     {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message}.
+   * @return this assertion itself for fluent API calls.
    */
-  public void hasNoMessageContaining(String message) {
+  public IdeTestLoggerAssertion hasNoMessageContaining(String message) {
 
-    fulfillsPredicate(e -> !e.message().contains(message), PredicateMode.MATCH_ALL, "No log message should contain '" + message + "'");
+    return fulfillsPredicate(e -> !e.message().contains(message), PredicateMode.MATCH_ALL, "No log message should contain '" + message + "'");
   }
 
   /**
    * @param messages the expected {@link com.devonfw.tools.ide.log.IdeSubLogger#log(String) log message}s in order.
+   * @return this assertion itself for fluent API calls.
    */
-  public void hasEntries(String... messages) {
+  public IdeTestLoggerAssertion hasEntries(String... messages) {
 
     assert (this.level != null);
     IdeLogEntry[] entries = new IdeLogEntry[messages.length];
     int i = 0;
     for (String message : messages) {
       entries[i++] = new IdeLogEntry(this.level, message);
     }
-    hasEntries(false, entries);
+    return hasEntries(false, entries);
   }
 
   /**
    * @param expectedEntries the expected {@link com.devonfw.tools.ide.log.IdeLogEntry log entries} in order.
+   * @return this assertion itself for fluent API calls.
    */
-  public void hasEntries(IdeLogEntry... expectedEntries) {
+  public IdeTestLoggerAssertion hasEntries(IdeLogEntry... expectedEntries) {
 
-    hasEntries(false, expectedEntries);
+    return hasEntries(false, expectedEntries);
   }
 
   /**
    * @param expectedEntries the expected {@link com.devonfw.tools.ide.log.IdeLogEntry log entries} to be logged in order without any other log statement in
-   * between them.
+   *     between them.
+   * @return this assertion itself for fluent API calls.
    */
-  public void hasEntriesWithNothingElseInBetween(IdeLogEntry... expectedEntries) {
+  public IdeTestLoggerAssertion hasEntriesWithNothingElseInBetween(IdeLogEntry... expectedEntries) {
 
-    hasEntries(true, expectedEntries);
+    return hasEntries(true, expectedEntries);
   }
 
-  private void hasEntries(boolean nothingElseInBetween, IdeLogEntry... expectedEntries) {
+  private IdeTestLoggerAssertion hasEntries(boolean nothingElseInBetween, IdeLogEntry... expectedEntries) {
 
     assert (expectedEntries.length > 0);
     int i = 0;
@@ -101,7 +108,7 @@ private void hasEntries(boolean nothingElseInBetween, IdeLogEntry... expectedEnt
         }
       }
       if (i == expectedEntries.length) {
-        return;
+        return this;
       }
       if (i > max) {
         max = i;
@@ -127,6 +134,7 @@ private void hasEntries(boolean nothingElseInBetween, IdeLogEntry... expectedEnt
       appendEntry(error, entry);
     }
     Assertions.fail(error.toString());
+    return this;
   }
 
   private static void appendEntry(StringBuilder sb, IdeLogEntry entry) {
@@ -136,7 +144,7 @@ private static void appendEntry(StringBuilder sb, IdeLogEntry entry) {
     sb.append('\n');
   }
 
-  private void fulfillsPredicate(Predicate<IdeLogEntry> predicate, PredicateMode mode, String errorMessage) {
+  private IdeTestLoggerAssertion fulfillsPredicate(Predicate<IdeLogEntry> predicate, PredicateMode mode, String errorMessage) {
 
     if (this.level != null) {
       errorMessage = errorMessage + " on level " + this.level;
@@ -145,17 +153,18 @@ private void fulfillsPredicate(Predicate<IdeLogEntry> predicate, PredicateMode m
       if ((this.level == null) || (this.level == entry.level())) {
         if (predicate.test(entry)) {
           if (mode == PredicateMode.MATCH_ONE) {
-            return;
+            return this;
           }
         } else if (mode == PredicateMode.MATCH_ALL) {
           Assertions.fail(errorMessage + "\nFound unexpected log entry: " + entry);
-          return;
+          return this;
         }
       }
     }
     if (mode == PredicateMode.MATCH_ONE) {
       Assertions.fail(errorMessage); // no log entry matched by predicate
     }
+    return this;
   }
 
 }

cli/src/test/resources/ide-projects/mvn/repository/java/java/default/bin/java

@@ -1,2 +1,2 @@
 #!/bin/bash
-echo "java mvn"
+echo java $*