Improve printing of values for options.

fniephaus · fniephaus · commit a9b2244abca5 · 2023-06-06T18:38:19.000+02:00
diff --git a/substratevm/src/com.oracle.svm.common/src/com/oracle/svm/common/option/CommonOptionParser.java b/substratevm/src/com.oracle.svm.common/src/com/oracle/svm/common/option/CommonOptionParser.java
@@ -276,18 +276,8 @@ public static OptionParseResult parseOption(EconomicMap<String, OptionDescriptor
                         selectedOptionTypes.add(OptionType.valueOf(enumString));
                     }
                 } catch (IllegalArgumentException e) {
-                    StringBuilder sb = new StringBuilder();
-                    boolean firstValue = true;
-                    for (OptionType ot : OptionType.values()) {
-                        if (firstValue) {
-                            firstValue = false;
-                        } else {
-                            sb.append(", ");
-                        }
-                        sb.append(ot.name());
-                    }
-                    String possibleValues = sb.toString();
-                    return OptionParseResult.error("Invalid value for option " + current + ". '" + enumString + "' is not one of: " + possibleValues);
+                    String possibleValues = StringUtil.joinSingleQuoted(OptionType.values());
+                    return OptionParseResult.error("Invalid value for option " + current + ". '" + enumString + "' is not one of " + possibleValues + ".");
                 }
             }
             return OptionParseResult.printFlags(selectedOptionTypes);
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/APIOptionHandler.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/APIOptionHandler.java
@@ -319,9 +319,8 @@ boolean consume(ArgumentQueue args) {
                 if (headArg.startsWith(groupNameAndSeparator)) {
                     GroupInfo groupInfo = entry.getValue();
                     String groupName = APIOption.Utils.optionName(groupInfo.group.name());
-                    String supportedValues = "'" + String.join("', '", groupInfo.supportedValues) + "'";
                     NativeImage.showError("In " + args.argumentOrigin + " '" + headArg.substring(groupNameAndSeparator.length()) + "' is not a valid value for the option " + groupName +
-                                    ". Supported values are " + supportedValues);
+                                    ". Supported values are " + StringUtil.joinSingleQuoted(groupInfo.supportedValues) + ".");
                 }
             }
         }
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/BundleSupport.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/BundleSupport.java
@@ -58,7 +58,6 @@
 import java.util.jar.JarFile;
 import java.util.jar.JarOutputStream;
 import java.util.jar.Manifest;
-import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import org.graalvm.util.json.JSONParserException;
@@ -71,6 +70,7 @@
 import com.oracle.svm.core.util.json.JsonWriter;
 import com.oracle.svm.util.ClassUtil;
 import com.oracle.svm.util.LogUtils;
+import com.oracle.svm.util.StringUtil;
 
 final class BundleSupport {
 
@@ -176,10 +176,8 @@ static BundleSupport create(NativeImage nativeImage, String bundleArg, NativeIma
             return bundleSupport;
 
         } catch (StringIndexOutOfBoundsException | IllegalArgumentException e) {
-            String suggestedVariants = Arrays.stream(BundleOptionVariants.values())
-                            .map(v -> BUNDLE_OPTION + "-" + v)
-                            .collect(Collectors.joining(", "));
-            throw NativeImage.showError("Unknown option " + bundleArg + ". Valid variants are: " + suggestedVariants + ".");
+            String suggestedVariants = StringUtil.joinSingleQuoted(Arrays.stream(BundleOptionVariants.values()).map(v -> BUNDLE_OPTION + "-" + v).toList());
+            throw NativeImage.showError("Unknown option '" + bundleArg + "'. Valid variants are " + suggestedVariants + ".");
         }
     }
 
diff --git a/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java b/substratevm/src/com.oracle.svm.driver/src/com/oracle/svm/driver/NativeImage.java
@@ -94,6 +94,7 @@
 import com.oracle.svm.util.LogUtils;
 import com.oracle.svm.util.ModuleSupport;
 import com.oracle.svm.util.ReflectionUtil;
+import com.oracle.svm.util.StringUtil;
 
 public class NativeImage {
 
@@ -1102,8 +1103,7 @@ private int completeImageBuild() {
             }
 
             if (!extraImageArgs.isEmpty()) {
-                String prefix = "Unknown argument" + (extraImageArgs.size() == 1 ? ": " : "s: ");
-                showError(extraImageArgs.stream().collect(Collectors.joining(", ", prefix, "")));
+                showError("Unknown argument(s): " + StringUtil.joinSingleQuoted(extraImageArgs));
             }
         }
 
@@ -1147,8 +1147,7 @@ private int completeImageBuild() {
         }
 
         if (!leftoverArgs.isEmpty()) {
-            String prefix = "Unrecognized option" + (leftoverArgs.size() == 1 ? ": " : "s: ");
-            showError(leftoverArgs.stream().collect(Collectors.joining(", ", prefix, "")));
+            showError("Unrecognized option(s): " + StringUtil.joinSingleQuoted(leftoverArgs));
         }
 
         LinkedHashSet<Path> finalImageModulePath = new LinkedHashSet<>(imageModulePath);
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageGenerator.java
@@ -306,6 +306,7 @@
 import com.oracle.svm.util.ImageBuildStatistics;
 import com.oracle.svm.util.ReflectionUtil;
 import com.oracle.svm.util.ReflectionUtil.ReflectionUtilError;
+import com.oracle.svm.util.StringUtil;
 
 import jdk.vm.ci.aarch64.AArch64;
 import jdk.vm.ci.amd64.AMD64;
@@ -1890,7 +1891,7 @@ private static <T extends Enum<T>> Set<T> parseCSVtoEnum(Class<T> enumType, List
             try {
                 result.add(Enum.valueOf(enumType, enumValue));
             } catch (IllegalArgumentException iae) {
-                throw VMError.shouldNotReachHere("Value '" + enumValue + "' does not exist. Available values are:\n" + Arrays.toString(availValues));
+                throw VMError.shouldNotReachHere("Value '" + enumValue + "' does not exist. Available values are " + StringUtil.joinSingleQuoted(availValues) + ".");
             }
         }
         return result;
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageOptions.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/NativeImageOptions.java
@@ -30,7 +30,6 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.text.SimpleDateFormat;
-import java.util.Arrays;
 import java.util.Date;
 
 import org.graalvm.collections.EconomicMap;
@@ -51,6 +50,7 @@
 import com.oracle.svm.core.util.UserError;
 import com.oracle.svm.hosted.classinitialization.ClassInitializationOptions;
 import com.oracle.svm.hosted.util.CPUType;
+import com.oracle.svm.util.StringUtil;
 
 public class NativeImageOptions {
 
@@ -171,14 +171,14 @@ public boolean compatibleWith(CStandards standard) {
         }
     }
 
-    @Option(help = "C standard to use in header files. Possible values are: [C89, C99, C11]", type = User)//
+    @Option(help = "C standard to use in header files. Possible values are 'C89', 'C99', and 'C11'.", type = User)//
     public static final HostedOptionKey<String> CStandard = new HostedOptionKey<>("C89");
 
     public static CStandards getCStandard() {
         try {
             return CStandards.valueOf(CStandard.getValue());
         } catch (IllegalArgumentException e) {
-            throw UserError.abort("C standard %s is not supported. Supported standards are: %s", CStandard.getValue(), Arrays.toString(CStandards.values()));
+            throw UserError.abort("C standard '%s' is not supported. Supported standards are %s.", CStandard.getValue(), StringUtil.joinSingleQuoted(CStandards.values()));
         }
     }
 
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/diagnostic/HostedHeapDumpFeature.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/diagnostic/HostedHeapDumpFeature.java
@@ -46,6 +46,7 @@
 import com.oracle.svm.core.option.SubstrateOptionsParser;
 import com.oracle.svm.core.util.UserError;
 import com.oracle.svm.hosted.FeatureImpl.DuringSetupAccessImpl;
+import com.oracle.svm.util.StringUtil;
 
 @AutomaticallyRegisteredFeature
 public class HostedHeapDumpFeature implements InternalFeature {
@@ -84,8 +85,8 @@ public boolean isInConfiguration(IsInConfigurationAccess access) {
             if (validPhases.contains(value)) {
                 phases.add(value);
             } else {
-                throw UserError.abort("Invalid value %s given for %s. Valid values are: %s.", value,
-                                SubstrateOptionsParser.commandArgument(Options.DumpHeap, ""), String.join(", ", validPhases));
+                throw UserError.abort("Invalid value %s given for %s. Valid values are %s.",
+                                value, SubstrateOptionsParser.commandArgument(Options.DumpHeap, ""), StringUtil.joinSingleQuoted(validPhases));
             }
         }
         return !phases.isEmpty();
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/util/CPUTypeAArch64.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/util/CPUTypeAArch64.java
@@ -45,6 +45,7 @@
 import com.oracle.svm.core.option.SubstrateOptionsParser;
 import com.oracle.svm.core.util.UserError;
 import com.oracle.svm.hosted.NativeImageOptions;
+import com.oracle.svm.util.StringUtil;
 
 import jdk.vm.ci.aarch64.AArch64;
 import jdk.vm.ci.aarch64.AArch64.CPUFeature;
@@ -63,7 +64,7 @@ public enum CPUTypeAArch64 implements CPUType {
     COMPATIBILITY(NativeImageOptions.MICRO_ARCHITECTURE_COMPATIBILITY, ARMV8_A),
     NATIVE(NativeImageOptions.MICRO_ARCHITECTURE_NATIVE, getNativeOrEmpty());
 
-    private static final String AVAILABLE_FEATURE_MODIFIERS = String.join(", ", new String[]{"aes", "lse", "fp", "simd"});
+    private static final String AVAILABLE_FEATURE_MODIFIERS = StringUtil.joinSingleQuoted("aes", "lse", "fp", "simd");
 
     private static CPUFeature[] getNativeOrEmpty() {
         CPUFeature[] empty = new CPUFeature[0];
@@ -132,7 +133,7 @@ public static EnumSet<CPUFeature> getCPUFeaturesForArch(String marchValue) {
             throw UserError.abort("Unsupported architecture '%s'. Please adjust '%s'. On AArch64, only %s are available.",
                             marchValue,
                             SubstrateOptionsParser.commandArgument(NativeImageOptions.MicroArchitecture, marchValue),
-                            List.of(values()).stream().map(v -> v.name).collect(Collectors.joining(", ")));
+                            StringUtil.joinSingleQuoted(values()));
         }
         List<CPUFeature> features = new ArrayList<>(value.getFeatures());
         processFeatureModifiers(features, archParts);
diff --git a/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/util/CPUTypeAMD64.java b/substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/util/CPUTypeAMD64.java
@@ -67,6 +67,7 @@
 import com.oracle.svm.core.util.UserError;
 import com.oracle.svm.hosted.NativeImageOptions;
 import com.oracle.svm.util.LogUtils;
+import com.oracle.svm.util.StringUtil;
 
 import jdk.vm.ci.amd64.AMD64;
 import jdk.vm.ci.amd64.AMD64.CPUFeature;
@@ -184,7 +185,7 @@ public static EnumSet<CPUFeature> getCPUFeaturesForArch(String marchValue) {
             throw UserError.abort("Unsupported architecture '%s'. Please adjust '%s'. On AMD64, only %s are available.",
                             marchValue,
                             SubstrateOptionsParser.commandArgument(NativeImageOptions.MicroArchitecture, marchValue),
-                            List.of(values()).stream().map(v -> v.name).collect(Collectors.joining(", ")));
+                            StringUtil.joinSingleQuoted(values()));
         }
         return value.getFeatures();
     }
diff --git a/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/TruffleFeature.java b/substratevm/src/com.oracle.svm.truffle/src/com/oracle/svm/truffle/TruffleFeature.java
@@ -98,7 +98,6 @@
 import java.util.function.ToLongBiFunction;
 import java.util.function.ToLongFunction;
 import java.util.function.UnaryOperator;
-import java.util.stream.Collectors;
 
 import org.graalvm.collections.Pair;
 import org.graalvm.compiler.api.replacements.SnippetReflectionProvider;
@@ -163,6 +162,7 @@
 import com.oracle.svm.truffle.api.SubstrateTruffleRuntime;
 import com.oracle.svm.truffle.api.SubstrateTruffleUniverseFactory;
 import com.oracle.svm.util.LogUtils;
+import com.oracle.svm.util.StringUtil;
 import com.oracle.truffle.api.CompilerAsserts;
 import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
@@ -870,9 +870,9 @@ public void beforeCompilation(BeforeCompilationAccess config) {
 
                 if (implementations.size() > 1) {
                     throw UserError.abort("More than one implementation of %s found. For performance reasons, Truffle languages must not provide new implementations, " +
-                                    "and instead only use the single implementation provided by the Truffle runtime. To disable this check, add %s to the native-image command line. Found classes: %s",
+                                    "and instead only use the single implementation provided by the Truffle runtime. To disable this check, add %s to the native-image command line. Classes found are %s.",
                                     Frame.class.getTypeName(), SubstrateOptionsParser.commandArgument(Options.TruffleCheckFrameImplementation, "-"),
-                                    implementations.stream().map(m -> m.toJavaName(true)).collect(Collectors.joining(", ")));
+                                    StringUtil.joinSingleQuoted(implementations.stream().map(m -> m.toJavaName(true)).toArray(String[]::new)));
                 } else {
                     assert implementations.size() == 0 || implementations.iterator().next().equals(frameType.getSingleImplementor());
                 }
diff --git a/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/StringUtil.java b/substratevm/src/com.oracle.svm.util/src/com/oracle/svm/util/StringUtil.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2021, 2023, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2021, 2021, Alibaba Group Holding Limited. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -27,6 +27,8 @@
 package com.oracle.svm.util;
 
 import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
 
 public class StringUtil {
 
@@ -69,4 +71,30 @@ public static String[] split(String value, String separator, int limit) {
 
         return list.toArray(new String[list.size()]);
     }
+
+    /**
+     * Joins an array of strings using single quotes, and `, ` as a delimiter. Useful to print
+     * possible values for options.
+     */
+    public static final String joinSingleQuoted(String... values) {
+        return "'" + String.join("', '", values) + "'";
+    }
+
+    /**
+     * See {@link #joinSingleQuoted(String...)}.
+     */
+    public static final String joinSingleQuoted(Object[] values) {
+        String[] strings = new String[values.length];
+        for (int i = 0; i < strings.length; i++) {
+            strings[i] = Objects.toString(values[i]);
+        }
+        return joinSingleQuoted(strings);
+    }
+
+    /**
+     * See {@link #joinSingleQuoted(String...)}.
+     */
+    public static final String joinSingleQuoted(List<String> strings) {
+        return joinSingleQuoted(strings.toArray(new String[strings.size()]));
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -319,9 +319,8 @@ boolean consume(ArgumentQueue args) {`
`319`	`319`	`if (headArg.startsWith(groupNameAndSeparator)) {`
`320`	`320`	`GroupInfo groupInfo = entry.getValue();`
`321`	`321`	`String groupName = APIOption.Utils.optionName(groupInfo.group.name());`
`322`		`- String supportedValues = "'" + String.join("', '", groupInfo.supportedValues) + "'";`
`323`	`322`	`NativeImage.showError("In " + args.argumentOrigin + " '" + headArg.substring(groupNameAndSeparator.length()) + "' is not a valid value for the option " + groupName +`
`324`		`- ". Supported values are " + supportedValues);`
	`323`	`+ ". Supported values are " + StringUtil.joinSingleQuoted(groupInfo.supportedValues) + ".");`
`325`	`324`	`}`
`326`	`325`	`}`
`327`	`326`	`}`
Original file line number	Diff line number	Diff line change
`@@ -94,6 +94,7 @@`
`94`	`94`	`import com.oracle.svm.util.LogUtils;`
`95`	`95`	`import com.oracle.svm.util.ModuleSupport;`
`96`	`96`	`import com.oracle.svm.util.ReflectionUtil;`
	`97`	`+import com.oracle.svm.util.StringUtil;`
`97`	`98`
`98`	`99`	`public class NativeImage {`
`99`	`100`
`@@ -1102,8 +1103,7 @@ private int completeImageBuild() {`
`1102`	`1103`	`}`
`1103`	`1104`
`1104`	`1105`	`if (!extraImageArgs.isEmpty()) {`
`1105`		`- String prefix = "Unknown argument" + (extraImageArgs.size() == 1 ? ": " : "s: ");`
`1106`		`- showError(extraImageArgs.stream().collect(Collectors.joining(", ", prefix, "")));`
	`1106`	`+ showError("Unknown argument(s): " + StringUtil.joinSingleQuoted(extraImageArgs));`
`1107`	`1107`	`}`
`1108`	`1108`	`}`
`1109`	`1109`
`@@ -1147,8 +1147,7 @@ private int completeImageBuild() {`
`1147`	`1147`	`}`
`1148`	`1148`
`1149`	`1149`	`if (!leftoverArgs.isEmpty()) {`
`1150`		`- String prefix = "Unrecognized option" + (leftoverArgs.size() == 1 ? ": " : "s: ");`
`1151`		`- showError(leftoverArgs.stream().collect(Collectors.joining(", ", prefix, "")));`
	`1150`	`+ showError("Unrecognized option(s): " + StringUtil.joinSingleQuoted(leftoverArgs));`
`1152`	`1151`	`}`
`1153`	`1152`
`1154`	`1153`	`LinkedHashSet<Path> finalImageModulePath = new LinkedHashSet<>(imageModulePath);`