Skip to content

Commit 6465976

Browse files
jhalexandjhalexand
committed
Give clearer errors when there are access problems with Options classes (#13)
* Give clearer messages when there are access problems with Options classes * Added explicit test for missing a no-args constructor * Replaced tabs with spaces to fix formatting
1 parent c608df6 commit 6465976

File tree

2 files changed

+89
-22
lines changed

2 files changed

+89
-22
lines changed

‎olcut-core/src/main/java/com/oracle/labs/mlrg/olcut/config/Options.java

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import com.oracle.labs.mlrg.olcut.util.Pair;
3232

33+
import java.lang.reflect.Constructor;
3334
import java.lang.reflect.Field;
3435
import java.lang.reflect.InvocationTargetException;
3536
import java.lang.reflect.Modifier;
@@ -154,8 +155,21 @@ public static List<List<String>> getUsage(Class<? extends Options> options) {
154155
list.addAll(optionsList);
155156

156157
return list;
157-
} catch (IllegalAccessException | InstantiationException | NoSuchMethodException | InvocationTargetException e) {
158-
throw new ArgumentException(e,"Could not instantiate Options class " + options.getName() +", it has no default constructor.");
158+
} catch (InstantiationException | NoSuchMethodException | InvocationTargetException e) {
159+
throw new ArgumentException(e,"Could not instantiate Options class " + options.getName() + ", it has no default constructor.");
160+
} catch (IllegalAccessException e) {
161+
if (!Modifier.isPublic(options.getModifiers())) {
162+
throw new ArgumentException(e,"Could not instantiate Options class " + options.getName() + ", it must be public.");
163+
}
164+
try {
165+
Constructor constructor = options.getDeclaredConstructor();
166+
if (!Modifier.isPublic(constructor.getModifiers())) {
167+
throw new ArgumentException(e,"Could not instantiate Options class " + options.getName() + ", its default constructor must be public.");
168+
}
169+
} catch (NoSuchMethodException nsme) {
170+
throw new ArgumentException(e,"Could not instantiate Options class " + options.getName() + ", it has no default constructor.");
171+
}
172+
throw new ArgumentException(e,"Could not instantiate Options class " + options.getName());
159173
}
160174

161175
}

‎olcut-core/src/test/java/com/oracle/labs/mlrg/olcut/config/OptionsTest.java

Lines changed: 73 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -36,23 +36,24 @@
3636
import java.util.Random;
3737

3838
import org.junit.jupiter.api.Test;
39+
import org.junit.jupiter.api.function.Executable;
3940

40-
import static org.junit.jupiter.api.Assertions.assertEquals;
41+
import static org.junit.jupiter.api.Assertions.*;
4142

4243
public class OptionsTest {
4344

44-
@Test
45-
public void testBasic() {
46-
String[] args = new String[] {"--deeper-string","How low can you go?",
47-
"--deep-string", "double bass",
48-
"--enum", "THINGS",
49-
"--output-string", "ringstay putoutay",
50-
"--seed", "42",
51-
"--baz", "X,Y,Z",
52-
"--output-file", "/tmp/output.txt",
53-
"--input-file", "/tmp/input.txt",
54-
"--pi", "3.1415927"};
55-
TestOptions options = new TestOptions();
45+
@Test
46+
public void testBasic() {
47+
String[] args = new String[] {"--deeper-string","How low can you go?",
48+
"--deep-string", "double bass",
49+
"--enum", "THINGS",
50+
"--output-string", "ringstay putoutay",
51+
"--seed", "42",
52+
"--baz", "X,Y,Z",
53+
"--output-file", "/tmp/output.txt",
54+
"--input-file", "/tmp/input.txt",
55+
"--pi", "3.1415927"};
56+
TestOptions options = new TestOptions();
5657
ConfigurationManager cm = new ConfigurationManager(args,options);
5758

5859
assertEquals(3.1415927d, options.pi, 0.00001);
@@ -69,18 +70,18 @@ public void testBasic() {
6970
String[] unparsedArgs = cm.getUnnamedArguments();
7071
assertEquals(0, unparsedArgs.length);
7172

72-
}
73-
74-
@Test
75-
public void testBackSlash() {
73+
}
74+
75+
@Test
76+
public void testBackSlash() {
7677
String deeperStringValue = ConfigurationManager.IS_WINDOWS ? "\\s+" : "\\\\s+";
77-
String[] args = new String[] {"--deeper-string", deeperStringValue};
78-
TestOptions options = new TestOptions();
78+
String[] args = new String[] {"--deeper-string", deeperStringValue};
79+
TestOptions options = new TestOptions();
7980
ConfigurationManager cm = new ConfigurationManager(args,options);
8081
assertEquals("\\s+", options.bar.deepOptions.deeperOptions.deeperString);
8182
cm.close();
8283

83-
}
84+
}
8485

8586
public static class CommaOptions implements Options {
8687
@Option(longName="my-chars",usage="The characters.")
@@ -99,6 +100,58 @@ public void testComma() {
99100
assertEquals('b', options.myChars[2]);
100101
assertEquals('c', options.myChars[3]);
101102
}
103+
104+
private static class PrivateClassOptions implements Options {
105+
@Option(charName='s',longName="something",usage="Provide something")
106+
public String s;
107+
}
108+
109+
public static class PrivateConstructorOptions implements Options {
110+
private PrivateConstructorOptions() {
111+
}
112+
113+
@Option(charName='s',longName="something",usage="Provide something")
114+
public String s;
115+
}
116+
117+
public static class NoDefaultConstructorOptions implements Options {
118+
public NoDefaultConstructorOptions(String aValue) {
119+
}
120+
121+
@Option(charName='s',longName="something",usage="Provide something")
122+
public String s;
123+
}
124+
125+
@Test
126+
public void testAccess() {
127+
PrivateClassOptions one = new PrivateClassOptions();
128+
Exception e = assertThrows(ArgumentException.class, new Executable() {
129+
@Override
130+
public void execute() throws Throwable {
131+
ConfigurationManager cm = new ConfigurationManager(new String[]{"-s", "donkey"}, one);
132+
}
133+
});
134+
assertTrue(e.getMessage().contains("must be public"));
135+
136+
PrivateConstructorOptions two = new PrivateConstructorOptions();
137+
e = assertThrows(ArgumentException.class, new Executable() {
138+
@Override
139+
public void execute() throws Throwable {
140+
ConfigurationManager cm = new ConfigurationManager(new String[]{"-s", "donkey"}, two);
141+
}
142+
});
143+
assertTrue(e.getMessage().contains("default constructor must be public"));
144+
145+
NoDefaultConstructorOptions three = new NoDefaultConstructorOptions("donkey");
146+
e = assertThrows(ArgumentException.class, new Executable() {
147+
@Override
148+
public void execute() throws Throwable {
149+
ConfigurationManager cm = new ConfigurationManager(new String[]{"-s", "donkey"}, three);
150+
}
151+
});
152+
assertTrue(e.getMessage().contains("no default constructor"));
153+
154+
}
102155
}
103156

104157
class TestOptions implements Options {

0 commit comments

Comments
 (0)