diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java index 6079e98c107..544eda1ff9c 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/loader/IdlModelParser.java @@ -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; @@ -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); @@ -653,7 +657,12 @@ private void parseProperties(ShapeId id, Consumer valueParser) { expect('}'); } - private void parseInlineableOperationMember(ShapeId id, String suffix, Consumer consumer) { + private void parseInlineableOperationMember( + ShapeId id, + String suffix, + Consumer 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 " @@ -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 traits = parseDocsAndTraits(); + if (defaultTrait != null) { + traits.add(defaultTrait); + } ShapeId id = ShapeId.fromRelative(modelFile.namespace(), name); SourceLocation location = currentLocation(); parseMixins(id); diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java index b985e0efbda..dbd5201d03b 100644 --- a/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java +++ b/smithy-model/src/main/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializer.java @@ -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; @@ -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; @@ -165,16 +169,20 @@ private Set getInlineableShapes(Model fullModel, Collection 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")) { + 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; } @@ -603,10 +611,8 @@ public Void operationShape(OperationShape shape) { serializeTraits(shape); codeWriter.openBlock("operation $L {", shape.getId().getName()); List 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(""); @@ -614,19 +620,24 @@ public Void operationShape(OperationShape shape) { return null; } - private Collection writeInlineableProperty(String key, ShapeId shapeId, String defaultName) { + private Collection 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 traits = structure.getAllTraits(); + if (defaultTrait != null) { + traits = new HashMap<>(traits); + traits.remove(defaultTrait); + } + serializeTraits(traits); } List nonMixinMembers = new ArrayList<>(); @@ -642,12 +653,16 @@ private Collection 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); + } } /** diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments.json index aa95b56c725..5e21ea10d7c 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/doc-comments.json @@ -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": {} } } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/custom-suffix.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/custom-suffix.json index c34aa06d7e1..80de469f6cf 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/custom-suffix.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/custom-suffix.json @@ -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": {} + } } } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.json b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.json index 199025aef5d..76354f99a1a 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.json +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.json @@ -16,6 +16,9 @@ "id": { "target": "smithy.api#String" } + }, + "traits": { + "smithy.api#input": {} } }, "com.example#DerivedNamesOutput": { @@ -24,6 +27,9 @@ "id": { "target": "smithy.api#String" } + }, + "traits": { + "smithy.api#output": {} } }, "com.example#UsesTraits": { @@ -43,7 +49,8 @@ } }, "traits": { - "smithy.api#sensitive": {} + "smithy.api#sensitive": {}, + "smithy.api#input": {} } }, "com.example#UsesTraitsOutput": { @@ -54,7 +61,8 @@ } }, "traits": { - "smithy.api#sensitive": {} + "smithy.api#sensitive": {}, + "smithy.api#output": {} } }, "com.example#NameBearer": { @@ -88,7 +96,10 @@ { "target": "com.example#NameBearer" } - ] + ], + "traits": { + "smithy.api#input": {} + } }, "com.example#UsesMixinsOutput": { "type": "structure", @@ -101,7 +112,10 @@ { "target": "com.example#NameBearer" } - ] + ], + "traits": { + "smithy.api#output": {} + } }, "com.example#UsesTraitsAndMixins": { "type": "operation", @@ -120,7 +134,8 @@ } }, "traits": { - "smithy.api#sensitive": {} + "smithy.api#sensitive": {}, + "smithy.api#input": {} }, "mixins": [ { @@ -136,7 +151,8 @@ } }, "traits": { - "smithy.api#sensitive": {} + "smithy.api#sensitive": {}, + "smithy.api#output": {} }, "mixins": [ { @@ -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", @@ -171,13 +193,38 @@ "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": {} + } + }, + "com.example#DuplicateTrait": { + "type": "operation", + "input": { + "target": "com.example#DuplicateTraitInput" + }, + "output": { + "target": "com.example#DuplicateTraitOutput" + } + }, + "com.example#DuplicateTraitInput": { + "type": "structure", + "members": {}, + "traits": { + "smithy.api#input": {} + } + }, + "com.example#DuplicateTraitOutput": { + "type": "structure", + "members": {}, + "traits": { + "smithy.api#output": {} } } } diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.smithy index 6824166c4d8..2438c748680 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.smithy +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/loader/valid/inline-io/inline-io.smithy @@ -67,3 +67,8 @@ operation HasDocComments { /// Here too {} } + +operation DuplicateTrait { + input := @input {} + output := @output {} +} diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/inline-io.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/inline-io.smithy index 90cfbf7541d..f6d5f0da608 100644 --- a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/inline-io.smithy +++ b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/cases/inline-io.smithy @@ -11,6 +11,11 @@ operation DerivedNames { } } +operation DoesNotInlineWithoutIOTrait { + input: DoesNotInlineWithoutIOTraitInput + output: DoesNotInlineWithoutIOTraitOutput +} + operation EmptyShapes { input := {} output := {} @@ -60,6 +65,10 @@ operation UsesTraitsAndMixins { } } +structure DoesNotInlineWithoutIOTraitInput {} + +structure DoesNotInlineWithoutIOTraitOutput {} + @mixin structure NameBearer { name: String