Skip to content

improve documentation truncation behavior #728

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 28, 2023
Merged

improve documentation truncation behavior #728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
de10abb
improve documentation truncation behavior
kuhe Mar 27, 2023
3c35b7e
add recursive test for structural doc gen
kuhe Mar 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 41 additions & 12 deletions ...va/software/amazon/smithy/typescript/codegen/documentation/StructureExampleGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import software.amazon.smithy.model.shapes.ListShape;
import software.amazon.smithy.model.shapes.MapShape;
import software.amazon.smithy.model.shapes.MemberShape;
import software.amazon.smithy.model.shapes.SetShape;
import software.amazon.smithy.model.shapes.Shape;
import software.amazon.smithy.model.shapes.StructureShape;
import software.amazon.smithy.model.shapes.UnionShape;
Expand Down Expand Up @@ -68,7 +69,10 @@ private static void structure(StructureShape structureShape,
append(indentation, buffer, "{},");
checkRequired(indentation, buffer, structureShape);
} else {
append(indentation, buffer, "{");
append(indentation, buffer,
"{" + (shapeTracker.getOccurrenceCount(structureShape) == 1
? " // " + structureShape.getId().getName()
: ""));
checkRequired(indentation, buffer, structureShape);
structureShape.getAllMembers().values().forEach(member -> {
append(indentation + 2, buffer, member.getMemberName() + ": ");
Expand All @@ -83,7 +87,9 @@ private static void union(UnionShape unionShape,
Model model,
int indentation,
ShapeTracker shapeTracker) {
append(indentation, buffer, "{ // Union: only one key present");
append(indentation, buffer, "{" + (shapeTracker.getOccurrenceCount(unionShape) == 1
? " // " + unionShape.getId().getName()
: "// ") + " Union: only one key present");
checkRequired(indentation, buffer, unionShape);
unionShape.getAllMembers().values().forEach(member -> {
append(indentation + 2, buffer, member.getMemberName() + ": ");
Expand All @@ -104,9 +110,10 @@ private static void shape(Shape shape,
target = shape;
}

shapeTracker.mark(shape, indentation);
if (shapeTracker.getOccurrenceDepths(shape) > 2) {
append(indentation, buffer, "\"<" + shape.getId().getName() + ">\",\n");
shapeTracker.mark(target, indentation);
if (shapeTracker.shouldTruncate(target)) {
append(indentation, buffer, "\"<" + target.getId().getName() + ">\",");
checkRequired(indentation, buffer, shape);
} else {
switch (target.getType()) {
case BIG_DECIMAL:
Expand Down Expand Up @@ -155,14 +162,18 @@ private static void shape(Shape shape,

case SET:
case LIST:
append(indentation, buffer, "[");
append(indentation, buffer, "[" + (shapeTracker.getOccurrenceCount(target) == 1
? " // " + target.getId().getName()
: ""));
checkRequired(indentation, buffer, shape);
ListShape list = (ListShape) target;
shape(list.getMember(), buffer, model, indentation + 2, shapeTracker);
append(indentation, buffer, "],\n");
break;
case MAP:
append(indentation, buffer, "{");
append(indentation, buffer, "{" + (shapeTracker.getOccurrenceCount(target) == 1
? " // " + target.getId().getName()
: ""));
checkRequired(indentation, buffer, shape);
append(indentation + 2, buffer, "\"<keys>\": ");
MapShape map = (MapShape) target;
Expand Down Expand Up @@ -261,23 +272,41 @@ private static void append(int indentation, StringBuilder buffer, String tail) {
* This handles the case of recursive shapes.
*/
private static class ShapeTracker {
private Map<Shape, Set<Integer>> data = new HashMap<Shape, Set<Integer>>();
private Map<Shape, Set<Integer>> depths = new HashMap<Shape, Set<Integer>>();
private Map<Shape, Integer> occurrences = new HashMap<Shape, Integer>();

/**
* Mark that a shape is observed at depth.
*/
public void mark(Shape shape, int depth) {
if (!data.containsKey(shape)) {
data.put(shape, new HashSet<>());
if (!depths.containsKey(shape)) {
depths.put(shape, new HashSet<>());
}
data.get(shape).add(depth);
depths.get(shape).add(depth);
occurrences.put(shape, occurrences.getOrDefault(shape, 0) + 1);
}

/**
* @return whether the shape should be truncated.
*/
public boolean shouldTruncate(Shape shape) {
return (shape instanceof MapShape || shape instanceof UnionShape || shape instanceof StructureShape
|| shape instanceof ListShape || shape instanceof SetShape)
&& (getOccurrenceCount(shape) > 5 || getOccurrenceDepths(shape) > 2);
}

/**
* @return the number of distinct depths in which the shape appears.
*/
public int getOccurrenceDepths(Shape shape) {
return data.getOrDefault(shape, Collections.emptySet()).size();
return depths.getOrDefault(shape, Collections.emptySet()).size();
}

/**
* @return total appearances of the shape.
*/
public int getOccurrenceCount(Shape shape) {
return occurrences.getOrDefault(shape, 0);
}
}
}
69 changes: 63 additions & 6 deletions ...oftware/amazon/smithy/typescript/codegen/documentation/StructureExampleGeneratorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,36 @@ public class StructureExampleGeneratorTest {
StructureShape structure = StructureShape.builder()
.id("foo.bar#structure")
.members(
List.<MemberShape>of(memberForString, memberForList, memberForMap))
List.<MemberShape>of(
memberForString, memberForList, memberForMap,
MemberShape.builder()
.id("foo.bar#structure$list2")
.target(list.getId())
.build(),
MemberShape.builder()
.id("foo.bar#structure$list3")
.target(list.getId())
.build(),
MemberShape.builder()
.id("foo.bar#structure$list4")
.target(list.getId())
.build(),
MemberShape.builder()
.id("foo.bar#structure$list5")
.target(list.getId())
.build(),
MemberShape.builder()
.id("foo.bar#structure$list6")
.target(list.getId())
.build(),
MemberShape.builder()
.id("foo.bar#structure$list7")
.target(list.getId())
.build(),
MemberShape.builder()
.id("foo.bar#structure$structure")
.target("foo.bar#structure")
.build()))
.build();

private Model model = Model.builder()
Expand All @@ -67,7 +96,7 @@ public void generatesStructuralHintDocumentation_map() {
assertThat(
StructureExampleGenerator.generateStructuralHintDocumentation(map, model),
equalTo("""
{
{ // map
"<keys>": "STRING_VALUE",
};"""));
}
Expand All @@ -77,14 +106,42 @@ public void generatesStructuralHintDocumentation_structure() {
assertThat(
StructureExampleGenerator.generateStructuralHintDocumentation(structure, model),
equalTo("""
{
{ // structure
string: "STRING_VALUE",
list: [
list: [ // list
"STRING_VALUE",
],
map: {
map: { // map
"<keys>": "STRING_VALUE",
},
list2: [
"STRING_VALUE",
],
list3: [
"STRING_VALUE",
],
list4: [
"STRING_VALUE",
],
list5: [
"STRING_VALUE",
],
list6: "<list>",
list7: "<list>",
structure: {
string: "STRING_VALUE",
list: "<list>",
map: {
"<keys>": "STRING_VALUE",
},
list2: "<list>",
list3: "<list>",
list4: "<list>",
list5: "<list>",
list6: "<list>",
list7: "<list>",
structure: "<structure>",
},
};"""));
}

Expand All @@ -93,7 +150,7 @@ public void generatesStructuralHintDocumentation_list() {
assertThat(
StructureExampleGenerator.generateStructuralHintDocumentation(list, model),
equalTo("""
[
[ // list
"STRING_VALUE",
];"""));
}
Expand Down