Repository: smithy-lang/smithy
Pull request #2316 (2024-06-11 16:58:24)

Add IDL serializer option to coerce inline IO
This commit adds a new IDL serializer configuration, coerceInlineIo, that will use inline IO to write input and output shapes that are only bound to one operation - even if they don't have the input or output trait applied.

It also fixes a bug where an empty newline was written before empty inline IO shape definitions.

---
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.



Patch:
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 8d0aec396..53000a44c 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
@@ -34,6 +34,7 @@ import java.util.function.Predicate;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.knowledge.OperationIndex;
 import software.amazon.smithy.model.loader.Prelude;
 import software.amazon.smithy.model.node.ArrayNode;
 import software.amazon.smithy.model.node.BooleanNode;
@@ -74,6 +75,7 @@ public final class SmithyIdlModelSerializer {
     private final String inlineInputSuffix;
     private final String inlineOutputSuffix;
     private final boolean inferInlineIoSuffixes;
+    private final boolean shouldCoerceInlineIo;
 
     /**
      * Trait serialization features.
@@ -114,15 +116,21 @@ public final class SmithyIdlModelSerializer {
         traitFilter = builder.traitFilter.and(FunctionalUtils.not(Trait::isSynthetic));
         basePath = builder.basePath;
         if (basePath != null) {
-            Function<Shape, Path> shapePlacer = builder.shapePlacer;
-            this.shapePlacer = shape -> this.basePath.resolve(shapePlacer.apply(shape));
+            Function<Shape, Path> placer = builder.shapePlacer;
+            shapePlacer = shape -> basePath.resolve(placer.apply(shape));
         } else {
-            this.shapePlacer = builder.shapePlacer;
+            shapePlacer = builder.shapePlacer;
+        }
+        componentOrder = builder.componentOrder;
+        inlineInputSuffix = builder.inlineInputSuffix;
+        inlineOutputSuffix = builder.inlineOutputSuffix;
+        shouldCoerceInlineIo = builder.shouldCoerceInlineIo;
+        // Force inferring suffixes on if coercion is on.
+        if (shouldCoerceInlineIo) {
+            inferInlineIoSuffixes = true;
+        } else {
+            inferInlineIoSuffixes = builder.inferInlineIoSuffixes;
         }
-        this.componentOrder = builder.componentOrder;
-        this.inlineInputSuffix = builder.inlineInputSuffix;
-        this.inlineOutputSuffix = builder.inlineOutputSuffix;
-        this.inferInlineIoSuffixes = builder.inferInlineIoSuffixes;
     }
 
     /**
@@ -193,6 +201,7 @@ public final class SmithyIdlModelSerializer {
             String inputSuffix,
             String outputSuffix
     ) {
+        OperationIndex operationIndex = OperationIndex.of(fullModel);
         Set<ShapeId> inlineableShapes = new HashSet<>();
         for (Shape shape : shapes) {
             if (!shape.isOperationShape()) {
@@ -201,14 +210,14 @@ public final class SmithyIdlModelSerializer {
             OperationShape operation = shape.asOperationShape().get();
             if (!operation.getInputShape().equals(UnitTypeTrait.UNIT)) {
                 Shape inputShape = fullModel.expectShape(operation.getInputShape());
-                if (shapes.contains(inputShape) && inputShape.hasTrait(InputTrait.ID)
+                if (shapes.contains(inputShape) && shouldInlineInputShape(operationIndex, inputShape)
                         && inputShape.getId().getName().equals(operation.getId().getName() + inputSuffix)) {
                     inlineableShapes.add(operation.getInputShape());
                 }
             }
             if (!operation.getOutputShape().equals(UnitTypeTrait.UNIT)) {
                 Shape outputShape = fullModel.expectShape(operation.getOutputShape());
-                if (shapes.contains(outputShape) && outputShape.hasTrait(OutputTrait.ID)
+                if (shapes.contains(outputShape) && shouldInlineOutputShape(operationIndex, outputShape)
                         && outputShape.getId().getName().equals(operation.getId().getName() + outputSuffix)) {
                     inlineableShapes.add(operation.getOutputShape());
                 }
@@ -217,6 +226,18 @@ public final class SmithyIdlModelSerializer {
         return inlineableShapes;
     }
 
+    private boolean shouldInlineInputShape(OperationIndex operationIndex, Shape target) {
+        // Only allow coercion if the shape is only used as one input.
+        return target.hasTrait(InputTrait.ID)
+                || (shouldCoerceInlineIo && operationIndex.getInputBindings(target).size() == 1);
+    }
+
+    private boolean shouldInlineOutputShape(OperationIndex operationIndex, Shape target) {
+        // Only allow coercion if the shape is only used as one output.
+        return target.hasTrait(OutputTrait.ID)
+                || (shouldCoerceInlineIo && operationIndex.getOutputBindings(target).size() == 1);
+    }
+
     private Pair<String, String> determineInlineSuffixes(Model fullModel, Collection<Shape> shapes) {
         if (!inferInlineIoSuffixes) {
             return Pair.of(inlineInputSuffix, inlineOutputSuffix);
@@ -330,6 +351,7 @@ public final class SmithyIdlModelSerializer {
         private String inlineInputSuffix = DEFAULT_INLINE_INPUT_SUFFIX;
         private String inlineOutputSuffix = DEFAULT_INLINE_OUTPUT_SUFFIX;
         private boolean inferInlineIoSuffixes = false;
+        private boolean shouldCoerceInlineIo = false;
 
         public Builder() {}
 
@@ -457,11 +479,30 @@ public final class SmithyIdlModelSerializer {
          * <p>The suffixes set by {@link #inlineInputSuffix(String)} and {@link #inlineOutputSuffix(String)}
          * will be the default values.
          *
-         * @param shouldinferInlineIoSuffixes Whether inline IO suffixes should be inferred for each file.
+         * @param shouldInferInlineIoSuffixes Whether inline IO suffixes should be inferred for each file.
+         * @return Returns the builder.
+         */
+        public Builder inferInlineIoSuffixes(boolean shouldInferInlineIoSuffixes) {
+            this.inferInlineIoSuffixes = shouldInferInlineIoSuffixes;
+            return this;
+        }
+
+        /**
+         * Determines whether inline IO should be coerced for shapes operation input
+         * and output that does not have the {@code @input} or {@code @output} trait,
+         * respectively.
+         *
+         * <p>If true, this will determine any shared IO suffixes for each file. Only the
+         * shapes present within each file will impact what that file's suffixes will be.
+         *
+         * <p>The suffixes set by {@link #inlineInputSuffix(String)} and {@link #inlineOutputSuffix(String)}
+         * will be the default values.
+         *
+         * @param shouldCoerceInlineIo Whether inline IO should be coerced for each file.
          * @return Returns the builder.
          */
-        public Builder inferInlineIoSuffixes(boolean shouldinferInlineIoSuffixes) {
-            this.inferInlineIoSuffixes = shouldinferInlineIoSuffixes;
+        public Builder coerceInlineIo(boolean shouldCoerceInlineIo) {
+            this.shouldCoerceInlineIo = shouldCoerceInlineIo;
             return this;
         }
 
@@ -833,7 +874,7 @@ public final class SmithyIdlModelSerializer {
         }
 
         private boolean hasOnlyDefaultTrait(Shape shape, ShapeId defaultTrait) {
-            return shape.getAllTraits().size() == 1 && shape.hasTrait(defaultTrait);
+            return shape.getAllTraits().isEmpty() || (shape.getAllTraits().size() == 1 && shape.hasTrait(defaultTrait));
         }
     }
 


Test patch:
diff --git a/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java b/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java
index 15431ecd5..27dea4cc8 100644
--- a/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java
+++ b/smithy-model/src/test/java/software/amazon/smithy/model/shapes/SmithyIdlModelSerializerTest.java
@@ -8,6 +8,7 @@ import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasKey;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.io.IOException;
 import java.net.URISyntaxException;
@@ -49,7 +50,7 @@ public class SmithyIdlModelSerializerTest {
         }
 
         String serializedString = serialized.entrySet().iterator().next().getValue();
-        Assertions.assertEquals(IoUtils.readUtf8File(path).replaceAll("\\R", "\n"), serializedString);
+        assertEquals(IoUtils.readUtf8File(path).replaceAll("\\R", "\n"), serializedString);
     }
 
     @Test
@@ -259,7 +260,7 @@ public class SmithyIdlModelSerializerTest {
         String expectedOutput = IoUtils.readUtf8Resource(getClass(), "idl-serialization/enum-mixin-output.smithy")
                 .replaceAll("\\R", "\n");
         String serializedString = serialized.entrySet().iterator().next().getValue();
-        Assertions.assertEquals(expectedOutput, serializedString);
+        assertEquals(expectedOutput, serializedString);
     }
 
     @Test
@@ -306,4 +307,21 @@ public class SmithyIdlModelSerializerTest {
             assertThat(actual, equalTo(expected));
         }
     }
+
+    @Test
+    public void coercesInlineIO() {
+        Model before = Model.assembler()
+                .addImport(getClass().getResource("idl-serialization/coerced-io/before.smithy"))
+                .assemble().unwrap();
+
+        Map<Path, String> reserialized = SmithyIdlModelSerializer.builder()
+                .coerceInlineIo(true)
+                .build()
+                .serialize(before);
+        String modelResult = reserialized.values().iterator().next().replace("\r\n", "\n");
+
+        String expected = IoUtils.readUtf8Url(getClass().getResource("idl-serialization/coerced-io/after.smithy"))
+                .replace("\r\n", "\n");
+        assertEquals(expected, modelResult);
+    }
 }
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/coerced-io/after.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/coerced-io/after.smithy
new file mode 100644
index 000000000..7f76e438f
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/coerced-io/after.smithy
@@ -0,0 +1,40 @@
+$version: "2.0"
+$operationInputSuffix: "Request"
+$operationOutputSuffix: "Response"
+
+namespace com.example
+
+service InlineService {
+    operations: [
+        MultipleBind1
+        MultipleBind2
+        WithoutTraits
+        WithTraits
+    ]
+}
+
+operation MultipleBind1 {
+    input: MultipleBindRequest
+    output: MultipleBindResponse
+}
+
+operation MultipleBind2 {
+    input: MultipleBindRequest
+    output: MultipleBindResponse
+}
+
+operation WithoutTraits {
+    input := {}
+    output := {}
+}
+
+operation WithTraits {
+    input := {
+        a: String
+    }
+    output := {}
+}
+
+structure MultipleBindRequest {}
+
+structure MultipleBindResponse {}
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/coerced-io/before.smithy b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/coerced-io/before.smithy
new file mode 100644
index 000000000..c4d9a2a59
--- /dev/null
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/shapes/idl-serialization/coerced-io/before.smithy
@@ -0,0 +1,48 @@
+$version: "2.0"
+
+namespace com.example
+
+service InlineService {
+    operations: [
+        MultipleBind1
+        MultipleBind2
+        WithoutTraits
+        WithTraits
+    ]
+}
+
+operation MultipleBind1 {
+    input: MultipleBindRequest
+    output: MultipleBindResponse
+}
+
+operation MultipleBind2 {
+    input: MultipleBindRequest
+    output: MultipleBindResponse
+}
+
+operation WithoutTraits {
+    input: WithoutTraitsRequest
+    output: WithoutTraitsResponse
+}
+
+operation WithTraits {
+    input: WithTraitsRequest
+    output: WithTraitsResponse
+}
+
+structure MultipleBindRequest {}
+
+structure MultipleBindResponse {}
+
+structure WithoutTraitsRequest {}
+
+structure WithoutTraitsResponse {}
+
+@input
+structure WithTraitsRequest {
+    a: String
+}
+
+@output
+structure WithTraitsResponse {}