From 684a219a26848c602f5e709bfc7bcb37255f2657 Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 9 Jun 2021 12:40:12 -0400 Subject: [PATCH 1/4] fix: escape brackets in generated kdoc when identifier is invalid --- .../amazon/smithy/kotlin/codegen/core/KotlinWriter.kt | 3 ++- .../smithy/kotlin/codegen/core/KotlinWriterTest.kt | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt index 977426e39..a468d2549 100644 --- a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt +++ b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt @@ -295,7 +295,7 @@ private val commonHtmlTags = setOf( // NOTE: Currently we look for specific strings of Html tags commonly found in docs // and remove them. A better solution would be to generally convert from HTML to "pure" // markdown such that formatting is preserved. -// TODO: https://www.pivotaltracker.com/story/show/177053427 +// TODO: https://github.com/awslabs/smithy-kotlin/issues/136 private fun sanitizeDocumentation(doc: String): String = doc .stripAll(commonHtmlTags) // Docs can have valid $ characters that shouldn't run through formatters. @@ -303,6 +303,7 @@ private fun sanitizeDocumentation(doc: String): String = doc // Services may have comment string literals embedded in documentation. .replace("/*", "&##47;*") .replace("*/", "*&##47;") + .replace(Regex("(\\[)(.*\\.)(\\])"), "`$1`$2`$3`") // Remove all strings from source string and return the result private fun String.stripAll(stripList: List): String { diff --git a/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt index 3295c3c6b..f25f825cf 100644 --- a/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt +++ b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt @@ -50,4 +50,14 @@ class KotlinWriterTest { val actual = unit.toString() actual.shouldContainOnlyOnceWithDiff("here is some sweet sweet html") } + + @Test + fun `it escapes invalid markdown links`() { + // https://github.com/awslabs/aws-sdk-kotlin/issues/153 + val unit = KotlinWriter(TestModelDefault.NAMESPACE) + val docs = "UserName@[SubDomain.]Domain.TopLevelDomain" + unit.dokka(docs) + val actual = unit.toString() + actual.shouldContainOnlyOnceWithDiff("UserName@`[`SubDomain.`]`Domain.TopLevelDomain") + } } From eb303e7d0eb39ba7db1e09d8eb86386ad8d4a9dd Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 9 Jun 2021 13:28:16 -0400 Subject: [PATCH 2/4] fix: escape square brackets in all documentation traits --- .../kotlin/codegen/core/KotlinWriter.kt | 1 - .../codegen/lang/DocumentationPreprocessor.kt | 35 +++++++++ ...tlin.codegen.integration.KotlinIntegration | 1 + .../kotlin/codegen/core/KotlinWriterTest.kt | 10 --- .../lang/DocumentationPreprocessorTest.kt | 71 +++++++++++++++++++ 5 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt create mode 100644 smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt diff --git a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt index a468d2549..e8bd1ea64 100644 --- a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt +++ b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriter.kt @@ -303,7 +303,6 @@ private fun sanitizeDocumentation(doc: String): String = doc // Services may have comment string literals embedded in documentation. .replace("/*", "&##47;*") .replace("*/", "*&##47;") - .replace(Regex("(\\[)(.*\\.)(\\])"), "`$1`$2`$3`") // Remove all strings from source string and return the result private fun String.stripAll(stripList: List): String { diff --git a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt new file mode 100644 index 000000000..063b130d0 --- /dev/null +++ b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt @@ -0,0 +1,35 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.kotlin.codegen.lang + +import software.amazon.smithy.kotlin.codegen.KotlinSettings +import software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration +import software.amazon.smithy.model.Model +import software.amazon.smithy.model.traits.DocumentationTrait +import software.amazon.smithy.model.transform.ModelTransformer + +/** + * Sanitize all instances of [DocumentationTrait] + */ +class DocumentationPreprocessor : KotlinIntegration { + + override fun preprocessModel(model: Model, settings: KotlinSettings): Model { + val transformer = ModelTransformer.create() + return transformer.mapTraits(model) { _, trait -> + when (trait) { + is DocumentationTrait -> { + val docs = sanitize(trait.value) + DocumentationTrait(docs, trait.sourceLocation) + } + else -> trait + } + } + } + + // KDoc comments use inline markdown. Replace square brackets with escaped equivalents so that they + // are not rendered as invalid links + private fun sanitize(str: String): String = str.replace(Regex("(\\[)(.*)(\\])"), "[$2]") +} diff --git a/smithy-kotlin-codegen/src/main/resources/META-INF/services/software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration b/smithy-kotlin-codegen/src/main/resources/META-INF/services/software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration index 01d560c86..427dd7e30 100644 --- a/smithy-kotlin-codegen/src/main/resources/META-INF/services/software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration +++ b/smithy-kotlin-codegen/src/main/resources/META-INF/services/software.amazon.smithy.kotlin.codegen.integration.KotlinIntegration @@ -1 +1,2 @@ software.amazon.smithy.kotlin.codegen.lang.BuiltinPreprocessor +software.amazon.smithy.kotlin.codegen.lang.DocumentationPreprocessor diff --git a/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt index f25f825cf..3295c3c6b 100644 --- a/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt +++ b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/core/KotlinWriterTest.kt @@ -50,14 +50,4 @@ class KotlinWriterTest { val actual = unit.toString() actual.shouldContainOnlyOnceWithDiff("here is some sweet sweet html") } - - @Test - fun `it escapes invalid markdown links`() { - // https://github.com/awslabs/aws-sdk-kotlin/issues/153 - val unit = KotlinWriter(TestModelDefault.NAMESPACE) - val docs = "UserName@[SubDomain.]Domain.TopLevelDomain" - unit.dokka(docs) - val actual = unit.toString() - actual.shouldContainOnlyOnceWithDiff("UserName@`[`SubDomain.`]`Domain.TopLevelDomain") - } } diff --git a/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt new file mode 100644 index 000000000..a782fd780 --- /dev/null +++ b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt @@ -0,0 +1,71 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0. + */ + +package software.amazon.smithy.kotlin.codegen.lang + +import org.junit.jupiter.api.Test +import software.amazon.smithy.kotlin.codegen.KotlinSettings +import software.amazon.smithy.kotlin.codegen.model.expectTrait +import software.amazon.smithy.kotlin.codegen.test.toSmithyModel +import software.amazon.smithy.model.shapes.ShapeId +import software.amazon.smithy.model.traits.DocumentationTrait +import kotlin.test.assertEquals + +class DocumentationPreprocessorTest { + @Test + fun itEscapesSquareBrackets() { + // https://github.com/awslabs/aws-sdk-kotlin/issues/153 + val model = """ + namespace com.test + + service FooService { + version: "1.0.0" + } + + @documentation("This should not be modified") + structure Foo { + @documentation("member docs") + Unit: Unit, + Str: String + } + + @documentation("UserName@[SubDomain.]Domain.TopLevelDomain") + structure Unit { + Int: Integer + } + + union MyUnion { + @documentation("foo [bar baz qux] quux") + Integer: Integer, + String: String, + Unit: Unit + } + """.toSmithyModel() + + val settings = KotlinSettings( + ShapeId.from("com.test#FooService"), + KotlinSettings.PackageSettings( + "test", + "1.0", + "" + ), + "Foo" + ) + + val integration = DocumentationPreprocessor() + val modified = integration.preprocessModel(model, settings) + val expectedDocs = listOf( + "com.test#Foo" to "This should not be modified", + "com.test#Foo\$Unit" to "member docs", + "com.test#Unit" to "UserName@[SubDomain.]Domain.TopLevelDomain", + "com.test#MyUnion\$Integer" to "foo [bar baz qux] quux", + ) + expectedDocs.forEach { (shapeId, expected) -> + val shape = modified.expectShape(ShapeId.from(shapeId)) + val docs = shape.expectTrait().value + assertEquals(expected, docs) + } + } +} From 9e643a740721e0844644fbb2db99baf4efc7640d Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Wed, 9 Jun 2021 15:20:26 -0400 Subject: [PATCH 3/4] fix: handle nested brackets --- .../kotlin/codegen/lang/DocumentationPreprocessor.kt | 9 ++++++++- .../kotlin/codegen/lang/DocumentationPreprocessorTest.kt | 4 ++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt index 063b130d0..010d65392 100644 --- a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt +++ b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt @@ -31,5 +31,12 @@ class DocumentationPreprocessor : KotlinIntegration { // KDoc comments use inline markdown. Replace square brackets with escaped equivalents so that they // are not rendered as invalid links - private fun sanitize(str: String): String = str.replace(Regex("(\\[)(.*)(\\])"), "[$2]") + private fun sanitize(str: String): String { + val re = Regex("\\[(.*)\\]") + var result = str + while (re.containsMatchIn(result)) { + result = result.replace(re, "[$1]") + } + return result + } } diff --git a/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt index a782fd780..e5b5c8874 100644 --- a/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt +++ b/smithy-kotlin-codegen/src/test/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessorTest.kt @@ -37,7 +37,7 @@ class DocumentationPreprocessorTest { } union MyUnion { - @documentation("foo [bar baz qux] quux") + @documentation("foo [bar [baz] qux] quux") Integer: Integer, String: String, Unit: Unit @@ -60,7 +60,7 @@ class DocumentationPreprocessorTest { "com.test#Foo" to "This should not be modified", "com.test#Foo\$Unit" to "member docs", "com.test#Unit" to "UserName@[SubDomain.]Domain.TopLevelDomain", - "com.test#MyUnion\$Integer" to "foo [bar baz qux] quux", + "com.test#MyUnion\$Integer" to "foo [bar [baz] qux] quux", ) expectedDocs.forEach { (shapeId, expected) -> val shape = modified.expectShape(ShapeId.from(shapeId)) From 00ac1bac2392c2a122f7bb0e84460a3e10f4892b Mon Sep 17 00:00:00 2001 From: Aaron J Todd Date: Thu, 10 Jun 2021 10:21:37 -0400 Subject: [PATCH 4/4] you win again tests --- .../kotlin/codegen/lang/DocumentationPreprocessor.kt | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt index 010d65392..295932ffb 100644 --- a/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt +++ b/smithy-kotlin-codegen/src/main/kotlin/software/amazon/smithy/kotlin/codegen/lang/DocumentationPreprocessor.kt @@ -31,12 +31,7 @@ class DocumentationPreprocessor : KotlinIntegration { // KDoc comments use inline markdown. Replace square brackets with escaped equivalents so that they // are not rendered as invalid links - private fun sanitize(str: String): String { - val re = Regex("\\[(.*)\\]") - var result = str - while (re.containsMatchIn(result)) { - result = result.replace(re, "[$1]") - } - return result - } + private fun sanitize(str: String): String = + str.replace("[", "[") + .replace("]", "]") }