Skip to content

Commit fa47f27

Browse files
h908714124h908714124
committed
forbid plain String parameters
1 parent f5cf257 commit fa47f27

File tree

9 files changed

+77
-73
lines changed

9 files changed

+77
-73
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ In this constructor, only three types of parameters are allowed:
3232

3333
* A `boolean` parameter declares a flag.
3434
* A `List<String>` parameter declares a repeatable argument.
35-
* A `String` or `Optional<String>` parameter declares an argument that may appear at most once.
35+
* A `Optional<String>` parameter declares an argument that may appear at most once.
3636

3737
The following additional rules apply:
3838

‎core/src/main/java/net/jbock/compiler/Analyser.java‎

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import static javax.lang.model.element.Modifier.PRIVATE;
2626
import static javax.lang.model.element.Modifier.PUBLIC;
2727
import static javax.lang.model.element.Modifier.STATIC;
28+
import static net.jbock.compiler.OptionType.REPEATABLE;
2829

2930
final class Analyser {
3031

@@ -112,26 +113,35 @@ private static MethodSpec removeFirstFlagMethod() {
112113
.build();
113114
}
114115

115-
private static MethodSpec checkConflictMethod(TypeName optionMapType, ClassName optionClass,
116-
ClassName optionTypeClass, FieldSpec optionType) {
117-
ParameterSpec optionMap = ParameterSpec.builder(optionMapType, "optionMap").build();
116+
private static MethodSpec checkConflictMethod(
117+
TypeName optionMapType,
118+
ClassName optionClass,
119+
ClassName optionTypeClass,
120+
FieldSpec optionType) {
121+
ParameterSpec optMap = ParameterSpec.builder(optionMapType, "optionMap").build();
118122
ParameterSpec token = ParameterSpec.builder(STRING, "token").build();
119123
ParameterSpec option = ParameterSpec.builder(optionClass, "option").build();
120124
ParameterSpec message = ParameterSpec.builder(STRING, "message").build();
125+
ParameterSpec bucket = ParameterSpec.builder(STRING_LIST, "bucket").build();
126+
ParameterSpec ignore = ParameterSpec.builder(optionClass, "__").build();
121127
CodeBlock block = CodeBlock.builder()
122-
.beginControlFlow("if ($N.$N == $T.$L)", option, optionType, optionTypeClass, OptionType.REPEATABLE)
123-
.addStatement("return")
128+
.addStatement("$T $N = $N.computeIfAbsent($N, $N -> new $T<>())",
129+
bucket.type, bucket, optMap, option, ignore, ArrayList.class)
130+
.beginControlFlow("if ($N.$N == $T.$L)", option, optionType, optionTypeClass, REPEATABLE)
131+
.addStatement("return $N", bucket)
124132
.endControlFlow()
125-
.beginControlFlow("if ($N.containsKey($N))", optionMap, option)
126-
.addStatement("$T $N = $N.$N == $T.$L ? $S : $S", STRING, message, option, optionType,
133+
.beginControlFlow("if (!$N.isEmpty())", bucket)
134+
.addStatement("$T $N = $N.$N == $T.$L ?\n $S :\n $S", STRING, message, option, optionType,
127135
optionTypeClass, OptionType.FLAG, "Duplicate flag", "Conflicting token")
128136
.addStatement("throw new $T($N + $S + $N)", IllegalArgumentException.class,
129137
message, ": ", token)
130138
.endControlFlow()
139+
.addStatement("return $N", bucket)
131140
.build();
132141
return MethodSpec.methodBuilder("checkConflict")
133-
.addParameters(Arrays.asList(optionMap, option, token))
142+
.addParameters(Arrays.asList(optMap, option, token))
134143
.addCode(block)
144+
.returns(STRING_LIST)
135145
.addModifiers(PRIVATE, STATIC)
136146
.build();
137147
}
@@ -200,9 +210,10 @@ private MethodSpec parseMethod() {
200210
.build();
201211
}
202212

203-
private static MethodSpec readOptionMethod(ClassName keysClass,
204-
FieldSpec longNames, FieldSpec shortNames,
205-
ClassName optionClass) {
213+
private static MethodSpec readOptionMethod(
214+
ClassName keysClass,
215+
FieldSpec longNames, FieldSpec shortNames,
216+
ClassName optionClass) {
206217
ParameterSpec names = ParameterSpec.builder(keysClass, "names").build();
207218
ParameterSpec token = ParameterSpec.builder(STRING, "token").build();
208219
ParameterSpec idxe = ParameterSpec.builder(INT, "idxe").build();
@@ -211,18 +222,14 @@ private static MethodSpec readOptionMethod(ClassName keysClass,
211222
.beginControlFlow("if ($N.length() < 2 || !$N.startsWith($S))", token, token, "-")
212223
.addStatement("return null")
213224
.endControlFlow()
214-
.beginControlFlow("if ($N.startsWith($S))", token, "--")
215-
.addStatement("$T $N = $N.indexOf('=')", INT, idxe, token)
216-
.beginControlFlow("if ($N < 0)", idxe)
217-
.addStatement("return $N.$N.get($N.substring(2))",
218-
names, longNames, token)
219-
.endControlFlow()
220-
.addStatement("return $N.$N.get($N.substring(2, $N))",
221-
names, longNames, token, idxe)
225+
.beginControlFlow("if (!$N.startsWith($S))", token, "--")
226+
.addStatement("return $N.$N.get($N.substring(1, 2))", names, shortNames, token)
222227
.endControlFlow()
223-
.addStatement("return $N.$N.get($N.substring(1, 2))",
224-
names, shortNames, token);
225-
228+
.addStatement("$T $N = $N.indexOf('=')", INT, idxe, token)
229+
.beginControlFlow("if ($N < 0)", idxe)
230+
.addStatement("return $N.$N.get($N.substring(2))", names, longNames, token)
231+
.endControlFlow()
232+
.addStatement("return $N.$N.get($N.substring(2, $N))", names, longNames, token, idxe);
226233
//@formatter:on
227234
return MethodSpec.methodBuilder("readOption")
228235
.addParameters(Arrays.asList(names, token))
@@ -265,54 +272,52 @@ private static MethodSpec readArgumentMethod() {
265272
.build();
266273
}
267274

268-
private static MethodSpec readMethod(ClassName keysClass,
269-
MethodSpec readOption,
270-
MethodSpec readArgument,
271-
TypeName optionMapType,
272-
ClassName optionClass,
273-
FieldSpec optionType,
274-
ClassName optionTypeClass,
275-
MethodSpec checkConflict,
276-
MethodSpec removeFirstFlag) {
275+
private static MethodSpec readMethod(
276+
ClassName keysClass,
277+
MethodSpec readOption,
278+
MethodSpec readArgument,
279+
TypeName optionMapType,
280+
ClassName optionClass,
281+
FieldSpec optionType,
282+
ClassName optionTypeClass,
283+
MethodSpec checkConflict,
284+
MethodSpec removeFirstFlag) {
277285
ParameterSpec names = ParameterSpec.builder(keysClass, "names").build();
278286
ParameterSpec optMap = ParameterSpec.builder(optionMapType, "optMap").build();
279287
ParameterSpec otherTokens = ParameterSpec.builder(STRING_LIST, "otherTokens").build();
280288
ParameterSpec it = ParameterSpec.builder(STRING_ITERATOR, "it").build();
281289
ParameterSpec bucket = ParameterSpec.builder(STRING_LIST, "bucket").build();
282290

291+
ParameterSpec originalToken = ParameterSpec.builder(STRING, "originalToken").build();
283292
ParameterSpec token = ParameterSpec.builder(STRING, "token").build();
284293
ParameterSpec option = ParameterSpec.builder(optionClass, "option").build();
285294
ParameterSpec ignore = ParameterSpec.builder(optionClass, "__").build();
286295
//@formatter:off
287296
CodeBlock.Builder builder = CodeBlock.builder()
297+
.addStatement("$T $N = $N", STRING, token, originalToken)
288298
.addStatement("$T $N = $N($N, $N)", option.type, option, readOption, names, token)
289299
.beginControlFlow("if ($N == null)", option)
290300
.addStatement("$N.add($N)", otherTokens, token)
291301
.addStatement("return")
292302
.endControlFlow()
293-
.addStatement("$N($N, $N, $N)", checkConflict, optMap, option, token)
294-
.addStatement("$T $N = $N.computeIfAbsent($N, $N -> new $T<>())",
295-
bucket.type, bucket, optMap, option, ignore, ArrayList.class)
296303
.beginControlFlow("while ($N.$N == $T.$L)", option, optionType, optionTypeClass, OptionType.FLAG)
304+
.addStatement("$T $N = $N($N, $N, $N)", bucket.type, bucket, checkConflict, optMap, option, token)
297305
.addStatement("$N.add($S)", bucket, "t")
298306
.addStatement("$N = $N($N)", token, removeFirstFlag, token)
299307
.beginControlFlow("if ($N == null)", token)
300-
.addStatement("break")
308+
.addStatement("return")
301309
.endControlFlow()
302310
.addStatement("$N = $N($N, $N)", option, readOption, names, token)
303311
.beginControlFlow("if ($N == null)", option)
304-
.addStatement("throw new $T($S)", IllegalArgumentException.class,
305-
"invalid token")
312+
.addStatement("throw new $T($S + $N)", IllegalArgumentException.class,
313+
"invalid token: ", originalToken)
306314
.endControlFlow()
307-
.addStatement("$N = $N.computeIfAbsent($N, $N -> new $T<>())",
308-
bucket, optMap, option, ignore, ArrayList.class)
309315
.endControlFlow()
310-
.beginControlFlow("if ($N.type != $T.$L)", option, optionTypeClass, OptionType.FLAG)
311-
.addStatement("$N.add($N($N, $N))", bucket, readArgument, token, it)
312-
.endControlFlow();
316+
.addStatement("$T $N = $N($N, $N, $N)", bucket.type, bucket, checkConflict, optMap, option, token)
317+
.addStatement("$N.add($N($N, $N))", bucket, readArgument, token, it);
313318
//@formatter:on
314319
return MethodSpec.methodBuilder("read")
315-
.addParameters(Arrays.asList(token, names, optMap, otherTokens, it))
320+
.addParameters(Arrays.asList(originalToken, names, optMap, otherTokens, it))
316321
.addModifiers(STATIC, PRIVATE)
317322
.addCode(builder.build())
318323
.build();

‎core/src/main/java/net/jbock/compiler/Binder.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,7 @@ private MethodSpec bindMethod() {
6666
OptionType optionType = constructor.parameters.get(j).optionType;
6767
if (optionType == OptionType.FLAG) {
6868
builder.add("$N.containsKey($T.$N)", optMap, option.optionClass, option.enumConstant(j));
69-
} else if (optionType == OptionType.AT_MOST_ONCE) {
70-
builder.add("$N.getOrDefault($T.$L, $T.emptyList()).stream()\n",
71-
optMap, option.optionClass, option.enumConstant(j), Collections.class)
72-
.add(" .findFirst().orElse(null)");
73-
} else if (optionType == OptionType.AT_MOST_ONCE_OPTIONAL) {
69+
} else if (optionType == OptionType.OPTIONAL) {
7470
builder.add("$N.getOrDefault($T.$L, $T.emptyList()).stream()\n",
7571
optMap, option.optionClass, option.enumConstant(j), Collections.class)
7672
.add(" .findFirst()");

‎core/src/main/java/net/jbock/compiler/OptionType.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
enum OptionType {
99

10-
FLAG, AT_MOST_ONCE, AT_MOST_ONCE_OPTIONAL, REPEATABLE, OTHER_TOKENS, EVERYTHING_AFTER;
10+
FLAG, OPTIONAL, REPEATABLE, OTHER_TOKENS, EVERYTHING_AFTER;
1111

1212
static TypeSpec define(ClassName optionTypeClass) {
1313
TypeSpec.Builder builder = TypeSpec.enumBuilder(optionTypeClass);

‎core/src/main/java/net/jbock/compiler/Param.java‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,16 +51,13 @@ private static OptionType getOptionType(VariableElement var) {
5151
if (type.getKind() == TypeKind.BOOLEAN) {
5252
return OptionType.FLAG;
5353
}
54-
if (equalsType(type, "java.lang.String")) {
55-
return OptionType.AT_MOST_ONCE;
56-
}
5754
if (isListOfString(type)) {
5855
return OptionType.REPEATABLE;
5956
}
6057
if (isOptionalString(type)) {
61-
return OptionType.AT_MOST_ONCE_OPTIONAL;
58+
return OptionType.OPTIONAL;
6259
}
63-
String message = "Only String, Optional<String>, boolean or List<String> allowed, " +
60+
String message = "Only Optional<String>, List<String> and boolean allowed, " +
6461
String.format("but parameter %s has type %s", var.getSimpleName(), type);
6562
throw new ValidationException(message, var);
6663
}

‎core/src/test/java/net/jbock/compiler/ProcessorTest.java‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ public void process() throws Exception {
1919
"package test;",
2020
"import net.jbock.CommandLineArguments;",
2121
"import net.jbock.LongName;",
22+
"import java.util.Optional;",
2223
"class JJob {",
23-
" @CommandLineArguments JJob(@LongName(\"x\") String a, String b) {}",
24+
" @CommandLineArguments JJob(@LongName(\"x\") Optional<String> a, Optional<String> b) {}",
2425
"}");
2526
JavaFileObject javaFile = forSourceLines("test.JJobParser", sourceLines);
2627
assertAbout(javaSources()).that(singletonList(javaFile))
@@ -34,8 +35,9 @@ public void duplicateName() throws Exception {
3435
"package test;",
3536
"import net.jbock.CommandLineArguments;",
3637
"import net.jbock.LongName;",
38+
"import java.util.Optional;",
3739
"class JJob {",
38-
" @CommandLineArguments JJob(@LongName(\"x\") String a, @LongName(\"x\") String b) {}",
40+
" @CommandLineArguments JJob(@LongName(\"x\") Optional<String> a, @LongName(\"x\") Optional<String> b) {}",
3941
"}");
4042
JavaFileObject javaFile = forSourceLines("test.JJobParser", sourceLines);
4143
assertAbout(javaSources()).that(singletonList(javaFile))
@@ -57,8 +59,8 @@ public void wrongType() throws Exception {
5759
assertAbout(javaSources()).that(singletonList(javaFile))
5860
.processedWith(new Processor())
5961
.failsToCompile()
60-
.withErrorContaining(
61-
"Only String, Optional<String>, boolean or List<String> allowed, but parameter a has type int");
62+
.withErrorContaining("Only Optional<String>, List<String> and boolean allowed, " +
63+
"but parameter a has type int");
6264
}
6365

6466
@Test
@@ -67,6 +69,7 @@ public void privateException() throws Exception {
6769
"package test;",
6870
"import net.jbock.CommandLineArguments;",
6971
"import net.jbock.LongName;",
72+
"import java.util.Optional;",
7073
"class JJob {",
7174
" @CommandLineArguments JJob(String a) throws Hammer {}",
7275
" private static final class Hammer extends Exception {}",
@@ -84,8 +87,9 @@ public void whitespace() throws Exception {
8487
"package test;",
8588
"import net.jbock.CommandLineArguments;",
8689
"import net.jbock.LongName;",
90+
"import java.util.Optional;",
8791
"class JJob {",
88-
" @CommandLineArguments JJob(@LongName(\"a b c\") String a) {}",
92+
" @CommandLineArguments JJob(@LongName(\"a b c\") Optional<String> a) {}",
8993
"}");
9094
JavaFileObject javaFile = forSourceLines("test.JJobParser", sourceLines);
9195
assertAbout(javaSources()).that(singletonList(javaFile))
@@ -107,9 +111,8 @@ public void booleanWrapper() throws Exception {
107111
assertAbout(javaSources()).that(singletonList(javaFile))
108112
.processedWith(new Processor())
109113
.failsToCompile()
110-
.withErrorContaining(
111-
"Only String, Optional<String>, boolean or List<String> allowed, " +
112-
"but parameter a has type java.lang.Boolean");
114+
.withErrorContaining("Only Optional<String>, List<String> and boolean allowed, " +
115+
"but parameter a has type java.lang.Boolean");
113116
}
114117

115118
@Test
@@ -137,8 +140,9 @@ public void everythingAfterCollidesWithOption() throws Exception {
137140
"import net.jbock.CommandLineArguments;",
138141
"import net.jbock.EverythingAfter;",
139142
"import java.util.List;",
143+
"import java.util.Optional;",
140144
"class JJob {",
141-
" @CommandLineArguments JJob(String a, @EverythingAfter(\"--a\") List<String> b) {}",
145+
" @CommandLineArguments JJob(Optional<String> a, @EverythingAfter(\"--a\") List<String> b) {}",
142146
"}");
143147
JavaFileObject javaFile = forSourceLines("test.JJobParser", sourceLines);
144148
assertAbout(javaSources()).that(singletonList(javaFile))

‎examples/src/main/java/net/zerobuilder/examples/gradle/GradleMan.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import java.io.IOException;
1111
import java.util.List;
12+
import java.util.Optional;
1213

1314
final class GradleMan {
1415

@@ -22,8 +23,8 @@ static final class Foo {
2223
final String bar;
2324

2425
@CommandLineArguments
25-
Foo(String bar) {
26-
this.bar = bar;
26+
Foo(Optional<String> bar) {
27+
this.bar = bar.orElse(null);
2728
}
2829
}
2930

@@ -32,22 +33,22 @@ static final class Foo {
3233
@ShortName('m')
3334
@ArgumentName("MESSAGE")
3435
@Description({"the message", "message goes here"})
35-
String message,
36+
Optional<String> message,
3637
@ShortName('f')
3738
@Description("the files")
3839
@ArgumentName("FILE")
3940
List<String> file,
4041
@Description("the dir")
4142
@ArgumentName("DIR")
42-
String dir,
43+
Optional<String> dir,
4344
@ShortName('c')
4445
@Description("cmos flag")
4546
boolean cmos,
4647
@ShortName('v')
4748
boolean verbose) throws IOException, NullPointerException {
48-
this.message = message;
49+
this.message = message.orElse(null);
4950
this.file = file;
50-
this.dir = dir;
51+
this.dir = dir.orElse(null);
5152
this.cmos = cmos;
5253
this.verbose = verbose;
5354
}

‎examples/src/main/java/net/zerobuilder/examples/gradle/NoName.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import net.jbock.CommandLineArguments;
55

66
import java.util.List;
7+
import java.util.Optional;
78

89
final class NoName {
910

@@ -12,10 +13,10 @@ final class NoName {
1213
final boolean cmos;
1314

1415
@CommandLineArguments
15-
NoName(String message,
16+
NoName(Optional<String> message,
1617
List<String> file,
1718
boolean cmos) {
18-
this.message = message;
19+
this.message = message.orElse(null);
1920
this.file = file;
2021
this.cmos = cmos;
2122
}

‎examples/src/test/java/net/zerobuilder/examples/gradle/GradleManTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ public void testMessageOption() throws Exception {
216216
assertThat(Option.MESSAGE.description().size(), is(2));
217217
assertThat(Option.MESSAGE.description().get(0), is("the message"));
218218
assertThat(Option.MESSAGE.description().get(1), is("message goes here"));
219-
assertThat(Option.MESSAGE.type(), is(OptionType.AT_MOST_ONCE));
219+
assertThat(Option.MESSAGE.type(), is(OptionType.OPTIONAL));
220220
assertThat(Option.MESSAGE.longName(), is("message"));
221221
assertThat(Option.MESSAGE.shortName(), is("m"));
222222
assertThat(Option.MESSAGE.descriptionParameter(), is("MESSAGE"));

0 commit comments

Comments
 (0)