Skip to content

Commit 9821ec9

Browse files
kuhekuhe
authored
improve documentation truncation behavior (#728)
* improve documentation truncation behavior * add recursive test for structural doc gen
1 parent 2409063 commit 9821ec9

File tree

2 files changed

+104
-18
lines changed

2 files changed

+104
-18
lines changed

‎smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/documentation/StructureExampleGenerator.java

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import software.amazon.smithy.model.shapes.ListShape;
1919
import software.amazon.smithy.model.shapes.MapShape;
2020
import software.amazon.smithy.model.shapes.MemberShape;
21+
import software.amazon.smithy.model.shapes.SetShape;
2122
import software.amazon.smithy.model.shapes.Shape;
2223
import software.amazon.smithy.model.shapes.StructureShape;
2324
import software.amazon.smithy.model.shapes.UnionShape;
@@ -68,7 +69,10 @@ private static void structure(StructureShape structureShape,
6869
append(indentation, buffer, "{},");
6970
checkRequired(indentation, buffer, structureShape);
7071
} else {
71-
append(indentation, buffer, "{");
72+
append(indentation, buffer,
73+
"{" + (shapeTracker.getOccurrenceCount(structureShape) == 1
74+
? " // " + structureShape.getId().getName()
75+
: ""));
7276
checkRequired(indentation, buffer, structureShape);
7377
structureShape.getAllMembers().values().forEach(member -> {
7478
append(indentation + 2, buffer, member.getMemberName() + ": ");
@@ -83,7 +87,9 @@ private static void union(UnionShape unionShape,
8387
Model model,
8488
int indentation,
8589
ShapeTracker shapeTracker) {
86-
append(indentation, buffer, "{ // Union: only one key present");
90+
append(indentation, buffer, "{" + (shapeTracker.getOccurrenceCount(unionShape) == 1
91+
? " // " + unionShape.getId().getName()
92+
: "// ") + " Union: only one key present");
8793
checkRequired(indentation, buffer, unionShape);
8894
unionShape.getAllMembers().values().forEach(member -> {
8995
append(indentation + 2, buffer, member.getMemberName() + ": ");
@@ -104,9 +110,10 @@ private static void shape(Shape shape,
104110
target = shape;
105111
}
106112

107-
shapeTracker.mark(shape, indentation);
108-
if (shapeTracker.getOccurrenceDepths(shape) > 2) {
109-
append(indentation, buffer, "\"<" + shape.getId().getName() + ">\",\n");
113+
shapeTracker.mark(target, indentation);
114+
if (shapeTracker.shouldTruncate(target)) {
115+
append(indentation, buffer, "\"<" + target.getId().getName() + ">\",");
116+
checkRequired(indentation, buffer, shape);
110117
} else {
111118
switch (target.getType()) {
112119
case BIG_DECIMAL:
@@ -155,14 +162,18 @@ private static void shape(Shape shape,
155162

156163
case SET:
157164
case LIST:
158-
append(indentation, buffer, "[");
165+
append(indentation, buffer, "[" + (shapeTracker.getOccurrenceCount(target) == 1
166+
? " // " + target.getId().getName()
167+
: ""));
159168
checkRequired(indentation, buffer, shape);
160169
ListShape list = (ListShape) target;
161170
shape(list.getMember(), buffer, model, indentation + 2, shapeTracker);
162171
append(indentation, buffer, "],\n");
163172
break;
164173
case MAP:
165-
append(indentation, buffer, "{");
174+
append(indentation, buffer, "{" + (shapeTracker.getOccurrenceCount(target) == 1
175+
? " // " + target.getId().getName()
176+
: ""));
166177
checkRequired(indentation, buffer, shape);
167178
append(indentation + 2, buffer, "\"<keys>\": ");
168179
MapShape map = (MapShape) target;
@@ -261,23 +272,41 @@ private static void append(int indentation, StringBuilder buffer, String tail) {
261272
* This handles the case of recursive shapes.
262273
*/
263274
private static class ShapeTracker {
264-
private Map<Shape, Set<Integer>> data = new HashMap<Shape, Set<Integer>>();
275+
private Map<Shape, Set<Integer>> depths = new HashMap<Shape, Set<Integer>>();
276+
private Map<Shape, Integer> occurrences = new HashMap<Shape, Integer>();
265277

266278
/**
267279
* Mark that a shape is observed at depth.
268280
*/
269281
public void mark(Shape shape, int depth) {
270-
if (!data.containsKey(shape)) {
271-
data.put(shape, new HashSet<>());
282+
if (!depths.containsKey(shape)) {
283+
depths.put(shape, new HashSet<>());
272284
}
273-
data.get(shape).add(depth);
285+
depths.get(shape).add(depth);
286+
occurrences.put(shape, occurrences.getOrDefault(shape, 0) + 1);
287+
}
288+
289+
/**
290+
* @return whether the shape should be truncated.
291+
*/
292+
public boolean shouldTruncate(Shape shape) {
293+
return (shape instanceof MapShape || shape instanceof UnionShape || shape instanceof StructureShape
294+
|| shape instanceof ListShape || shape instanceof SetShape)
295+
&& (getOccurrenceCount(shape) > 5 || getOccurrenceDepths(shape) > 2);
274296
}
275297

276298
/**
277299
* @return the number of distinct depths in which the shape appears.
278300
*/
279301
public int getOccurrenceDepths(Shape shape) {
280-
return data.getOrDefault(shape, Collections.emptySet()).size();
302+
return depths.getOrDefault(shape, Collections.emptySet()).size();
303+
}
304+
305+
/**
306+
* @return total appearances of the shape.
307+
*/
308+
public int getOccurrenceCount(Shape shape) {
309+
return occurrences.getOrDefault(shape, 0);
281310
}
282311
}
283312
}

‎smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/documentation/StructureExampleGeneratorTest.java

Lines changed: 63 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,36 @@ public class StructureExampleGeneratorTest {
5353
StructureShape structure = StructureShape.builder()
5454
.id("foo.bar#structure")
5555
.members(
56-
List.<MemberShape>of(memberForString, memberForList, memberForMap))
56+
List.<MemberShape>of(
57+
memberForString, memberForList, memberForMap,
58+
MemberShape.builder()
59+
.id("foo.bar#structure$list2")
60+
.target(list.getId())
61+
.build(),
62+
MemberShape.builder()
63+
.id("foo.bar#structure$list3")
64+
.target(list.getId())
65+
.build(),
66+
MemberShape.builder()
67+
.id("foo.bar#structure$list4")
68+
.target(list.getId())
69+
.build(),
70+
MemberShape.builder()
71+
.id("foo.bar#structure$list5")
72+
.target(list.getId())
73+
.build(),
74+
MemberShape.builder()
75+
.id("foo.bar#structure$list6")
76+
.target(list.getId())
77+
.build(),
78+
MemberShape.builder()
79+
.id("foo.bar#structure$list7")
80+
.target(list.getId())
81+
.build(),
82+
MemberShape.builder()
83+
.id("foo.bar#structure$structure")
84+
.target("foo.bar#structure")
85+
.build()))
5786
.build();
5887

5988
private Model model = Model.builder()
@@ -67,7 +96,7 @@ public void generatesStructuralHintDocumentation_map() {
6796
assertThat(
6897
StructureExampleGenerator.generateStructuralHintDocumentation(map, model),
6998
equalTo("""
70-
{
99+
{ // map
71100
"<keys>": "STRING_VALUE",
72101
};"""));
73102
}
@@ -77,14 +106,42 @@ public void generatesStructuralHintDocumentation_structure() {
77106
assertThat(
78107
StructureExampleGenerator.generateStructuralHintDocumentation(structure, model),
79108
equalTo("""
80-
{
109+
{ // structure
81110
string: "STRING_VALUE",
82-
list: [
111+
list: [ // list
83112
"STRING_VALUE",
84113
],
85-
map: {
114+
map: { // map
86115
"<keys>": "STRING_VALUE",
87116
},
117+
list2: [
118+
"STRING_VALUE",
119+
],
120+
list3: [
121+
"STRING_VALUE",
122+
],
123+
list4: [
124+
"STRING_VALUE",
125+
],
126+
list5: [
127+
"STRING_VALUE",
128+
],
129+
list6: "<list>",
130+
list7: "<list>",
131+
structure: {
132+
string: "STRING_VALUE",
133+
list: "<list>",
134+
map: {
135+
"<keys>": "STRING_VALUE",
136+
},
137+
list2: "<list>",
138+
list3: "<list>",
139+
list4: "<list>",
140+
list5: "<list>",
141+
list6: "<list>",
142+
list7: "<list>",
143+
structure: "<structure>",
144+
},
88145
};"""));
89146
}
90147

@@ -93,7 +150,7 @@ public void generatesStructuralHintDocumentation_list() {
93150
assertThat(
94151
StructureExampleGenerator.generateStructuralHintDocumentation(list, model),
95152
equalTo("""
96-
[
153+
[ // list
97154
"STRING_VALUE",
98155
];"""));
99156
}

0 commit comments

Comments
 (0)