From 0e5542b74acf11bccf71edaecb6733d680956737 Mon Sep 17 00:00:00 2001 From: akarshy21 <58502269+akarshy21@users.noreply.github.com> Date: Thu, 5 Nov 2026 05:36:20 +0000 Subject: [PATCH 1/2] Give every shape its own synthetic operation in python-type-codegen CreateSyntheticService incremented the operation index only for shapes that get a synthetic input wrapper, so an error (or a kept @input/@output shape) shared its operation ID with the next shape. One of the two operations then replaced the other, and shapes only it reached were silently left out of models.py. Increment the index once per operation and test that every error and structure is generated. --- .../codegen/types/PythonTypeCodegenTest.java | 92 +++++++++++++++++++ .../codegen/types/CreateSyntheticService.java | 3 +- 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java b/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java index ddd37d6ea..07706daf7 100644 --- a/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java +++ b/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java @@ -4,7 +4,13 @@ */ package software.amazon.smithy.python.codegen.types; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; +import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; import software.amazon.smithy.build.FileManifest; @@ -56,4 +62,90 @@ public void testCodegenWithoutService(@TempDir Path tempDir) { .build(); plugin.execute(context); } + + @Test + public void testEveryShapeIsGeneratedAlongsideErrors(@TempDir Path tempDir) throws IOException { + // Error shapes get their own synthetic operation; the shapes next to them must too. + List errors = List.of("ErrorA", "ErrorB", "ErrorC", "ErrorD", "ErrorE"); + List structures = List.of( + "ShapeA", + "ShapeB", + "ShapeC", + "ShapeD", + "ShapeE", + "ShapeF", + "ShapeG", + "ShapeH"); + StringBuilder source = new StringBuilder("$version: \"2.0\"\nnamespace example.errors\n"); + for (String error : errors) { + source.append("@error(\"client\")\nstructure ").append(error).append(" { message: String }\n"); + } + for (String structure : structures) { + source.append("structure ").append(structure).append(" { value: String }\n"); + } + Model model = Model.assembler() + .addUnparsedModel("errors.smithy", source.toString()) + .assemble() + .unwrap(); + PluginContext context = PluginContext.builder() + .fileManifest(FileManifest.create(tempDir)) + .settings( + ObjectNode.builder() + .withMember("module", "errors_test") + .withMember("moduleVersion", "0.0.1") + .build()) + .model(model) + .build(); + + new PythonTypeCodegenPlugin().execute(context); + + String models = Files.readString(tempDir.resolve("src/errors_test/models.py")); + List missing = new ArrayList<>(); + errors.stream().filter(name -> !models.contains("class " + name + "(")).forEach(missing::add); + structures.stream().filter(name -> !models.contains("class " + name + ":")).forEach(missing::add); + assertEquals(List.of(), missing, "shapes missing from models.py"); + } + + @Test + public void testErrorsAreGeneratedAlongsideInputsAndOutputs(@TempDir Path tempDir) throws IOException { + // With generateInputsAndOutputs, @input and @output shapes also get synthetic operations, + // and must not take the IDs of the errors and structures around them. + List errors = List.of("ErrorA", "ErrorB", "ErrorC", "ErrorD", "ErrorE"); + List structures = List.of("ShapeA", "ShapeB", "ShapeC", "ShapeD"); + List operations = List.of("OpA", "OpB", "OpC", "OpD", "OpE", "OpF"); + StringBuilder source = new StringBuilder("$version: \"2.0\"\nnamespace example.io\n"); + for (String error : errors) { + source.append("@error(\"client\")\nstructure ").append(error).append(" { message: String }\n"); + } + for (String structure : structures) { + source.append("structure ").append(structure).append(" { value: String }\n"); + } + for (String operation : operations) { + source.append("operation ") + .append(operation) + .append(" { input := { value: String }, output := { value: String } }\n"); + } + Model model = Model.assembler() + .addUnparsedModel("io.smithy", source.toString()) + .assemble() + .unwrap(); + PluginContext context = PluginContext.builder() + .fileManifest(FileManifest.create(tempDir)) + .settings( + ObjectNode.builder() + .withMember("module", "io_test") + .withMember("moduleVersion", "0.0.1") + .withMember("generateInputsAndOutputs", true) + .build()) + .model(model) + .build(); + + new PythonTypeCodegenPlugin().execute(context); + + String models = Files.readString(tempDir.resolve("src/io_test/models.py")); + List missing = new ArrayList<>(); + errors.stream().filter(name -> !models.contains("class " + name + "(")).forEach(missing::add); + structures.stream().filter(name -> !models.contains("class " + name + ":")).forEach(missing::add); + assertEquals(List.of(), missing, "shapes missing from models.py"); + } } diff --git a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/CreateSyntheticService.java b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/CreateSyntheticService.java index 32fe19805..a6da394b3 100644 --- a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/CreateSyntheticService.java +++ b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/CreateSyntheticService.java @@ -72,13 +72,14 @@ Model transform(ModelTransformer transformer, Model model) { .addTrait(new InputTrait()) .addMember("member", shape.getId()) .build(); - index++; operationBuilder.input(input); addedShapes.add(input); } serviceBuilder.addOperation(operationId); addedShapes.add(operationBuilder.build()); + // Every shape needs its own operation; a reused ID replaces the earlier one. + index++; } addedShapes.add(serviceBuilder.build()); From 1484d666d455aedd327337f93a0ba7dc545c7eb3 Mon Sep 17 00:00:00 2001 From: akarshy21 <58502269+akarshy21@users.noreply.github.com> Date: Wed, 11 Nov 2026 23:16:25 +0000 Subject: [PATCH 2/2] Honor generateInputsAndOutputs in python-type-codegen CreateSyntheticService included @input and @output structures when generateInputsAndOutputs was set, but DirectedPythonTypeCodegen always skipped them in types mode, so the setting had no effect. Pass the setting to the generator and generate those structures when it is on. Also list selector and generateInputsAndOutputs as known settings so using them no longer logs an additional-properties warning. --- .../codegen/types/PythonTypeCodegenTest.java | 141 +++++++++++------- .../types/DirectedPythonTypeCodegen.java | 14 +- .../types/PythonTypeCodegenPlugin.java | 2 +- .../types/PythonTypeCodegenSettings.java | 8 +- 4 files changed, 104 insertions(+), 61 deletions(-) diff --git a/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java b/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java index 07706daf7..b45c0655a 100644 --- a/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java +++ b/codegen/plugins/types/src/it/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenTest.java @@ -66,86 +66,115 @@ public void testCodegenWithoutService(@TempDir Path tempDir) { @Test public void testEveryShapeIsGeneratedAlongsideErrors(@TempDir Path tempDir) throws IOException { // Error shapes get their own synthetic operation; the shapes next to them must too. - List errors = List.of("ErrorA", "ErrorB", "ErrorC", "ErrorD", "ErrorE"); - List structures = List.of( - "ShapeA", - "ShapeB", - "ShapeC", - "ShapeD", - "ShapeE", - "ShapeF", - "ShapeG", - "ShapeH"); - StringBuilder source = new StringBuilder("$version: \"2.0\"\nnamespace example.errors\n"); - for (String error : errors) { - source.append("@error(\"client\")\nstructure ").append(error).append(" { message: String }\n"); - } - for (String structure : structures) { - source.append("structure ").append(structure).append(" { value: String }\n"); - } - Model model = Model.assembler() - .addUnparsedModel("errors.smithy", source.toString()) - .assemble() - .unwrap(); - PluginContext context = PluginContext.builder() - .fileManifest(FileManifest.create(tempDir)) - .settings( - ObjectNode.builder() - .withMember("module", "errors_test") - .withMember("moduleVersion", "0.0.1") - .build()) - .model(model) - .build(); + String models = generate(tempDir, errors() + structures(8), settings()); - new PythonTypeCodegenPlugin().execute(context); + assertEquals(List.of(), missing(models, errorNames(), structureNames(8)), "shapes missing from models.py"); + } - String models = Files.readString(tempDir.resolve("src/errors_test/models.py")); - List missing = new ArrayList<>(); - errors.stream().filter(name -> !models.contains("class " + name + "(")).forEach(missing::add); - structures.stream().filter(name -> !models.contains("class " + name + ":")).forEach(missing::add); - assertEquals(List.of(), missing, "shapes missing from models.py"); + @Test + public void testInputsAndOutputsAreGeneratedWhenEnabled(@TempDir Path tempDir) throws IOException { + // @input and @output shapes get synthetic operations too, and are generated when asked for. + String models = generate( + tempDir, + errors() + structures(4) + operations(), + settings().withMember("generateInputsAndOutputs", true)); + + List expected = new ArrayList<>(structureNames(4)); + expected.addAll(ioNames()); + assertEquals(List.of(), missing(models, errorNames(), expected), "shapes missing from models.py"); + } + + @Test + public void testInputsAndOutputsAreGeneratedForServiceWhenEnabled(@TempDir Path tempDir) throws IOException { + String service = + "service IoService { operations: [OpA, OpB, OpC, OpD, OpE, OpF], errors: [ErrorA, ErrorB, ErrorC, ErrorD, ErrorE] }\n"; + String models = generate( + tempDir, + errors() + operations() + service, + settings().withMember("service", "example.types#IoService") + .withMember("generateInputsAndOutputs", true)); + + assertEquals(List.of(), missing(models, errorNames(), ioNames()), "shapes missing from models.py"); } @Test - public void testErrorsAreGeneratedAlongsideInputsAndOutputs(@TempDir Path tempDir) throws IOException { - // With generateInputsAndOutputs, @input and @output shapes also get synthetic operations, - // and must not take the IDs of the errors and structures around them. - List errors = List.of("ErrorA", "ErrorB", "ErrorC", "ErrorD", "ErrorE"); - List structures = List.of("ShapeA", "ShapeB", "ShapeC", "ShapeD"); - List operations = List.of("OpA", "OpB", "OpC", "OpD", "OpE", "OpF"); - StringBuilder source = new StringBuilder("$version: \"2.0\"\nnamespace example.io\n"); - for (String error : errors) { + public void testInputsAndOutputsAreSkippedByDefault(@TempDir Path tempDir) throws IOException { + String models = generate(tempDir, errors() + structures(4) + operations(), settings()); + + assertEquals(List.of(), missing(models, errorNames(), structureNames(4)), "shapes missing from models.py"); + List generatedIo = ioNames().stream().filter(name -> models.contains("class " + name + ":")).toList(); + assertEquals(List.of(), generatedIo, "input/output shapes generated without generateInputsAndOutputs"); + } + + private static final List OPERATIONS = List.of("OpA", "OpB", "OpC", "OpD", "OpE", "OpF"); + + private static List errorNames() { + return List.of("ErrorA", "ErrorB", "ErrorC", "ErrorD", "ErrorE"); + } + + private static List structureNames(int count) { + return List.of("ShapeA", "ShapeB", "ShapeC", "ShapeD", "ShapeE", "ShapeF", "ShapeG", "ShapeH") + .subList(0, count); + } + + private static List ioNames() { + List names = new ArrayList<>(); + for (String operation : OPERATIONS) { + names.add(operation + "Input"); + names.add(operation + "Output"); + } + return names; + } + + private static String errors() { + StringBuilder source = new StringBuilder(); + for (String error : errorNames()) { source.append("@error(\"client\")\nstructure ").append(error).append(" { message: String }\n"); } - for (String structure : structures) { + return source.toString(); + } + + private static String structures(int count) { + StringBuilder source = new StringBuilder(); + for (String structure : structureNames(count)) { source.append("structure ").append(structure).append(" { value: String }\n"); } - for (String operation : operations) { + return source.toString(); + } + + private static String operations() { + StringBuilder source = new StringBuilder(); + for (String operation : OPERATIONS) { source.append("operation ") .append(operation) .append(" { input := { value: String }, output := { value: String } }\n"); } + return source.toString(); + } + + private static ObjectNode.Builder settings() { + return ObjectNode.builder().withMember("module", "types_test").withMember("moduleVersion", "0.0.1"); + } + + private static String generate(Path tempDir, String shapes, ObjectNode.Builder settings) throws IOException { Model model = Model.assembler() - .addUnparsedModel("io.smithy", source.toString()) + .addUnparsedModel("test.smithy", "$version: \"2.0\"\nnamespace example.types\n" + shapes) .assemble() .unwrap(); PluginContext context = PluginContext.builder() .fileManifest(FileManifest.create(tempDir)) - .settings( - ObjectNode.builder() - .withMember("module", "io_test") - .withMember("moduleVersion", "0.0.1") - .withMember("generateInputsAndOutputs", true) - .build()) + .settings(settings.build()) .model(model) .build(); - new PythonTypeCodegenPlugin().execute(context); + return Files.readString(tempDir.resolve("src/types_test/models.py")); + } - String models = Files.readString(tempDir.resolve("src/io_test/models.py")); + // Errors are declared as `class Name(...)`, other structures as `class Name:`. + private static List missing(String models, List errors, List structures) { List missing = new ArrayList<>(); errors.stream().filter(name -> !models.contains("class " + name + "(")).forEach(missing::add); structures.stream().filter(name -> !models.contains("class " + name + ":")).forEach(missing::add); - assertEquals(List.of(), missing, "shapes missing from models.py"); + return missing; } } diff --git a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/DirectedPythonTypeCodegen.java b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/DirectedPythonTypeCodegen.java index 41fde17dc..8026a0c81 100644 --- a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/DirectedPythonTypeCodegen.java +++ b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/DirectedPythonTypeCodegen.java @@ -40,6 +40,13 @@ final class DirectedPythonTypeCodegen implements DirectedCodegen { + + private final boolean generateInputsAndOutputs; + + DirectedPythonTypeCodegen(boolean generateInputsAndOutputs) { + this.generateInputsAndOutputs = generateInputsAndOutputs; + } + @Override public SymbolProvider createSymbolProvider(CreateSymbolProviderDirective directive) { return new PythonSymbolProvider(directive.model(), directive.settings()); @@ -79,7 +86,7 @@ public void generateService(GenerateServiceDirective directive) { - // If we're only generating data shapes, there's no need to generate input or output shapes. + // Types-only codegen skips input and output shapes unless generateInputsAndOutputs is set. if (!shouldGenerateStructure(directive.settings(), directive.shape())) { return; } @@ -98,8 +105,9 @@ private boolean shouldGenerateStructure(PythonSettings settings, Shape shape) { if (shape.getId().getNamespace().equals("smithy.synthetic")) { return false; } - return !(settings.artifactType().equals(PythonSettings.ArtifactType.TYPES) - && (shape.hasTrait(InputTrait.class) || shape.hasTrait(OutputTrait.class))); + return generateInputsAndOutputs + || !(settings.artifactType().equals(PythonSettings.ArtifactType.TYPES) + && (shape.hasTrait(InputTrait.class) || shape.hasTrait(OutputTrait.class))); } @Override diff --git a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenPlugin.java b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenPlugin.java index 763696e46..e2b5e8c40 100644 --- a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenPlugin.java +++ b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenPlugin.java @@ -39,7 +39,7 @@ public void execute(PluginContext context) { } runner.settings(pythonSettings); - runner.directedCodegen(new DirectedPythonTypeCodegen()); + runner.directedCodegen(new DirectedPythonTypeCodegen(typeSettings.generateInputsAndOutputs())); runner.fileManifest(context.getFileManifest()); runner.service(pythonSettings.service()); runner.model(model); diff --git a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenSettings.java b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenSettings.java index ff70f76a6..1f2883165 100644 --- a/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenSettings.java +++ b/codegen/plugins/types/src/main/java/software/amazon/smithy/python/codegen/types/PythonTypeCodegenSettings.java @@ -84,7 +84,13 @@ public PythonSettings toPythonSettings() { * @return Returns the extracted settings. */ public static PythonTypeCodegenSettings fromNode(ObjectNode config) { - config.warnIfAdditionalProperties(Arrays.asList(SERVICE, MODULE_NAME, MODULE_DESCRIPTION, MODULE_VERSION)); + config.warnIfAdditionalProperties(Arrays.asList( + SERVICE, + MODULE_NAME, + MODULE_DESCRIPTION, + MODULE_VERSION, + SELECTOR, + GENERATE_INPUTS_AND_OUTPUTS)); String moduleName = config.expectStringMember(MODULE_NAME).getValue(); Builder builder = builder()