Updates based on PR feedback

SamRemis · SamRemis · commit cf9e9ec2382a · 2025-03-19T18:08:23.000-04:00
diff --git a/codegen/aws/core/build.gradle.kts b/codegen/aws/core/build.gradle.kts
@@ -12,5 +12,4 @@ extra["moduleName"] = "software.amazon.smithy.python.aws.codegen"
 dependencies {
     implementation(project(":core"))
     implementation(libs.smithy.aws.traits)
-    implementation("org.jsoup:jsoup:1.19.1")
 }
diff --git a/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsRstDocFileGenerator.java b/codegen/aws/core/src/main/java/software/amazon/smithy/python/aws/codegen/AwsRstDocFileGenerator.java
@@ -23,6 +23,10 @@ public List<? extends CodeInterceptor<? extends CodeSection, PythonWriter>> inte
             GenerationContext context
     ) {
         return List.of(
+                // We generate custom RST files for each member that we want to have
+                //  its own page.  This gives us much more fine-grained control of
+                //  what gets generated than just using automodule or autoclass on
+                //  the client would alone.
                 new OperationGenerationInterceptor(context),
                 new StructureGenerationInterceptor(context),
                 new ErrorGenerationInterceptor(context),
diff --git a/codegen/aws/core/src/main/resources/META-INF/services/software.amazon.smithy.python.codegen.integrations.PythonIntegration b/codegen/aws/core/src/main/resources/META-INF/services/software.amazon.smithy.python.codegen.integrations.PythonIntegration
@@ -8,5 +8,4 @@ software.amazon.smithy.python.aws.codegen.AwsProtocolsIntegration
 software.amazon.smithy.python.aws.codegen.AwsServiceIdIntegration
 software.amazon.smithy.python.aws.codegen.AwsUserAgentIntegration
 software.amazon.smithy.python.aws.codegen.AwsStandardRegionalEndpointsIntegration
-software.amazon.smithy.python.aws.codegen.AwsDocConverter
 software.amazon.smithy.python.aws.codegen.AwsRstDocFileGenerator
diff --git a/codegen/aws/core/src/test/java/software/amazon/smithy/python/aws/codegen/MarkdownToRstDocConverterTest.java b/codegen/aws/core/src/test/java/software/amazon/smithy/python/aws/codegen/MarkdownToRstDocConverterTest.java
@@ -8,101 +8,102 @@
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import software.amazon.smithy.python.codegen.writer.MarkdownToRstDocConverter;
 
-public class AwsDocConverterTest {
+public class MarkdownToRstDocConverterTest {
 
-    private AwsDocConverter awsDocConverter;
+    private MarkdownToRstDocConverter markdownToRstDocConverter;
 
     @BeforeEach
     public void setUp() {
-        awsDocConverter = new AwsDocConverter();
+        markdownToRstDocConverter = new MarkdownToRstDocConverter();
     }
 
     @Test
-    public void testConvertHtmlToRstWithTitleAndParagraph() {
+    public void testConvertCommonmarkToRstWithTitleAndParagraph() {
         String html = "<html><body><h1>Title</h1><p>Paragraph</p></body></html>";
         String expected = "\n\nTitle\n=====\nParagraph\n";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithImportantNote() {
+    public void testConvertCommonmarkToRstWithImportantNote() {
         String html = "<html><body><important>Important note</important></body></html>";
         String expected = "\n\n.. important::\n    Important note\n";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithList() {
+    public void testConvertCommonmarkToRstWithList() {
         String html = "<html><body><ul><li>Item 1</li><li>Item 2</li></ul></body></html>";
         String expected = "\n\n* Item 1\n\n* Item 2\n\n";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithMixedElements() {
+    public void testConvertCommonmarkToRstWithMixedElements() {
         String html = "<html><body><h1>Title</h1><p>Paragraph</p><ul><li>Item 1</li><li>Item 2</li></ul></body></html>";
         String expected = "\n\nTitle\n=====\nParagraph\n\n* Item 1\n\n* Item 2\n\n";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithNestedElements() {
+    public void testConvertCommonmarkToRstWithNestedElements() {
         String html = "<html><body><h1>Title</h1><p>Paragraph with <strong>bold</strong> text</p></body></html>";
         String expected = "\n\nTitle\n=====\nParagraph with **bold** text\n";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithAnchorTag() {
+    public void testConvertCommonmarkToRstWithAnchorTag() {
         String html = "<html><body><a href='https://example.com'>Link</a></body></html>";
         String expected = "\n`Link <https://example.com>`_";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithBoldTag() {
+    public void testConvertCommonmarkToRstWithBoldTag() {
         String html = "<html><body><b>Bold text</b></body></html>";
         String expected = "\n**Bold text**";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithItalicTag() {
+    public void testConvertCommonmarkToRstWithItalicTag() {
         String html = "<html><body><i>Italic text</i></body></html>";
         String expected = "\n*Italic text*";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithCodeTag() {
+    public void testConvertCommonmarkToRstWithCodeTag() {
         String html = "<html><body><code>code snippet</code></body></html>";
         String expected = "\n``code snippet``";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithNoteTag() {
+    public void testConvertCommonmarkToRstWithNoteTag() {
         String html = "<html><body><note>Note text</note></body></html>";
         String expected = "\n\n.. note::\n    Note text\n";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 
     @Test
-    public void testConvertHtmlToRstWithNestedList() {
+    public void testConvertCommonmarkToRstWithNestedList() {
         String html = "<html><body><ul><li>Item 1<ul><li>Subitem 1</li></ul></li><li>Item 2</li></ul></body></html>";
         String expected = "\n\n* Item 1\n  * Subitem 1\n\n* Item 2\n\n";
-        String result = awsDocConverter.convertHtmlToRst(html);
+        String result = markdownToRstDocConverter.convertCommonmarkToRst(html);
         assertEquals(expected, result);
     }
 }
diff --git a/codegen/core/build.gradle.kts b/codegen/core/build.gradle.kts
@@ -15,4 +15,6 @@ dependencies {
     implementation(libs.smithy.protocol.test.traits)
     // We have this because we're using RestJson1 as a 'generic' protocol.
     implementation(libs.smithy.aws.traits)
+    implementation(libs.jsoup)
+    implementation(libs.commonmark)
 }
diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/SetupGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/SetupGenerator.java
@@ -14,6 +14,8 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+
+import software.amazon.smithy.aws.traits.ServiceTrait;
 import software.amazon.smithy.codegen.core.CodegenException;
 import software.amazon.smithy.codegen.core.SymbolDependency;
 import software.amazon.smithy.codegen.core.WriterDelegator;
@@ -270,6 +272,8 @@ private static void writeDocsSkeleton(
             PythonSettings settings,
             GenerationContext context
     ) {
+        //TODO Add a configurable flag to disable the generation of the sphinx files
+        //TODO Add a configuration that will allow users to select a sphinx theme
         context.writerDelegator().useFileWriter("pyproject.toml", "", writer -> {
             writer.addDependency(SmithyPythonDependency.SPHINX);
             writer.addDependency(SmithyPythonDependency.SPHINX_PYDATA_THEME);
@@ -291,14 +295,18 @@ private static void writeConf(
             PythonSettings settings,
             GenerationContext context
     ) {
+        var service = context.model().expectShape(settings.service());
         String version = settings.moduleVersion();
+        String project = service.getTrait(TitleTrait.class)
+                .map(StringTrait::getValue)
+                .orElse(service.getTrait(ServiceTrait.class).get().getSdkId());
         context.writerDelegator().useFileWriter("docs/conf.py", "", writer -> {
             writer.write("""
                     import os
                     import sys
                     sys.path.insert(0, os.path.abspath('..'))
 
-                    project = 'AWS SDK for Python'
+                    project = '$L'
                     author = 'Boto'
                     release = '$L'
 
@@ -322,7 +330,7 @@ private static void writeConf(
                     }
 
                     autodoc_typehints = 'both'
-                                """, version);
+                                """, project, version);
         });
     }
 
diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/StructureGenerator.java
@@ -317,8 +317,7 @@ private boolean hasDocs() {
     private void writeMemberDocs(MemberShape member) {
         member.getMemberTrait(model, DocumentationTrait.class).ifPresent(trait -> {
             String memberName = symbolProvider.toMemberName(member);
-            String docs = writer.formatDocs(String.format(":param %s: %s", memberName, trait.getValue()))
-                    .replace("\n", "\n    ");
+            String docs = writer.formatDocs(String.format(":param %s: %s", memberName, trait.getValue()));
             writer.write(docs);
         });
     }
diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/UnionGenerator.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/generators/UnionGenerator.java
@@ -133,6 +133,8 @@ raise NotImplementedError()
         writer.popState();
 
         writer.pushState(new UnionSection(shape, parentName, memberNames));
+        // We need to use the old union syntax until we either migrate away from
+        // Sphinx or Sphinx fixes the issue upstream: https://github.com/sphinx-doc/sphinx/issues/10785
         writer.write("$L = Union[$L]\n", parentName, String.join(" | ", memberNames));
         shape.getTrait(DocumentationTrait.class).ifPresent(trait -> writer.writeDocs(trait.getValue()));
         writer.popState();
diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/MarkdownToRstDocConverter.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/MarkdownToRstDocConverter.java
@@ -2,53 +2,62 @@
  * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
  * SPDX-License-Identifier: Apache-2.0
  */
-package software.amazon.smithy.python.aws.codegen;
+package software.amazon.smithy.python.codegen.writer;
 
 import org.jsoup.Jsoup;
 import org.jsoup.nodes.Document;
 import org.jsoup.nodes.Element;
 import org.jsoup.nodes.Node;
 import org.jsoup.nodes.TextNode;
 import org.jsoup.select.NodeVisitor;
-import software.amazon.smithy.model.Model;
-import software.amazon.smithy.model.shapes.Shape;
-import software.amazon.smithy.model.traits.DocumentationTrait;
-import software.amazon.smithy.model.transform.ModelTransformer;
-import software.amazon.smithy.python.codegen.PythonSettings;
-import software.amazon.smithy.python.codegen.integrations.PythonIntegration;
+import software.amazon.smithy.utils.SetUtils;
 import software.amazon.smithy.utils.SmithyInternalApi;
+import org.commonmark.node.BlockQuote;
+import org.commonmark.node.FencedCodeBlock;
+import org.commonmark.node.Heading;
+import org.commonmark.node.HtmlBlock;
+import org.commonmark.node.ListBlock;
+import org.commonmark.node.ThematicBreak;
+import org.commonmark.parser.Parser;
+import org.commonmark.renderer.html.HtmlRenderer;
+
+import static org.jsoup.nodes.Document.OutputSettings.Syntax.html;
 
 /**
- * Add a runtime plugin to convert the HTML docs that are provided by services into
- * RST
+ * Add a runtime plugin to convert the HTML docs that are provided by services into RST
  */
 @SmithyInternalApi
-public class AwsDocConverter implements PythonIntegration {
-    @Override
-    public Model preprocessModel(Model model, PythonSettings settings) {
-        return ModelTransformer.create().mapShapes(model, shape -> {
-            if (shape.hasTrait(DocumentationTrait.class)) {
-                DocumentationTrait docTrait = shape.getTrait(DocumentationTrait.class).get();
-                String html = docTrait.getValue();
-                String rst = convertHtmlToRst(html);
-                DocumentationTrait newDocTrait = new DocumentationTrait(rst);
-                return Shape.shapeToBuilder(shape)
-                        .addTrait(newDocTrait)
-                        .build();
-            } else {
-                return shape;
-            }
-        });
+public class MarkdownToRstDocConverter {
+    private static final Parser MARKDOWN_PARSER = Parser.builder()
+            .enabledBlockTypes(SetUtils.of(
+                    Heading.class, HtmlBlock.class, ThematicBreak.class, FencedCodeBlock.class,
+                    BlockQuote.class, ListBlock.class))
+            .build();
+
+    // Singleton instance
+    private static final MarkdownToRstDocConverter DOC_CONVERTER = new MarkdownToRstDocConverter();
+
+    // Private constructor to prevent instantiation
+    private MarkdownToRstDocConverter() {
+        // Constructor
     }
 
-    String convertHtmlToRst(String html) {
-        Document document = Jsoup.parse(html);
+    public static MarkdownToRstDocConverter getInstance() {
+        return DOC_CONVERTER;
+    }
+
+
+    public String convertCommonmarkToRst(String commonmark) {
+        String html =
+                HtmlRenderer.builder().escapeHtml(false).build().render(MARKDOWN_PARSER.parse(commonmark));
+        Document document = Jsoup.parse(commonmark);
         RstNodeVisitor visitor = new RstNodeVisitor();
         document.body().traverse(visitor);
         return "\n" + visitor;
     }
 
     private static class RstNodeVisitor implements NodeVisitor {
+        //TODO migrate away from StringBuilder to use a SimpleCodeWriter
         private final StringBuilder sb = new StringBuilder();
         private boolean inList = false;
         private int listDepth = 0;
@@ -127,7 +136,8 @@ public void tail(Node node, int depth) {
                         sb.append("``");
                         break;
                     case "important":
-                    case "note", "p":
+                    case "note":
+                    case "p":
                         sb.append("\n");
                         break;
                     case "ul":
@@ -159,4 +169,4 @@ public String toString() {
             return sb.toString();
         }
     }
-}
+}
diff --git a/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/PythonWriter.java b/codegen/core/src/main/java/software/amazon/smithy/python/codegen/writer/PythonWriter.java
@@ -39,6 +39,7 @@
 public final class PythonWriter extends SymbolWriter<PythonWriter, ImportDeclarations> {
 
     private static final Logger LOGGER = Logger.getLogger(PythonWriter.class.getName());
+    private static final MarkdownToRstDocConverter DOC_CONVERTER = MarkdownToRstDocConverter.getInstance();
 
     private final String fullPackageName;
     private final String commentStart;
@@ -152,8 +153,8 @@ public PythonWriter writeDocs(String docs) {
      * @return Formatted documentation.
      */
     public String formatDocs(String docs) {
-        String rstDocs = wrapRST(docs).toString();
-        return rstDocs.replace("$", "$$");
+        String rstDocs = DOC_CONVERTER.convertCommonmarkToRst(docs);
+        return wrapRST(rstDocs).toString().replace("$", "$$");
     }
 
     public static String wrapRST(String text) {
diff --git a/codegen/gradle/libs.versions.toml b/codegen/gradle/libs.versions.toml
@@ -6,11 +6,14 @@ spotbugs = "6.0.22"
 spotless = "7.0.2"
 smithy-gradle-plugins = "1.2.0"
 dep-analysis = "2.11.0"
+jsoup = "1.19.1"
+commonmark = "0.15.2"
 
 [libraries]
 smithy-model = { module = "software.amazon.smithy:smithy-model", version.ref = "smithy" }
 smithy-codegen = { module = "software.amazon.smithy:smithy-codegen-core", version.ref = "smithy" }
 smithy-aws-traits = { module = "software.amazon.smithy:smithy-aws-traits", version.ref = "smithy" }
+smithy-aws-endpoints = { module = "software.amazon.smithy:smithy-aws-endpoints", version.ref = "smithy" }
 smithy-aws-protocol-tests = { module = "software.amazon.smithy:smithy-aws-protocol-tests", version.ref = "smithy" }
 smithy-protocol-test-traits = { module = "software.amazon.smithy:smithy-protocol-test-traits", version.ref = "smithy"}
 smithy-waiters = { module = "software.amazon.smithy:smithy-waiters", version.ref = "smithy" }
@@ -28,6 +31,9 @@ spotbugs = { module = "com.github.spotbugs.snom:spotbugs-gradle-plugin", version
 spotless = { module = "com.diffplug.spotless:spotless-plugin-gradle", version.ref = "spotless" }
 dependency-analysis = { module = "com.autonomousapps:dependency-analysis-gradle-plugin", version.ref = "dep-analysis" }
 
+jsoup = { module = "org.jsoup:jsoup", version.ref = "jsoup" }
+commonmark = { module = "com.atlassian.commonmark:commonmark", version.ref ="commonmark" }
+
 [plugins]
 smithy-gradle-base = { id = "software.amazon.smithy.gradle.smithy-base", version.ref = "smithy-gradle-plugins" }
 smithy-gradle-jar = { id = "software.amazon.smithy.gradle.smithy-jar", version.ref = "smithy-gradle-plugins" }

Original file line number	Diff line number	Diff line change
`@@ -12,5 +12,4 @@ extra["moduleName"] = "software.amazon.smithy.python.aws.codegen"`
`12`	`12`	`dependencies {`
`13`	`13`	`implementation(project(":core"))`
`14`	`14`	`implementation(libs.smithy.aws.traits)`
`15`		`- implementation("org.jsoup:jsoup:1.19.1")`
`16`	`15`	`}`
Original file line number	Diff line number	Diff line change
`@@ -15,4 +15,6 @@ dependencies {`
`15`	`15`	`implementation(libs.smithy.protocol.test.traits)`
`16`	`16`	`// We have this because we're using RestJson1 as a 'generic' protocol.`
`17`	`17`	`implementation(libs.smithy.aws.traits)`
	`18`	`+ implementation(libs.jsoup)`
	`19`	`+ implementation(libs.commonmark)`
`18`	`20`	`}`