From 57d6b1a57dc47709cf20d5bc74958f82d55643a2 Mon Sep 17 00:00:00 2001 From: Rishi Date: Mon, 27 Jan 2014 14:48:20 -0800 Subject: [PATCH 1/3] Better error message while parsing enums --- .../airline/ParseOptionConversionException.java | 9 +++++++-- .../java/io/airlift/airline/TypeConverter.java | 16 ++++++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/main/java/io/airlift/airline/ParseOptionConversionException.java b/src/main/java/io/airlift/airline/ParseOptionConversionException.java index b83ab1cfe..34baabce1 100644 --- a/src/main/java/io/airlift/airline/ParseOptionConversionException.java +++ b/src/main/java/io/airlift/airline/ParseOptionConversionException.java @@ -24,14 +24,19 @@ public class ParseOptionConversionException extends ParseException private final String value; private final String typeName; - ParseOptionConversionException(String optionTitle, String value, String typeName) + ParseOptionConversionException(String optionTitle, String value, String typeName, String additionalComment) { - super("%s: can not convert \"%s\" to a %s", optionTitle, value, typeName); + super("%s: can not convert \"%s\" to a %s. %s", optionTitle, value, typeName, additionalComment); this.optionTitle = optionTitle; this.value = value; this.typeName = typeName; } + ParseOptionConversionException(String optionTitle, String value, String typeName) + { + this(optionTitle, value, typeName, ""); + } + public String getOptionTitle() { return optionTitle; diff --git a/src/main/java/io/airlift/airline/TypeConverter.java b/src/main/java/io/airlift/airline/TypeConverter.java index ae4543f40..6f9feae12 100644 --- a/src/main/java/io/airlift/airline/TypeConverter.java +++ b/src/main/java/io/airlift/airline/TypeConverter.java @@ -1,8 +1,10 @@ package io.airlift.airline; +import com.google.common.base.Joiner; import com.google.common.base.Preconditions; import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; public class TypeConverter @@ -53,16 +55,26 @@ else if (Double.class.isAssignableFrom(type) || Double.TYPE.isAssignableFrom(typ if (valueOf.getReturnType().isAssignableFrom(type)) { return valueOf.invoke(null, value); } - } catch (Throwable ignored) { + } + catch (Throwable ignored) { } // Look for a static valueOf(String) method (this covers enums which have a valueOf method) try { Method valueOf = type.getMethod("valueOf", String.class); if (valueOf.getReturnType().isAssignableFrom(type)) { - return valueOf.invoke(null, value); + try { + return valueOf.invoke(null, value); + } + catch (InvocationTargetException e) { + String errorMsg = "Invalid " + name + ", Valid values are: " + Joiner.on(", ").join(type.getEnumConstants()); + throw new ParseOptionConversionException(name, value, type.getSimpleName(), errorMsg); + } } } + catch (ParseOptionConversionException e) { + throw e; + } catch (Throwable ignored) { } From 9b0d02458f602b7f18e5019d91c8ea80699c71ed Mon Sep 17 00:00:00 2001 From: Rishi Date: Tue, 25 Mar 2014 12:05:28 -0700 Subject: [PATCH 2/3] Address review comments --- src/main/java/io/airlift/airline/TypeConverter.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/airlift/airline/TypeConverter.java b/src/main/java/io/airlift/airline/TypeConverter.java index 6f9feae12..7ef6898d2 100644 --- a/src/main/java/io/airlift/airline/TypeConverter.java +++ b/src/main/java/io/airlift/airline/TypeConverter.java @@ -67,8 +67,10 @@ else if (Double.class.isAssignableFrom(type) || Double.TYPE.isAssignableFrom(typ return valueOf.invoke(null, value); } catch (InvocationTargetException e) { - String errorMsg = "Invalid " + name + ", Valid values are: " + Joiner.on(", ").join(type.getEnumConstants()); - throw new ParseOptionConversionException(name, value, type.getSimpleName(), errorMsg); + if (type.isEnum()) { + String message = String.format("Invalid %s, Valid values are: %s", name, Joiner.on(", ").join(type.getEnumConstants())); + throw new ParseOptionConversionException(name, value, type.getSimpleName(), message); + } } } } From ff7432459816f05bd9f96d76b8d4e563a11c97a9 Mon Sep 17 00:00:00 2001 From: Rishi Date: Thu, 27 Mar 2014 10:46:46 -0700 Subject: [PATCH 3/3] Address review comments --- src/main/java/io/airlift/airline/TypeConverter.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/main/java/io/airlift/airline/TypeConverter.java b/src/main/java/io/airlift/airline/TypeConverter.java index 7ef6898d2..9aadc9b81 100644 --- a/src/main/java/io/airlift/airline/TypeConverter.java +++ b/src/main/java/io/airlift/airline/TypeConverter.java @@ -1,11 +1,15 @@ package io.airlift.airline; +import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Arrays; +import java.util.List; public class TypeConverter { @@ -68,7 +72,14 @@ else if (Double.class.isAssignableFrom(type) || Double.TYPE.isAssignableFrom(typ } catch (InvocationTargetException e) { if (type.isEnum()) { - String message = String.format("Invalid %s, Valid values are: %s", name, Joiner.on(", ").join(type.getEnumConstants())); + List enumConstantNames = Lists.transform(Arrays.asList(((Class) type).getEnumConstants()), new Function() { + @Override + public String apply(Enum input) + { + return input.name(); + } + }); + String message = String.format("Invalid %s, Valid values are: %s", name, Joiner.on(", ").join(enumConstantNames)); throw new ParseOptionConversionException(name, value, type.getSimpleName(), message); } }