Skip to content
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

Add input and output traits with inlined IO #1007

Merged
merged 2 commits into from
Dec 9, 2021
Merged

Add input and output traits with inlined IO #1007

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4981a3a
Add input and output traits with inlined IO
JordonPhillips Dec 8, 2021
f70dc43
Add test case for duplicated traits
JordonPhillips Dec 9, 2021
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
22 changes: 17 additions & 5 deletions smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@
import software.amazon.smithy.model.shapes.TimestampShape;
import software.amazon.smithy.model.shapes.UnionShape;
import software.amazon.smithy.model.traits.DocumentationTrait;
import software.amazon.smithy.model.traits.InputTrait;
import software.amazon.smithy.model.traits.OutputTrait;
import software.amazon.smithy.model.traits.RequiredTrait;
import software.amazon.smithy.model.traits.TraitFactory;
import software.amazon.smithy.model.validation.Severity;
Expand Down Expand Up @@ -616,10 +618,12 @@ private void parseOperationStatement(ShapeId id, SourceLocation location) {
parseProperties(id, propertyName -> {
switch (propertyName) {
case "input":
parseInlineableOperationMember(id, operationInputSuffix, builder::input);
TraitEntry inputTrait = new TraitEntry(InputTrait.ID.toString(), Node.objectNode(), true);
parseInlineableOperationMember(id, operationInputSuffix, builder::input, inputTrait);
break;
case "output":
parseInlineableOperationMember(id, operationOutputSuffix, builder::output);
TraitEntry outputTrait = new TraitEntry(OutputTrait.ID.toString(), Node.objectNode(), true);
parseInlineableOperationMember(id, operationOutputSuffix, builder::output, outputTrait);
break;
case "errors":
parseIdList(builder::addError);
Expand Down Expand Up @@ -653,7 +657,12 @@ private void parseProperties(ShapeId id, Consumer<String> valueParser) {
expect('}');
}

private void parseInlineableOperationMember(ShapeId id, String suffix, Consumer<ShapeId> consumer) {
private void parseInlineableOperationMember(
ShapeId id,
String suffix,
Consumer<ShapeId> consumer,
TraitEntry defaultTrait
) {
if (peek() == '=') {
if (!modelFile.getVersion().supportsInlineOperationIO()) {
throw syntax(id, "Inlined operation inputs and outputs can only be used with Smithy version 2 or "
Expand All @@ -662,15 +671,18 @@ private void parseInlineableOperationMember(ShapeId id, String suffix, Consumer<
expect('=');
clearPendingDocs();
ws();
consumer.accept(parseInlineStructure(id.getName() + suffix));
consumer.accept(parseInlineStructure(id.getName() + suffix, defaultTrait));
} else {
ws();
modelFile.addForwardReference(ParserUtils.parseShapeId(this), consumer);
}
}

private ShapeId parseInlineStructure(String name) {
private ShapeId parseInlineStructure(String name, TraitEntry defaultTrait) {
List<TraitEntry> traits = parseDocsAndTraits();
if (defaultTrait != null) {
traits.add(defaultTrait);
kstich marked this conversation as resolved.
Show resolved Hide resolved
}
ShapeId id = ShapeId.fromRelative(modelFile.namespace(), name);
SourceLocation location = currentLocation();
parseMixins(id);
Expand Down
47 changes: 31 additions & 16 deletions smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand All @@ -42,9 +43,12 @@
import software.amazon.smithy.model.traits.AnnotationTrait;
import software.amazon.smithy.model.traits.DocumentationTrait;
import software.amazon.smithy.model.traits.IdRefTrait;
import software.amazon.smithy.model.traits.InputTrait;
import software.amazon.smithy.model.traits.OutputTrait;
import software.amazon.smithy.model.traits.RequiredTrait;
import software.amazon.smithy.model.traits.Trait;
import software.amazon.smithy.model.traits.TraitDefinition;
import software.amazon.smithy.model.traits.UnitTypeTrait;
import software.amazon.smithy.utils.CodeWriter;
import software.amazon.smithy.utils.FunctionalUtils;
import software.amazon.smithy.utils.ListUtils;
Expand Down Expand Up @@ -165,16 +169,20 @@ private Set<ShapeId> getInlineableShapes(Model fullModel, Collection<Shape> shap
continue;
}
OperationShape operation = shape.asOperationShape().get();
operation.getInput().ifPresent(shapeId -> {
if (shapes.contains(fullModel.expectShape(shapeId))) {
inlineableShapes.add(shapeId);
if (!operation.getInputShape().equals(UnitTypeTrait.UNIT)) {
Shape inputShape = fullModel.expectShape(operation.getInputShape());
if (shapes.contains(inputShape) && inputShape.hasTrait(InputTrait.ID)
&& operation.getInputShape().getName().equals(operation.getId().getName() + "Input")) {
JordonPhillips marked this conversation as resolved.
Show resolved Hide resolved
inlineableShapes.add(operation.getInputShape());
}
});
operation.getOutput().ifPresent(shapeId -> {
if (shapes.contains(fullModel.expectShape(shapeId))) {
inlineableShapes.add(shapeId);
}
if (!operation.getOutputShape().equals(UnitTypeTrait.UNIT)) {
Shape outputShape = fullModel.expectShape(operation.getOutputShape());
if (shapes.contains(outputShape) && outputShape.hasTrait(OutputTrait.ID)
&& operation.getOutputShape().getName().equals(operation.getId().getName() + "Output")) {
inlineableShapes.add(operation.getOutputShape());
}
});
}
}
return inlineableShapes;
}
Expand Down Expand Up @@ -603,30 +611,33 @@ public Void operationShape(OperationShape shape) {
serializeTraits(shape);
codeWriter.openBlock("operation $L {", shape.getId().getName());
List<MemberShape> mixinMembers = new ArrayList<>();
mixinMembers.addAll(writeInlineableProperty(
"input", shape.getInputShape(), shape.getId().getName() + "Input"));
mixinMembers.addAll(writeInlineableProperty(
"output", shape.getOutputShape(), shape.getId().getName() + "Output"));
mixinMembers.addAll(writeInlineableProperty("input", shape.getInputShape(), InputTrait.ID));
mixinMembers.addAll(writeInlineableProperty("output", shape.getOutputShape(), OutputTrait.ID));
codeWriter.writeOptionalIdList("errors", shape.getErrors());
codeWriter.closeBlock("}");
codeWriter.write("");
applyIntroducedTraits(mixinMembers);
return null;
}

private Collection<MemberShape> writeInlineableProperty(String key, ShapeId shapeId, String defaultName) {
private Collection<MemberShape> writeInlineableProperty(String key, ShapeId shapeId, ShapeId defaultTrait) {
if (!inlineableShapes.contains(shapeId)) {
codeWriter.write("$L: $I", key, shapeId);
return Collections.emptyList();
}

StructureShape structure = model.expectShape(shapeId, StructureShape.class);
if (structure.getAllTraits().isEmpty()) {
if (hasOnlyDefaultTrait(structure, defaultTrait)) {
codeWriter.writeInline("$L := ", key);
} else {
codeWriter.write("$L := ", key);
codeWriter.indent();
serializeTraits(structure);
Map<ShapeId, Trait> traits = structure.getAllTraits();
if (defaultTrait != null) {
traits = new HashMap<>(traits);
traits.remove(defaultTrait);
}
serializeTraits(traits);
}

List<MemberShape> nonMixinMembers = new ArrayList<>();
Expand All @@ -642,12 +653,16 @@ private Collection<MemberShape> writeInlineableProperty(String key, ShapeId shap
writeMixins(structure, !nonMixinMembers.isEmpty());
writeShapeMembers(nonMixinMembers, true);

if (!structure.getAllTraits().isEmpty()) {
if (!hasOnlyDefaultTrait(structure, defaultTrait)) {
codeWriter.dedent();
}

return mixinMembers;
}

private boolean hasOnlyDefaultTrait(Shape shape, ShapeId defaultTrait) {
return shape.getAllTraits().size() == 1 && shape.hasTrait(defaultTrait);
}
}

/**
Expand Down
8 changes: 6 additions & 2 deletions smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,16 @@
}
},
"smithy.example#MyOperationInput": {
"type": "structure"
"type": "structure",
"traits": {
"smithy.api#input": {}
}
},
"smithy.example#MyOperationOutput": {
"type": "structure",
"traits": {
"smithy.api#documentation": "These are not ignored because they come AFTER the walrus\noperator."
"smithy.api#documentation": "These are not ignored because they come AFTER the walrus\noperator.",
"smithy.api#output": {}
}
}
}
Expand Down
10 changes: 8 additions & 2 deletions ...src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/custom-suffix.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,16 @@
}
},
"com.example#OperationRequest": {
"type": "structure"
"type": "structure",
"traits": {
"smithy.api#input": {}
}
},
"com.example#OperationResponse": {
"type": "structure"
"type": "structure",
"traits": {
"smithy.api#output": {}
}
}
}
}
44 changes: 34 additions & 10 deletions ...del/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
"id": {
"target": "smithy.api#String"
}
},
"traits": {
"smithy.api#input": {}
}
},
"com.example#DerivedNamesOutput": {
Expand All @@ -24,6 +27,9 @@
"id": {
"target": "smithy.api#String"
}
},
"traits": {
"smithy.api#output": {}
}
},
"com.example#UsesTraits": {
Expand All @@ -43,7 +49,8 @@
}
},
"traits": {
"smithy.api#sensitive": {}
"smithy.api#sensitive": {},
"smithy.api#input": {}
}
},
"com.example#UsesTraitsOutput": {
Expand All @@ -54,7 +61,8 @@
}
},
"traits": {
"smithy.api#sensitive": {}
"smithy.api#sensitive": {},
"smithy.api#output": {}
}
},
"com.example#NameBearer": {
Expand Down Expand Up @@ -88,7 +96,10 @@
{
"target": "com.example#NameBearer"
}
]
],
"traits": {
"smithy.api#input": {}
}
},
"com.example#UsesMixinsOutput": {
"type": "structure",
Expand All @@ -101,7 +112,10 @@
{
"target": "com.example#NameBearer"
}
]
],
"traits": {
"smithy.api#output": {}
}
},
"com.example#UsesTraitsAndMixins": {
"type": "operation",
Expand All @@ -120,7 +134,8 @@
}
},
"traits": {
"smithy.api#sensitive": {}
"smithy.api#sensitive": {},
"smithy.api#input": {}
},
"mixins": [
{
Expand All @@ -136,7 +151,8 @@
}
},
"traits": {
"smithy.api#sensitive": {}
"smithy.api#sensitive": {},
"smithy.api#output": {}
},
"mixins": [
{
Expand All @@ -154,10 +170,16 @@
}
},
"com.example#EmptyShapesInput": {
"type": "structure"
"type": "structure",
"traits": {
"smithy.api#input": {}
}
},
"com.example#EmptyShapesOutput": {
"type": "structure"
"type": "structure",
"traits": {
"smithy.api#output": {}
}
},
"com.example#HasDocComments": {
"type": "operation",
Expand All @@ -171,13 +193,15 @@
"com.example#HasDocCommentsInput": {
"type": "structure",
"traits": {
"smithy.api#documentation": "The trait parser automagically handles these"
"smithy.api#documentation": "The trait parser automagically handles these",
"smithy.api#input": {}
}
},
"com.example#HasDocCommentsOutput": {
"type": "structure",
"traits": {
"smithy.api#documentation": "Here too"
"smithy.api#documentation": "Here too",
"smithy.api#output": {}
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions ...st/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/inline-io.smithy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@ operation DerivedNames {
}
}

operation DoesNotInlineWithoutIOTrait {
input: DoesNotInlineWithoutIOTraitInput
output: DoesNotInlineWithoutIOTraitOutput
}

operation EmptyShapes {
input := {}
output := {}
Expand Down Expand Up @@ -60,6 +65,10 @@ operation UsesTraitsAndMixins {
}
}

structure DoesNotInlineWithoutIOTraitInput {}

structure DoesNotInlineWithoutIOTraitOutput {}

@mixin
structure NameBearer {
name: String
Expand Down