Skip to content

Commit d8d144e

Browse files
joewyzjoewyz
committed
Add fixs for nested list / map traits' incorrectly generated field type, , and methods.
1 parent 11af1f9 commit d8d144e

File tree

14 files changed

+258
-38
lines changed

14 files changed

+258
-38
lines changed

‎smithy-trait-codegen/src/it/java/software/amazon/smithy/traitcodegen/test/LoadsFromModelTest.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,11 @@
2424
import com.example.traits.idref.NestedIdRefHolder;
2525
import com.example.traits.lists.DocumentListTrait;
2626
import com.example.traits.lists.ListMember;
27+
import com.example.traits.lists.NestedListTrait;
2728
import com.example.traits.lists.NumberListTrait;
2829
import com.example.traits.lists.StructureListTrait;
2930
import com.example.traits.maps.MapValue;
31+
import com.example.traits.maps.NestedMapTrait;
3032
import com.example.traits.maps.StringDocumentMapTrait;
3133
import com.example.traits.maps.StringStringMapTrait;
3234
import com.example.traits.maps.StringToStructMapTrait;
@@ -45,6 +47,7 @@
4547
import com.example.traits.structures.BasicAnnotationTrait;
4648
import com.example.traits.structures.NestedA;
4749
import com.example.traits.structures.NestedB;
50+
import com.example.traits.structures.StructWithListOfMapTrait;
4851
import com.example.traits.structures.StructureTrait;
4952
import com.example.traits.timestamps.DateTimeTimestampTrait;
5053
import com.example.traits.timestamps.EpochSecondsTimestampTrait;
@@ -155,6 +158,10 @@ static Stream<Arguments> loadsModelTests() {
155158
ObjectNode.builder().withMember("a", "a").build().toNode(),
156159
ObjectNode.builder().withMember("b", "b").withMember("c", 1).build().toNode(),
157160
Node.from("string")))),
161+
Arguments.of("lists/nested-list-trait.smithy",
162+
NestedListTrait.class,
163+
MapUtils.of("getValues",
164+
ListUtils.of(ListUtils.of(ListUtils.of("a"))))),
158165
// Maps
159166
Arguments.of("maps/string-string-map-trait.smithy",
160167
StringStringMapTrait.class,
@@ -185,6 +192,16 @@ static Stream<Arguments> loadsModelTests() {
185192
Node.from("stuff"),
186193
"d",
187194
Node.from(1)))),
195+
Arguments.of("maps/nested-map-trait.smithy",
196+
NestedMapTrait.class,
197+
MapUtils.of("getValues",
198+
MapUtils.of(
199+
"a",
200+
MapUtils.of(
201+
"b",
202+
MapUtils.of(
203+
"c",
204+
"d"))))),
188205
// Mixins
189206
Arguments.of("mixins/struct-with-mixin-member.smithy",
190207
StructureListWithMixinMemberTrait.class,
@@ -284,6 +301,15 @@ static Stream<Arguments> loadsModelTests() {
284301
Optional.empty(),
285302
"getFieldEOrEmpty",
286303
null),
304+
Arguments.of("structures/struct-with-listofmap-trait.smithy",
305+
StructWithListOfMapTrait.class,
306+
MapUtils.of(
307+
"getName",
308+
Optional.of("a"),
309+
"getItems",
310+
Optional.of(
311+
ListUtils.of(
312+
MapUtils.of("b", "c"))))),
287313
// Timestamps
288314
Arguments.of("timestamps/struct-with-nested-timestamps.smithy",
289315
StructWithNestedTimestampsTrait.class,

‎smithy-trait-codegen/src/it/resources/software/amazon/smithy/traitcodegen/test/lists/nested-list-trait.smithy‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
$version: "2.0"
2+
3+
namespace test.smithy.traitcodegen
4+
5+
use test.smithy.traitcodegen.lists#NestedListTrait
6+
7+
@NestedListTrait([[["a"]]])
8+
structure myStruct {}

‎smithy-trait-codegen/src/it/resources/software/amazon/smithy/traitcodegen/test/maps/nested-map-trait.smithy‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
$version: "2.0"
2+
3+
namespace test.smithy.traitcodegen
4+
5+
use test.smithy.traitcodegen.maps#NestedMapTrait
6+
7+
@NestedMapTrait(a:{b:{c:"d"}})
8+
structure myStruct {}

‎smithy-trait-codegen/src/it/resources/software/amazon/smithy/traitcodegen/test/structures/struct-with-listofmap-trait.smithy‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
$version: "2.0"
2+
3+
namespace test.smithy.traitcodegen
4+
5+
use test.smithy.traitcodegen.structures#StructWithListOfMapTrait
6+
7+
@StructWithListOfMapTrait({
8+
name: "a"
9+
items: [{
10+
b:"c"
11+
}]
12+
})
13+
structure myStruct {
14+
15+
}

‎smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/generators/BuilderGenerator.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ private void writeBuilderClass() {
8888
writer.writeDocString(writer.format("Builder for {@link $T}.", symbol));
8989
writer.writeInline("public static final class Builder $C", (Runnable) this::writeBuilderInterface);
9090
writer.indent();
91+
writer.newLine();
9192
baseShape.accept(new BuilderPropertyGenerator());
9293
writer.newLine();
9394
writer.writeWithNoFormatting("private Builder() {}").newLine();
@@ -142,6 +143,8 @@ private void writeToBuilderMethod() {
142143
// Set all builder properties for any members in the shape
143144
if (baseShape.isListShape()) {
144145
writer.writeWithNoFormatting(".values(getValues());");
146+
} else if (baseShape.isMapShape()) {
147+
writer.writeWithNoFormatting(".values(values);");
145148
} else {
146149
Iterator<MemberShape> memberIterator = baseShape.members().iterator();
147150
while (memberIterator.hasNext()) {

‎smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/generators/FromNodeGenerator.java‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ public Void listShape(ListShape shape) {
254254
writer.writeInline(memberPrefix + "ArrayMember($1S, n -> $3C, builder::$2L)",
255255
fieldName,
256256
memberName,
257-
(Runnable) () -> shape.getMember().accept(new FromNodeMapperVisitor(writer, model, "n")));
257+
(Runnable) () -> shape.getMember().accept(new FromNodeMapperVisitor(writer, model, "n", 1)));
258258
return null;
259259
}
260260

@@ -340,10 +340,23 @@ public Void mapShape(MapShape shape) {
340340
+ "ObjectMember($S, o -> o.getMembers().forEach((k, v) -> {\n",
341341
"}))",
342342
fieldName,
343-
() -> writer.write("builder.put$L($C, $C);\n",
344-
StringUtils.capitalize(memberName),
345-
(Runnable) () -> shape.getKey().accept(new FromNodeMapperVisitor(writer, model, "k")),
346-
(Runnable) () -> shape.getValue().accept(new FromNodeMapperVisitor(writer, model, "v"))));
343+
() -> {
344+
writer.write("builder.put$L(\n", StringUtils.capitalize(memberName));
345+
writer.indent();
346+
writer.write("$C,\n",
347+
(Runnable) () -> shape.getKey()
348+
.accept(new FromNodeMapperVisitor(writer,
349+
model,
350+
"k")));
351+
writer.write("$C\n",
352+
(Runnable) () -> shape.getValue()
353+
.accept(new FromNodeMapperVisitor(writer,
354+
model,
355+
"v",
356+
1)));
357+
writer.dedent();
358+
writer.write(");\n");
359+
});
347360
writer.enableNewlines();
348361
return null;
349362
}

‎smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/generators/FromNodeMapperVisitor.java‎

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

77
import java.time.Instant;
88
import java.time.format.DateTimeFormatter;
9+
import java.util.stream.Collectors;
910
import software.amazon.smithy.model.Model;
1011
import software.amazon.smithy.model.shapes.BigDecimalShape;
1112
import software.amazon.smithy.model.shapes.BigIntegerShape;
@@ -42,11 +43,17 @@ final class FromNodeMapperVisitor extends ShapeVisitor.DataShapeVisitor<Void> {
4243
private final TraitCodegenWriter writer;
4344
private final Model model;
4445
private final String varName;
46+
private final int nestedLevel;
4547

4648
FromNodeMapperVisitor(TraitCodegenWriter writer, Model model, String varName) {
49+
this(writer, model, varName, 0);
50+
}
51+
52+
FromNodeMapperVisitor(TraitCodegenWriter writer, Model model, String varName, int nestedLevel) {
4753
this.writer = writer;
4854
this.model = model;
4955
this.varName = varName;
56+
this.nestedLevel = nestedLevel;
5057
}
5158

5259
@Override
@@ -60,21 +67,69 @@ public Void listShape(ListShape shape) {
6067
writer.write("$L.expectArrayNode()", varName);
6168
writer.indent();
6269
writer.writeWithNoFormatting(".getElements().stream()");
63-
writer.write(".map(n -> $C)",
64-
(Runnable) () -> shape.getMember().accept(new FromNodeMapperVisitor(writer, model, "n")));
65-
writer.writeWithNoFormatting(".forEach(builder::addValues);");
70+
if (nestedLevel == 0) { // Triggered when shape is a member.
71+
writer.write(".map(n -> $C)",
72+
(Runnable) () -> shape.getMember()
73+
.accept(new FromNodeMapperVisitor(writer,
74+
model,
75+
"n",
76+
nestedLevel + 1)));
77+
writer.writeWithNoFormatting(".forEach(builder::addValues);");
78+
} else { // Triggered when shape is nested.
79+
writer.write(".map($L -> $C)",
80+
varName + nestedLevel,
81+
(Runnable) () -> shape.getMember()
82+
.accept(new FromNodeMapperVisitor(writer,
83+
model,
84+
varName + nestedLevel,
85+
nestedLevel + 1)));
86+
writer.write(".collect($T.toList())", Collectors.class);
87+
}
6688
writer.dedent();
6789
return null;
6890
}
6991

7092
@Override
7193
public Void mapShape(MapShape shape) {
72-
writer.openBlock("$L.expectObjectNode().getMembers().forEach((k, v) -> {",
73-
"});",
74-
varName,
75-
() -> writer.write("builder.putValues($C, $C);",
76-
(Runnable) () -> shape.getKey().accept(new FromNodeMapperVisitor(writer, model, "k")),
77-
(Runnable) () -> shape.getValue().accept(new FromNodeMapperVisitor(writer, model, "v"))));
94+
if (nestedLevel == 0) { // Triggered when shape is a member.
95+
writer.openBlock("$L.expectObjectNode().getMembers().forEach((k, v) -> {",
96+
"});",
97+
varName,
98+
() -> writer.write("builder.putValues($C, $C);",
99+
(Runnable) () -> shape.getKey()
100+
.accept(new FromNodeMapperVisitor(writer,
101+
model,
102+
"k")),
103+
(Runnable) () -> shape.getValue()
104+
.accept(new FromNodeMapperVisitor(writer,
105+
model,
106+
"v",
107+
nestedLevel + 1))));
108+
} else { // Triggered when shape is nested.
109+
String entryName = "entry" + nestedLevel;
110+
writer.write("$L.expectObjectNode()", varName);
111+
writer.indent();
112+
writer.writeWithNoFormatting(".getMembers().entrySet().stream()");
113+
writer.indent();
114+
writer.write(".collect($T.toMap(", Collectors.class);
115+
writer.indent();
116+
writer.write("$L -> $C, ",
117+
entryName,
118+
(Runnable) () -> shape.getKey()
119+
.accept(new FromNodeMapperVisitor(writer,
120+
model,
121+
entryName + ".getKey()")));
122+
writer.write("$L -> $C",
123+
entryName,
124+
(Runnable) () -> shape.getValue()
125+
.accept(new FromNodeMapperVisitor(writer,
126+
model,
127+
entryName + ".getValue()",
128+
nestedLevel + 1)));
129+
writer.dedent();
130+
writer.write("))");
131+
writer.dedent();
132+
}
78133
return null;
79134
}
80135

‎smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/generators/ToNodeGenerator.java‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public Void listShape(ListShape shape) {
7373
writer.write("return values.stream()")
7474
.indent()
7575
.write(".map(s -> $C)",
76-
(Runnable) () -> shape.getMember().accept(new ToNodeMapperVisitor("s")))
76+
(Runnable) () -> shape.getMember().accept(new ToNodeMapperVisitor("s", 1)))
7777
.write(".collect($T.collect(getSourceLocation()));", ArrayNode.class)
7878
.dedent();
7979
return null;
@@ -101,10 +101,10 @@ public Void mapShape(MapShape shape) {
101101
.write("$C, $C))",
102102
(Runnable) () -> shape.getKey()
103103
.accept(
104-
new ToNodeMapperVisitor("entry.getKey()")),
104+
new ToNodeMapperVisitor("entry.getKey()", 1)),
105105
(Runnable) () -> shape.getValue()
106106
.accept(
107-
new ToNodeMapperVisitor("entry.getValue()")))
107+
new ToNodeMapperVisitor("entry.getValue()", 1)))
108108
.dedent()
109109
.write(".collect($1T.collect($2T::getKey, $2T::getValue))",
110110
ObjectNode.class,
@@ -203,9 +203,15 @@ private void toStringCreator() {
203203
*/
204204
private final class ToNodeMapperVisitor extends TraitVisitor<Void> {
205205
private final String varName;
206+
private final int nestedLevel;
206207

207208
ToNodeMapperVisitor(String varName) {
209+
this(varName, 0);
210+
}
211+
212+
ToNodeMapperVisitor(String varName, int nestedLevel) {
208213
this.varName = varName;
214+
this.nestedLevel = nestedLevel;
209215
}
210216

211217
@Override
@@ -226,27 +232,41 @@ public Void booleanShape(BooleanShape shape) {
226232

227233
@Override
228234
public Void listShape(ListShape shape) {
229-
writer.write("$L.stream().map(s -> $C).collect($T.collect())",
230-
varName,
231-
(Runnable) () -> shape.getMember().accept(new ToNodeMapperVisitor("s")),
232-
ArrayNode.class);
235+
if (nestedLevel == 0) {
236+
writer.write("$L.stream().map(s -> $C).collect($T.collect())",
237+
varName,
238+
(Runnable) () -> shape.getMember().accept(new ToNodeMapperVisitor("s", nestedLevel + 1)),
239+
ArrayNode.class);
240+
} else {
241+
writer.write("$L.stream().map($L -> $C).collect($T.collect())",
242+
varName,
243+
"s" + nestedLevel,
244+
(Runnable) () -> shape.getMember()
245+
.accept(new ToNodeMapperVisitor("s" + nestedLevel,
246+
nestedLevel + 1)),
247+
ArrayNode.class);
248+
}
249+
233250
return null;
234251
}
235252

236253
@Override
237254
public Void mapShape(MapShape shape) {
255+
String entryName = "entry" + nestedLevel;
238256
writer.openBlock("$L.entrySet().stream()",
239257
"",
240258
varName,
241-
() -> writer.write(".map(entry -> new $T<>(", AbstractMap.SimpleImmutableEntry.class)
259+
() -> writer.write(".map($L -> new $T<>(", entryName, AbstractMap.SimpleImmutableEntry.class)
242260
.indent()
243261
.write("$C, $C))",
244262
(Runnable) () -> shape.getKey()
245-
.accept(
246-
new ToNodeMapperVisitor("entry.getKey()")),
263+
.accept(new ToNodeMapperVisitor(
264+
entryName + ".getKey()",
265+
nestedLevel + 1)),
247266
(Runnable) () -> shape.getValue()
248-
.accept(
249-
new ToNodeMapperVisitor("entry.getValue()")))
267+
.accept(new ToNodeMapperVisitor(
268+
entryName + ".getValue()",
269+
nestedLevel + 1)))
250270
.dedent()
251271
.write(".collect($1T.collect($2T::getKey, $2T::getValue))",
252272
ObjectNode.class,

‎smithy-trait-codegen/src/main/java/software/amazon/smithy/traitcodegen/writer/TraitCodegenWriter.java‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -215,17 +215,7 @@ public String apply(Object type, String indent) {
215215

216216
// Add type references as type references (ex. `List<InnerType>`)
217217
StringBuilder builder = new StringBuilder();
218-
builder.append(getPlaceholder(typeSymbol));
219-
builder.append("<");
220-
Iterator<SymbolReference> iterator = typeSymbol.getReferences().iterator();
221-
while (iterator.hasNext()) {
222-
String placeholder = getPlaceholder(iterator.next().getSymbol());
223-
builder.append(placeholder);
224-
if (iterator.hasNext()) {
225-
builder.append(", ");
226-
}
227-
}
228-
builder.append(">");
218+
processSymbol(typeSymbol, builder);
229219
return builder.toString();
230220
}
231221

@@ -242,6 +232,32 @@ private String getPlaceholder(Symbol symbol) {
242232
// Return a placeholder value that will be filled when toString is called
243233
return format("$${$L:L}", normalizedSymbol.getFullName());
244234
}
235+
236+
private boolean isGenericType(Symbol symbol) {
237+
String name = symbol.getName();
238+
return name.equals("List") || name.equals("Map")
239+
|| name.equals("Set")
240+
|| name.equals("Collection");
241+
}
242+
243+
// Recursively process the symbols using Java Type.
244+
private void processSymbol(Symbol symbol, StringBuilder builder) {
245+
builder.append(getPlaceholder(symbol));
246+
if (isGenericType(symbol)) {
247+
builder.append("<");
248+
}
249+
Iterator<SymbolReference> iterator = symbol.getReferences().iterator();
250+
while (iterator.hasNext()) {
251+
Symbol referenceSymbol = iterator.next().getSymbol();
252+
processSymbol(referenceSymbol, builder);
253+
if (iterator.hasNext()) {
254+
builder.append(", ");
255+
}
256+
}
257+
if (isGenericType(symbol)) {
258+
builder.append(">");
259+
}
260+
}
245261
}
246262

247263
/**

0 commit comments

Comments
 (0)