MLE-24984 Friendly error message when using removed short options

Author: rjrudin

.copyrightconfig

@@ -11,4 +11,4 @@ startyear: 2024
 #   - Dotfiles already skipped automatically
 # Enable by removing the leading '# ' from the next line and editing values.
 # filesexcluded: third_party/*, docs/generated/*.md, assets/*.png, scripts/temp_*.py, vendor/lib.js
-filesexcluded: .github/*, README.md, CONTRIBUTING.md, Jenkinsfile, gradle/*, docker-compose.yml, *.gradle, gradle.properties, gradlew, gradlew.bat, *.json, *.xml, *.txt, **/test/resources/**, docs/**, flux-cli/hadoop/*, CODEOWNERS
+filesexcluded: .github/*, README.md, CONTRIBUTING.md, Jenkinsfile, gradle/*, docker-compose.yml, *.gradle, gradlew, gradlew.bat, *.json, *.xml, *.txt, **/test/resources/**, docs/**, flux-cli/hadoop/*, CODEOWNERS, *.properties

flux-cli/src/main/java/com/marklogic/flux/cli/ShortErrorMessageHandler.java

@@ -6,6 +6,7 @@
 import picocli.CommandLine;
 
 import java.io.PrintWriter;
+import java.util.Map;
 import java.util.Objects;
 
 /**
@@ -14,6 +15,16 @@
  */
 class ShortErrorMessageHandler implements CommandLine.IParameterExceptionHandler {
 
+    private static final Map<String, String> SHORT_OPTIONS_REPLACED_IN_TWO_DOT_ZERO_RELEASE = Map.of(
+        "-P", "--spark-prop",
+        "-R", "--doc-prop",
+        "-M", "--doc-metadata",
+        "-S", "--splitter-prop",
+        "-X", "--xpath-namespace",
+        "-L", "--classifier-prop",
+        "-E", "--embedder-prop"
+    );
+
     public int handleParseException(CommandLine.ParameterException ex, String[] args) {
         final CommandLine commandLine = ex.getCommandLine();
         Objects.requireNonNull(commandLine);
@@ -29,7 +40,8 @@ public int handleParseException(CommandLine.ParameterException ex, String[] args
 
         final String exceptionMessage = getErrorMessageToPrint(ex);
         if (exceptionMessage != null) {
-            err.println(colorScheme.errorText(exceptionMessage)); // bold red
+            err.println(colorScheme.errorText(exceptionMessage));
+            printHelpfulMessageForReplacedSingleLetterOption(exceptionMessage, err, colorScheme);
         }
 
         CommandLine.UnmatchedArgumentException.printSuggestions(ex, err);
@@ -55,4 +67,24 @@ private String getErrorMessageToPrint(Exception ex) {
         }
         return message;
     }
+
+    private void printHelpfulMessageForReplacedSingleLetterOption(String exceptionMessage, PrintWriter err, CommandLine.Help.ColorScheme colorScheme) {
+        SHORT_OPTIONS_REPLACED_IN_TWO_DOT_ZERO_RELEASE.keySet().forEach(shortOption -> {
+            final String indicatorOfReplacedShortOption = "Unknown option: '" + shortOption;
+            if (exceptionMessage.startsWith(indicatorOfReplacedShortOption)) {
+                String longOption = SHORT_OPTIONS_REPLACED_IN_TWO_DOT_ZERO_RELEASE.get(shortOption);
+                err.println("");
+                err.println(colorScheme.errorText("Did you mean to use %s instead of %s, as %s was replaced in the 2.0 release with %s?"
+                    .formatted(longOption, shortOption, shortOption, longOption)));
+
+                String optionValue = exceptionMessage.substring(indicatorOfReplacedShortOption.length());
+                if (optionValue.endsWith("'")) {
+                    optionValue = optionValue.substring(0, optionValue.length() - 1);
+                }
+
+                err.println(colorScheme.errorText("If so, use %s %s instead.".formatted(longOption, optionValue)));
+                err.println("");
+            }
+        });
+    }
 }

flux-cli/src/test/java/com/marklogic/flux/impl/HandleErrorTest.java

@@ -5,9 +5,12 @@
 
 import com.marklogic.flux.AbstractTest;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import picocli.CommandLine;
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * Note that some tests will print a "An illegal reflective access operation has occurred" warning that occurs
@@ -142,4 +145,33 @@ void dontAbortOnWriteFailure() {
         assertFalse(stderr.contains("Command failed"), "The command should not have failed since it defaults to not " +
             "aborting on a write failure. Actual stderr: " + stderr);
     }
+
+    @ParameterizedTest
+    @CsvSource({
+        "-P,--spark-prop",
+        "-E,--embedder-prop",
+        "-L,--classifier-prop",
+        "-S,--splitter-prop",
+        "-R,--doc-prop",
+        "-M,--doc-metadata",
+        "-X,--xpath-namespace",
+    })
+    void niceErrorMessageForShortOptionsRemovedInTwoDotZeroRelease(String shortOption, String longOption) {
+        // Verifying we get a helpful error message for each short option removed in the 2.0 release. The command here
+        // is not important.
+        String stderr = runAndReturnStderr(
+            CommandLine.ExitCode.USAGE,
+            "import-orc-files",
+            "--path", "src/test/resources/orc-files/authors.orc",
+            "--connection-string", makeConnectionString(),
+            "--permissions", DEFAULT_PERMISSIONS,
+            "%stheKey=theValue".formatted(shortOption)
+        );
+
+        assertTrue(stderr.contains("Did you mean to use %s instead of %s, as %s was replaced in the 2.0 release with %s?"
+            .formatted(longOption, shortOption, shortOption, longOption)), "Unexpected stderr: " + stderr);
+
+        assertTrue(stderr.contains("If so, use %s theKey=theValue instead.".formatted(longOption)),
+            "Unexpected stderr: " + stderr);
+    }
 }