Add fixes for symbols and errors

1. Reserved words are now escaped in the symbol visitor to account for
   escaping reserved words in symbol references (like a list of Record
   becomes "Array<_Record>".
2. Error structures now normalize the "message" member regardless of the
   model. This prevents typing issues with super.
3. "message" is no longer generated as a property of SmithyException
   subtypes.
4. Fixed a bug in how relative module imports were handled and
   simplified turning module names into file names. Now the module name
   of shapes that are generated always contains "index" to fix
   relativizing imports.

Author: mtdowling

smithy-typescript-codegen-test/model/main.smithy

@@ -69,7 +69,9 @@ structure CityCoordinates {
 structure NoSuchResource {
     /// The type of resource that was not found.
     @required
-    resourceType: String
+    resourceType: String,
+
+    message: String,
 }
 
 // The paginated trait indicates that the operation may

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java

@@ -82,7 +82,6 @@ void execute() {
         nonTraits.shapes().sorted().forEach(shape -> shape.accept(this));
 
         // Write each pending writer.
-        // writers.forEach((filename, writer) -> fileManifest.writeFile(filename, writer.toString()));
         writers.writeFiles();
 
         // Write the package.json file, including all symbol dependencies.
@@ -250,24 +249,28 @@ private void renderErrorStructure(StructureShape shape) {
         writer.openBlock("export class $L extends $$SmithyException {", symbol.getName());
 
         // Write properties.
+        // Skip "message" since it is something that SmithyException defines.
         StructuredMemberWriter config = new StructuredMemberWriter(
                 model, symbolProvider, shape.getAllMembers().values());
         config.memberPrefix = "readonly ";
+        config.skipMembers.add("message");
         config.writeMembers(writer, shape);
 
         // Write constructor.
         writer.openBlock("constructor(args: {");
         writer.write("$$service: string;");
-        if (!shape.getMemberNames().contains("message")) {
-            writer.write("message?: string;");
-        }
+        writer.write("message?: string;");
+
         config.memberPrefix = "";
         config.noDocs = true;
         config.writeMembers(writer, shape);
         writer.closeBlock("}) {");
         writer.indent();
 
         writer.openBlock("super({");
+        // Provide a default value for message in case it was optional in the shape.
+        // It's required in SmithyException, so provide a default value.
+        writer.write("message: args.message || \"\",");
         writer.write("id: $S,", shape.getId());
         writer.write("name: $S,", shape.getId().getName());
         writer.write("fault: $S,", errorTrait.getValue());
@@ -276,7 +279,9 @@ private void renderErrorStructure(StructureShape shape) {
 
         for (MemberShape member : shape.getAllMembers().values()) {
             String memberName = symbolProvider.toMemberName(member);
-            writer.write("this.$1L = args.$1L;", memberName);
+            if (!memberName.equals("message")) {
+                writer.write("this.$1L = args.$1L;", memberName);
+            }
         }
 
         writer.closeBlock("}"); // constructor

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/StructuredMemberWriter.java

@@ -16,13 +16,17 @@
 package software.amazon.smithy.typescript.codegen;
 
 import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
 import software.amazon.smithy.codegen.core.SymbolProvider;
 import software.amazon.smithy.model.Model;
 import software.amazon.smithy.model.shapes.MemberShape;
 import software.amazon.smithy.model.shapes.Shape;
 
 /**
  * Generates objects, interfaces, enums, etc.
+ *
+ * TODO: Replace this with a builder for generating classes and interfaces.
  */
 final class StructuredMemberWriter {
 
@@ -31,6 +35,7 @@ final class StructuredMemberWriter {
     Collection<MemberShape> members;
     String memberPrefix = "";
     boolean noDocs;
+    final Set<String> skipMembers = new HashSet<>();
 
     StructuredMemberWriter(Model model, SymbolProvider symbolProvider, Collection<MemberShape> members) {
         this.model = model;
@@ -41,13 +46,18 @@ final class StructuredMemberWriter {
     void writeMembers(TypeScriptWriter writer, Shape shape) {
         int position = -1;
         for (MemberShape member : members) {
+            if (skipMembers.contains(member.getMemberName())) {
+                continue;
+            }
+
             position++;
             boolean wroteDocs = !noDocs && writer.writeMemberDocs(model, member);
             String memberName = TypeScriptUtils.sanitizePropertyName(symbolProvider.toMemberName(member));
             String optionalSuffix = shape.isUnionShape() || !member.isRequired() ? "?" : "";
             String typeSuffix = member.isRequired() ? " | undefined" : "";
             writer.write("${L}${L}${L}: ${T}${L};", memberPrefix, memberName, optionalSuffix,
                          symbolProvider.toSymbol(member), typeSuffix);
+
             if (wroteDocs && position < members.size() - 1) {
                 writer.write("");
             }

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/SymbolVisitor.java

@@ -18,10 +18,12 @@
 import static java.lang.String.format;
 
 import java.util.Locale;
-import java.util.StringJoiner;
 import java.util.logging.Logger;
 import java.util.regex.Pattern;
 import software.amazon.smithy.codegen.core.CodegenException;
+import software.amazon.smithy.codegen.core.ReservedWordSymbolProvider;
+import software.amazon.smithy.codegen.core.ReservedWords;
+import software.amazon.smithy.codegen.core.ReservedWordsBuilder;
 import software.amazon.smithy.codegen.core.ShapeIdShader;
 import software.amazon.smithy.codegen.core.Symbol;
 import software.amazon.smithy.codegen.core.SymbolProvider;
@@ -72,11 +74,11 @@ final class SymbolVisitor implements SymbolProvider, ShapeVisitor<Symbol> {
     private static final String BIG_JS_VERSION = "^5.2.2";
     private static final Pattern SHAPE_ID_NAMESPACE_SPLITTER = Pattern.compile("\\.");
     private static final Pattern SHAPE_ID_NAMESPACE_PART_SPLITTER = Pattern.compile("_");
-    private static final Pattern SLASH_SPLITTER = Pattern.compile("/");
 
     private final Model model;
     private final ShapeIdShader shader;
     private final String targetNamespace;
+    private final ReservedWordSymbolProvider.Escaper escaper;
 
     SymbolVisitor(Model model, String rootNamespace, String targetNamespace) {
         this.model = model;
@@ -90,13 +92,30 @@ final class SymbolVisitor implements SymbolProvider, ShapeVisitor<Symbol> {
         } else {
             shader = null;
         }
+
+        // Load reserved words from a new-line delimited file.
+        ReservedWords reservedWords = new ReservedWordsBuilder()
+                .loadWords(TypeScriptCodegenPlugin.class.getResource("reserved-words.txt"))
+                .build();
+
+        escaper = ReservedWordSymbolProvider.builder()
+                .nameReservedWords(reservedWords)
+                // Only escape words when the symbol has a definition file to
+                // prevent escaping intentional references to built-in types.
+                .escapePredicate((shape, symbol) -> !StringUtils.isEmpty(symbol.getDefinitionFile()))
+                .buildEscaper();
     }
 
     @Override
     public Symbol toSymbol(Shape shape) {
         Symbol symbol = shape.accept(this);
         LOGGER.fine(() -> "Creating symbol from " + shape + ": " + symbol);
-        return symbol;
+        return escaper.escapeSymbol(shape, symbol);
+    }
+
+    @Override
+    public String toMemberName(MemberShape shape) {
+        return escaper.escapeMemberName(shape.getMemberName());
     }
 
     @Override
@@ -210,7 +229,7 @@ public Symbol documentShape(DocumentShape shape) {
     public Symbol operationShape(OperationShape shape) {
         ShapeId shaded = shadeShapeId(shape);
         String commandName = StringUtils.capitalize(shaded.getName()) + "Command";
-        return createSymbolBuilder(shape, commandName, formatModuleName(shaded)).build();
+        return createGeneratedSymbolBuilder(shape, commandName, formatModuleName(shaded)).build();
     }
 
     @Override
@@ -295,7 +314,7 @@ private ShapeId shadeShapeId(ToShapeId id) {
     private Symbol.Builder createObjectSymbolBuilder(Shape shape) {
         ShapeId shaded = shadeShapeId(shape);
         String name = StringUtils.capitalize(shaded.getName());
-        return createSymbolBuilder(shape, name, formatModuleName(shaded));
+        return createGeneratedSymbolBuilder(shape, name, formatModuleName(shaded));
     }
 
     private Symbol.Builder createSymbolBuilder(Shape shape, String typeName) {
@@ -306,7 +325,11 @@ private Symbol.Builder createSymbolBuilder(Shape shape, String typeName, String
         return Symbol.builder()
                 .putProperty("shape", shape)
                 .name(typeName)
-                .namespace(namespace, "/")
+                .namespace(namespace, "/");
+    }
+
+    private Symbol.Builder createGeneratedSymbolBuilder(Shape shape, String typeName, String namespace) {
+        return createSymbolBuilder(shape, typeName, namespace)
                 .definitionFile(toFilename(shape, typeName, namespace));
     }
 
@@ -325,27 +348,14 @@ private String formatModuleName(ToShapeId id) {
             result.append("/");
         }
 
-        // Remove the trailing "/".
-        result.deleteCharAt(result.length() - 1);
-        return result.toString();
+        return result.append("index").toString();
     }
 
     private String toFilename(Shape shape, String name, String namespace) {
         if (shape.isServiceShape()) {
             return name + "Client.ts";
         }
 
-        String filename;
-        if (namespace == null) {
-            filename = "index.ts";
-        } else {
-            StringJoiner joiner = new StringJoiner("/", "", "/index.ts");
-            for (String part : SLASH_SPLITTER.split(namespace)) {
-                joiner.add(part);
-            }
-            filename = joiner.toString();
-        }
-
-        return format("types/%s", filename).replace("//", "/");
+        return "types/" + namespace.replace(".", "/") + ".ts";
     }
 }

smithy-typescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/TypeScriptCodegenPlugin.java

@@ -17,12 +17,8 @@
 
 import software.amazon.smithy.build.PluginContext;
 import software.amazon.smithy.build.SmithyBuildPlugin;
-import software.amazon.smithy.codegen.core.ReservedWordSymbolProvider;
-import software.amazon.smithy.codegen.core.ReservedWords;
-import software.amazon.smithy.codegen.core.ReservedWordsBuilder;
 import software.amazon.smithy.codegen.core.SymbolProvider;
 import software.amazon.smithy.model.Model;
-import software.amazon.smithy.utils.StringUtils;
 
 /**
  * Plugin to trigger TypeScript code generation.
@@ -58,19 +54,6 @@ public static SymbolProvider createSymbolProvider(Model model) {
      * @return Returns the created provider.
      */
     public static SymbolProvider createSymbolProvider(Model model, String rootNamespace, String targetNamespace) {
-        SymbolVisitor symbolProvider = new SymbolVisitor(model, rootNamespace, targetNamespace);
-
-        // Load reserved words from a new-line delimited file.
-        ReservedWords reservedWords = new ReservedWordsBuilder()
-                .loadWords(TypeScriptCodegenPlugin.class.getResource("reserved-words.txt"))
-                .build();
-
-        return ReservedWordSymbolProvider.builder()
-                .nameReservedWords(reservedWords)
-                .symbolProvider(symbolProvider)
-                // Only escape words when the symbol has a namespace. This
-                // prevents escaping intentional references to reserved words.
-                .escapePredicate((shape, symbol) -> !StringUtils.isEmpty(symbol.getNamespace()))
-                .build();
+        return new SymbolVisitor(model, rootNamespace, targetNamespace);
     }
 }

smithy-typescript-codegen/src/main/resources/software/amazon/smithy/typescript/codegen/shapeTypes.ts

@@ -30,9 +30,9 @@ export class SmithyException extends Error implements SmithyStructure {
     name: string;
     service: string;
     fault: "client" | "server";
-    message?: string;
+    message?: string | undefined;
   }) {
-    super(args.message);
+    super(args.message || "");
     this.$id = args.id;
     this.name = args.name;
     this.$service = args.service;

smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/CodegenVisitorTest.java

@@ -0,0 +1,62 @@
+package software.amazon.smithy.typescript.codegen;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+
+import org.junit.jupiter.api.Test;
+import software.amazon.smithy.build.MockManifest;
+import software.amazon.smithy.build.PluginContext;
+import software.amazon.smithy.model.Model;
+import software.amazon.smithy.model.node.Node;
+
+public class CodegenVisitorTest {
+    @Test
+    public void properlyGeneratesEmptyMessageMemberOfException() {
+        testErrorStructureCodegen("error-test-empty.smithy");
+    }
+
+    @Test
+    public void properlyGeneratesOptionalMessageMemberOfException() {
+        testErrorStructureCodegen("error-test-optional-message.smithy");
+    }
+
+    @Test
+    public void properlyGeneratesRequiredMessageMemberOfException() {
+        testErrorStructureCodegen("error-test-required-message.smithy");
+    }
+
+    public void testErrorStructureCodegen(String file) {
+        Model model = Model.assembler()
+                .addImport(getClass().getResource(file))
+                .assemble()
+                .unwrap();
+        MockManifest manifest = new MockManifest();
+        PluginContext context = PluginContext.builder()
+                .model(model)
+                .fileManifest(manifest)
+                .settings(Node.objectNodeBuilder()
+                                  .withMember("service", Node.from("smithy.example#Example"))
+                                  .withMember("package", Node.from("example"))
+                                  .withMember("packageVersion", Node.from("1.0.0"))
+                                  .build())
+                .build();
+
+        new TypeScriptCodegenPlugin().execute(context);
+        String contents = manifest.getFileString("/types/smithy/example/index.ts").get();
+
+        assertThat(contents, containsString("export class Err extends $SmithyException {\n"
+                                            + "  constructor(args: {\n"
+                                            + "    $service: string;\n"
+                                            + "    message?: string;\n"
+                                            + "  }) {\n"
+                                            + "    super({\n"
+                                            + "      message: args.message || \"\",\n"
+                                            + "      id: \"smithy.example#Err\",\n"
+                                            + "      name: \"Err\",\n"
+                                            + "      fault: \"client\",\n"
+                                            + "      service: args.$service,\n"
+                                            + "    });\n"
+                                            + "  }\n"
+                                            + "}"));
+    }
+}

smithy-typescript-codegen/src/test/java/software/amazon/smithy/typescript/codegen/ImportDeclarationsTest.java

@@ -43,6 +43,15 @@ public void relativizesImports() {
         assertThat(result, containsString("import { Baz } from \"./bam\";"));
     }
 
+    @Test
+    public void relativizesImportsWithTrailingFilename() {
+        ImportDeclarations declarations = new ImportDeclarations("foo/bar/index");
+        declarations.addImport("Baz", "", "./shared/shapeTypes");
+        String result = declarations.toString();
+
+        assertThat(result, containsString("import { Baz } from \"../../../shared/shapeTypes\";"));
+    }
+
     @Test
     public void automaticallyCorrectsBasePath() {
         ImportDeclarations declarations = new ImportDeclarations("/foo/bar");