From d0f744869f30b06ad00013b527d611a81dada218 Mon Sep 17 00:00:00 2001 From: david-perez Date: Mon, 3 Jun 2024 17:56:50 +0200 Subject: [PATCH 01/24] Do not set `RUSTFLAGS` environment variable when invoking Cargo commands via Gradle tasks Otherwise, if you generate a crate and compile it using Gradle, like for example using the invocation: ``` ./gradlew -P modules='simple' -P cargoCommands='test' codegen-server-test:build ``` And then manually run a `cargo` command within the generated crate directory, or open the project using `rust-analyzer`, Cargo will re-compile the project from scratch, with `CARGO_LOG=cargo::core::compiler::fingerprint=trace` reporting that flags have changed since the last compilation. Instead, it's best if we persist these flags to `.cargo/config.toml`, so all `cargo` invocations, either through Gradle, manually, or through `rust-analyzer`, use the same set. This way, if no files were changed, subsequent compilations since code generation will truly be no-ops, with Cargo reusing all artifacts. Note this commit fixes a regression that was introduced when `--cfg aws_sdk_unstable` was introduced in #2614, since I fixed this the first time back in #1422. --- aws/sdk-adhoc-test/build.gradle.kts | 3 +-- aws/sdk/build.gradle.kts | 3 +-- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 14 ++++---------- codegen-client-test/build.gradle.kts | 3 +-- codegen-server-test/build.gradle.kts | 3 +-- codegen-server-test/python/build.gradle.kts | 3 +-- codegen-server-test/typescript/build.gradle.kts | 3 +-- gradle.properties | 8 +------- 8 files changed, 11 insertions(+), 29 deletions(-) diff --git a/aws/sdk-adhoc-test/build.gradle.kts b/aws/sdk-adhoc-test/build.gradle.kts index 1f2b7fde5a..3bdc663dad 100644 --- a/aws/sdk-adhoc-test/build.gradle.kts +++ b/aws/sdk-adhoc-test/build.gradle.kts @@ -20,7 +20,6 @@ java { } val smithyVersion: String by project -val defaultRustDocFlags: String by project val properties = PropertyRetriever(rootProject, project) val pluginName = "rust-client-codegen" @@ -78,7 +77,7 @@ tasks["smithyBuild"].dependsOn("generateSmithyBuild") tasks["assemble"].finalizedBy("generateCargoWorkspace") project.registerModifyMtimeTask() -project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile, defaultRustDocFlags) +project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile) tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString }) diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 6e2165eb12..95b27bad1e 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -31,7 +31,6 @@ configure { } val smithyVersion: String by project -val defaultRustDocFlags: String by project val properties = PropertyRetriever(rootProject, project) val crateHasherToolPath = rootProject.projectDir.resolve("tools/ci-build/crate-hasher") @@ -442,7 +441,7 @@ tasks["assemble"].apply { outputs.upToDateWhen { false } } -project.registerCargoCommandsTasks(outputDir.asFile, defaultRustDocFlags) +project.registerCargoCommandsTasks(outputDir.asFile) project.registerGenerateCargoConfigTomlTask(outputDir.asFile) //The task name "test" is already registered by one of our plugins diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index b7681a5b56..55af4d3655 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -205,13 +205,15 @@ fun Project.registerGenerateCargoWorkspaceTask( fun Project.registerGenerateCargoConfigTomlTask(outputDir: File) { this.tasks.register("generateCargoConfigToml") { description = "generate `.cargo/config.toml`" + // TODO(https://github.com/smithy-lang/smithy-rs/issues/1068): Once doc normalization + // is completed, warnings can be prohibited in rustdoc by setting `rustdocflags` to `-D warnings`. doFirst { outputDir.resolve(".cargo").mkdirs() outputDir.resolve(".cargo/config.toml") .writeText( """ [build] - rustflags = ["--deny", "warnings"] + rustflags = ["--deny", "warnings", "--cfg", "aws_sdk_unstable"] """.trimIndent(), ) } @@ -255,10 +257,7 @@ fun Project.registerModifyMtimeTask() { } } -fun Project.registerCargoCommandsTasks( - outputDir: File, - defaultRustDocFlags: String, -) { +fun Project.registerCargoCommandsTasks(outputDir: File) { val dependentTasks = listOfNotNull( "assemble", @@ -269,29 +268,24 @@ fun Project.registerCargoCommandsTasks( this.tasks.register(Cargo.CHECK.toString) { dependsOn(dependentTasks) workingDir(outputDir) - environment("RUSTFLAGS", "--cfg aws_sdk_unstable") commandLine("cargo", "check", "--lib", "--tests", "--benches", "--all-features") } this.tasks.register(Cargo.TEST.toString) { dependsOn(dependentTasks) workingDir(outputDir) - environment("RUSTFLAGS", "--cfg aws_sdk_unstable") commandLine("cargo", "test", "--all-features", "--no-fail-fast") } this.tasks.register(Cargo.DOCS.toString) { dependsOn(dependentTasks) workingDir(outputDir) - environment("RUSTDOCFLAGS", defaultRustDocFlags) - environment("RUSTFLAGS", "--cfg aws_sdk_unstable") commandLine("cargo", "doc", "--no-deps", "--document-private-items") } this.tasks.register(Cargo.CLIPPY.toString) { dependsOn(dependentTasks) workingDir(outputDir) - environment("RUSTFLAGS", "--cfg aws_sdk_unstable") commandLine("cargo", "clippy") } } diff --git a/codegen-client-test/build.gradle.kts b/codegen-client-test/build.gradle.kts index de4b54d5b3..c69a4ae719 100644 --- a/codegen-client-test/build.gradle.kts +++ b/codegen-client-test/build.gradle.kts @@ -15,7 +15,6 @@ plugins { } val smithyVersion: String by project -val defaultRustDocFlags: String by project val properties = PropertyRetriever(rootProject, project) fun getSmithyRuntimeMode(): String = properties.get("smithy.runtime.mode") ?: "orchestrator" @@ -125,7 +124,7 @@ tasks["smithyBuild"].dependsOn("generateSmithyBuild") tasks["assemble"].finalizedBy("generateCargoWorkspace") project.registerModifyMtimeTask() -project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile, defaultRustDocFlags) +project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile) tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString }) diff --git a/codegen-server-test/build.gradle.kts b/codegen-server-test/build.gradle.kts index cab36dbbcc..4134cdd039 100644 --- a/codegen-server-test/build.gradle.kts +++ b/codegen-server-test/build.gradle.kts @@ -16,7 +16,6 @@ plugins { } val smithyVersion: String by project -val defaultRustDocFlags: String by project val properties = PropertyRetriever(rootProject, project) val pluginName = "rust-server-codegen" @@ -103,7 +102,7 @@ tasks["smithyBuild"].dependsOn("generateSmithyBuild") tasks["assemble"].finalizedBy("generateCargoWorkspace", "generateCargoConfigToml") project.registerModifyMtimeTask() -project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile, defaultRustDocFlags) +project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile) tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString }) diff --git a/codegen-server-test/python/build.gradle.kts b/codegen-server-test/python/build.gradle.kts index f9129a2fde..b6ee22147a 100644 --- a/codegen-server-test/python/build.gradle.kts +++ b/codegen-server-test/python/build.gradle.kts @@ -16,7 +16,6 @@ plugins { } val smithyVersion: String by project -val defaultRustDocFlags: String by project val properties = PropertyRetriever(rootProject, project) val buildDir = layout.buildDirectory.get().asFile @@ -120,7 +119,7 @@ tasks["smithyBuild"].dependsOn("generateSmithyBuild") tasks["assemble"].finalizedBy("generateCargoWorkspace") project.registerModifyMtimeTask() -project.registerCargoCommandsTasks(buildDir.resolve(workingDirUnderBuildDir), defaultRustDocFlags) +project.registerCargoCommandsTasks(buildDir.resolve(workingDirUnderBuildDir)) tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString }) diff --git a/codegen-server-test/typescript/build.gradle.kts b/codegen-server-test/typescript/build.gradle.kts index 428da17df6..22c27b7d90 100644 --- a/codegen-server-test/typescript/build.gradle.kts +++ b/codegen-server-test/typescript/build.gradle.kts @@ -16,7 +16,6 @@ plugins { } val smithyVersion: String by project -val defaultRustDocFlags: String by project val properties = PropertyRetriever(rootProject, project) val buildDir = layout.buildDirectory.get().asFile @@ -49,7 +48,7 @@ tasks["smithyBuild"].dependsOn("generateSmithyBuild") tasks["assemble"].finalizedBy("generateCargoWorkspace") project.registerModifyMtimeTask() -project.registerCargoCommandsTasks(buildDir.resolve(workingDirUnderBuildDir), defaultRustDocFlags) +project.registerCargoCommandsTasks(buildDir.resolve(workingDirUnderBuildDir)) tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString }) diff --git a/gradle.properties b/gradle.properties index 6d06ec08dc..c9e5ab7774 100644 --- a/gradle.properties +++ b/gradle.properties @@ -31,10 +31,4 @@ kotlinVersion=1.9.20 ktlintVersion=1.0.1 kotestVersion=5.8.0 # Avoid registering dependencies/plugins/tasks that are only used for testing purposes -isTestingEnabled=true - -# TODO(https://github.com/smithy-lang/smithy-rs/issues/1068): Once doc normalization -# is completed, warnings can be prohibited in rustdoc. -# -# defaultRustDocFlags=-D warnings -defaultRustDocFlags= +isTestingEnabled=true \ No newline at end of file From c7b242c807f5edaacef2fcf2a9a6a1e113692b4f Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 4 Jun 2024 16:25:41 +0200 Subject: [PATCH 02/24] Clear RUSTDOCFLAGS --- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index 55af4d3655..8557e54da0 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -280,6 +280,10 @@ fun Project.registerCargoCommandsTasks(outputDir: File) { this.tasks.register(Cargo.DOCS.toString) { dependsOn(dependentTasks) workingDir(outputDir) + // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194#issuecomment-2147657902) + // Clear `RUSTDOCFLAGS` because in the CI Docker image we bake in `-D warnings`, but we currently + // generate docs with warnings. + environment("RUSTDOCFLAGS", "") commandLine("cargo", "doc", "--no-deps", "--document-private-items") } From 0dfd5714ab1b58f24f72ad70245a0ab8181dbd0f Mon Sep 17 00:00:00 2001 From: david-perez Date: Tue, 4 Jun 2024 17:51:30 +0200 Subject: [PATCH 03/24] Fix some Clippy warnings --- .../ServerBuilderGeneratorCommon.kt | 37 ++++++++++++------- .../generators/ServerServiceGenerator.kt | 1 + 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt index 9c80cd1258..0dac1299a5 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt @@ -38,6 +38,8 @@ import software.amazon.smithy.rust.codegen.core.rustlang.rust import software.amazon.smithy.rust.codegen.core.rustlang.rustTemplate import software.amazon.smithy.rust.codegen.core.rustlang.writable import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType +import software.amazon.smithy.rust.codegen.core.smithy.RuntimeType.Companion.preludeScope import software.amazon.smithy.rust.codegen.core.smithy.RustSymbolProvider import software.amazon.smithy.rust.codegen.core.smithy.generators.PrimitiveInstantiator import software.amazon.smithy.rust.codegen.core.smithy.rustType @@ -108,15 +110,16 @@ fun generateFallbackCodeToDefaultValue( "DefaultValue" to defaultValue, ) } else { - when (targetShape) { - is NumberShape, is EnumShape, is BooleanShape -> { - writer.rustTemplate(".unwrap_or(#{DefaultValue:W})", "DefaultValue" to defaultValue) - } + val node = member.expectTrait().toNode()!! + if ((targetShape is DocumentShape && (node is BooleanNode || node is NumberNode)) || + targetShape is BooleanShape || + targetShape is NumberShape || + targetShape is EnumShape) { + writer.rustTemplate(".unwrap_or(#{DefaultValue:W})", "DefaultValue" to defaultValue) + } else { // Values for the Rust types of the rest of the shapes require heap allocations, so we calculate them - // in a (lazily-executed) closure for slight performance gains. - else -> { - writer.rustTemplate(".unwrap_or_else(|| #{DefaultValue:W})", "DefaultValue" to defaultValue) - } + // in a (lazily-executed) closure for minimal performance gains. + writer.rustTemplate(".unwrap_or_else(##[allow(clippy::redundant_closure)] || #{DefaultValue:W})", "DefaultValue" to defaultValue) } } } @@ -125,7 +128,7 @@ fun generateFallbackCodeToDefaultValue( * Returns a writable to construct a Rust value of the correct type holding the modeled `@default` value on the * [member] shape. */ -fun defaultValue( +private fun defaultValue( model: Model, runtimeConfig: RuntimeConfig, symbolProvider: RustSymbolProvider, @@ -164,12 +167,12 @@ fun defaultValue( } is ListShape -> { check(node is ArrayNode && node.isEmpty) - rust("Vec::new()") + rustTemplate("#{Vec}::new()", *preludeScope) } is MapShape -> { check(node is ObjectNode && node.isEmpty) - rust("std::collections::HashMap::new()") + rustTemplate("#{HashMap}::new()", "HashMap" to RuntimeType.HashMap) } is DocumentShape -> { @@ -188,7 +191,8 @@ fun defaultValue( is StringNode -> rustTemplate( - "#{SmithyTypes}::Document::String(String::from(${node.value.dq()}))", + "#{SmithyTypes}::Document::String(#{String}::from(${node.value.dq()}))", + *preludeScope, "SmithyTypes" to types, ) @@ -207,14 +211,19 @@ fun defaultValue( is ArrayNode -> { check(node.isEmpty) - rustTemplate("""#{SmithyTypes}::Document::Array(Vec::new())""", "SmithyTypes" to types) + rustTemplate( + """#{SmithyTypes}::Document::Array(#{Vec}::new())""", + *preludeScope, + "SmithyTypes" to types + ) } is ObjectNode -> { check(node.isEmpty) rustTemplate( - "#{SmithyTypes}::Document::Object(std::collections::HashMap::new())", + "#{SmithyTypes}::Document::Object(#{HashMap}::new())", "SmithyTypes" to types, + "HashMap" to RuntimeType.HashMap, ) } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt index 1898c6c3db..db95531f71 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerServiceGenerator.kt @@ -756,6 +756,7 @@ class ServerServiceGenerator( rustTemplate( """ /// An enumeration of all [operations](https://smithy.io/2.0/spec/service-types.html##operation) in $serviceName. + ##[allow(clippy::enum_variant_names)] ##[derive(Debug, PartialEq, Eq, Clone, Copy)] pub enum Operation { $operations From ee343d5998b97fb284fa65a5f4ad257d124ff5bd Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 12:15:11 +0200 Subject: [PATCH 04/24] Fix more Clippy warnings --- .../generators/http/HttpBindingGenerator.kt | 18 +++++++----------- .../generators/ServerBuilderGeneratorCommon.kt | 4 ++-- .../http/ServerRequestBindingGenerator.kt | 2 ++ 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt index 9687bb2492..c9997ba67a 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/http/HttpBindingGenerator.kt @@ -332,17 +332,13 @@ class HttpBindingGenerator( } } if (targetShape.hasTrait()) { - if (codegenTarget == CodegenTarget.SERVER) { - rust( - "Ok(#T::try_from(body_str)?)", - symbolProvider.toSymbol(targetShape), - ) - } else { - rust( - "Ok(#T::from(body_str))", - symbolProvider.toSymbol(targetShape), - ) - } + // - In servers, `T` is an unconstrained `String` that will be constrained when building the + // builder. + // - In clients, `T` will directly be the target generated enum type. + rust( + "Ok(#T::from(body_str))", + symbolProvider.toSymbol(targetShape), + ) } else { rust("Ok(body_str.to_string())") } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt index 0dac1299a5..d17be36d09 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt @@ -117,8 +117,8 @@ fun generateFallbackCodeToDefaultValue( targetShape is EnumShape) { writer.rustTemplate(".unwrap_or(#{DefaultValue:W})", "DefaultValue" to defaultValue) } else { - // Values for the Rust types of the rest of the shapes require heap allocations, so we calculate them - // in a (lazily-executed) closure for minimal performance gains. + // Values for the Rust types of the rest of the shapes might require heap allocations, + // so we calculate them in a (lazily-executed) closure for minimal performance gains. writer.rustTemplate(".unwrap_or_else(##[allow(clippy::redundant_closure)] || #{DefaultValue:W})", "DefaultValue" to defaultValue) } } diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt index ce02e2c99b..12ecad9b4b 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/http/ServerRequestBindingGenerator.kt @@ -34,6 +34,8 @@ class ServerRequestBindingGenerator( HttpBindingGenerator( protocol, codegenContext, + // Note how we parse the HTTP-bound values into _unconstrained_ types; they will be constrained when + // building the builder. codegenContext.unconstrainedShapeSymbolProvider, operationShape, listOf( From e3ad6ccfe01b9d324678a2dd6f874673f553c1fa Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 12:17:07 +0200 Subject: [PATCH 05/24] Add EOF newline to gradle.properties --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index c9e5ab7774..3180aa95ff 100644 --- a/gradle.properties +++ b/gradle.properties @@ -31,4 +31,4 @@ kotlinVersion=1.9.20 ktlintVersion=1.0.1 kotestVersion=5.8.0 # Avoid registering dependencies/plugins/tasks that are only used for testing purposes -isTestingEnabled=true \ No newline at end of file +isTestingEnabled=true From c1060efe5091e83f871f81d395c02491155b343a Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 12:56:41 +0200 Subject: [PATCH 06/24] Remove unnecessary semicolon --- .../client/smithy/generators/client/FluentClientGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index a39849d59c..cc5d78fe65 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt @@ -277,7 +277,7 @@ private fun baseClientRuntimePluginsFn( .with_client_plugin(crate::config::ServiceRuntimePlugin::new(config.clone())) .with_client_plugin(#{NoAuthRuntimePlugin}::new()); - #{additional_client_plugins:W}; + #{additional_client_plugins:W} for plugin in configured_plugins { plugins = plugins.with_client_plugin(plugin); From f891ab6c413cd5d9f38dee086a0692afc4ac1e09 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 13:05:04 +0200 Subject: [PATCH 07/24] Fix fluent client docs --- .../codegen/client/smithy/generators/client/FluentClientDocs.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt index 1ceaf043cc..4660da8fd0 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt @@ -34,7 +34,7 @@ object FluentClientDocs { or identity resolver to be configured. The config is used to customize various aspects of the client, such as: - - [HTTP Connector](crate::config::Builder::http_connector) + - [the underlying HTTP client](crate::config::Builder::http_client) - [Retry](crate::config::Builder::retry_config) - [Timeouts](crate::config::Builder::timeout_config) - [... and more](crate::config::Builder) From ec934439d8e80268e6cb0de5d00ed4ad4bc555c7 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 14:03:37 +0200 Subject: [PATCH 08/24] Escape brackets in HTML elements' text --- .../rust/codegen/core/rustlang/RustWriter.kt | 49 +++++++++++++++++-- .../codegen/core/rustlang/RustWriterTest.kt | 12 +++++ 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt index c0922525b0..94b3dc67c6 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriter.kt @@ -8,6 +8,7 @@ package software.amazon.smithy.rust.codegen.core.rustlang import org.intellij.lang.annotations.Language import org.jsoup.Jsoup import org.jsoup.nodes.Element +import org.jsoup.select.Elements import software.amazon.smithy.codegen.core.CodegenException import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.codegen.core.SymbolDependencyContainer @@ -336,10 +337,10 @@ fun > T.docsOrFallback( docs("_Note: ${it}_") } } else if (autoSuppressMissingDocs) { + // Otherwise, suppress the missing docs lint for this shape since + // the lack of documentation is a modeling issue rather than a codegen issue. rust("##[allow(missing_docs)] // documentation missing in model") } - // Otherwise, suppress the missing docs lint for this shape since - // the lack of documentation is a modeling issue rather than a codegen issue. return this } @@ -439,16 +440,56 @@ fun RustWriter.deprecatedShape(shape: Shape): RustWriter { fun > T.escape(text: String): String = text.replace("$expressionStart", "$expressionStart$expressionStart") -/** Parse input as HTML and normalize it */ +/** Parse input as HTML and normalize it, for suitable use within Rust docs. */ fun normalizeHtml(input: String): String { val doc = Jsoup.parse(input) doc.body().apply { - normalizeAnchors() // Convert anchor tags missing href attribute into pre tags + normalizeAnchors() + escapeBracketsInText() } return doc.body().html() } +/** + * Escape brackets in elements' inner text. + * + * For example: + * + * ```html + * + *

Text without brackets

+ *
Some text with [brackets]
+ * Another [example, 1] + * + * ``` + * + * gets converted to: + * + * ```html + * + *

Text without brackets

+ *
Some text with \[brackets\]
+ * Another \[example, 1\] + * + * ``` + * + * This is useful when rendering Rust docs, since text within brackets might get misinterpreted as broken Markdown doc + * links (`[link text](https://example.com)`). + **/ +private fun Element.escapeBracketsInText() { + // Select all elements that directly contain text with opening or closing brackets. + val elements: Elements = select("*:containsOwn([), *:containsOwn(])") + + // Loop through each element and escape the brackets in the text. + for (element: Element in elements) { + val originalText = element.text() + val escapedText = originalText.replace("[", "\\[").replace("]", "\\]") + element.text(escapedText) + } +} + +/** Convert anchor tags missing `href` attribute into `code` tags. **/ private fun Element.normalizeAnchors() { getElementsByTag("a").forEach { val link = it.attr("href") diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt index 18f8cbfadc..0b834d5205 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt @@ -99,6 +99,18 @@ class RustWriterTest { output shouldContain "/// Top level module" } + @Test + fun `normalize HTML`() { + val output = normalizeHtml(""" + Link without href attribute +
Some text with [brackets]
+ ] mismatched [ is escaped too + """) + output shouldContain "Link without href attribute" + output shouldContain "Some text with \\[brackets\\]" + output shouldContain "\\] mismatched \\[ is escaped too" + } + @Test fun `generate doc links`() { val model = From 9cf2f0c2f9131e30b92984f68e501e789f15d32d Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 15:17:54 +0200 Subject: [PATCH 09/24] Dont reference types module if not generated --- .../codegen/client/smithy/ClientRustModule.kt | 21 ++++++++++++++++--- .../customizations/ClientDocsGenerator.kt | 18 +++++++++++++++- 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt index 2da355cb5c..ddc04afd00 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt @@ -7,10 +7,14 @@ package software.amazon.smithy.rust.codegen.client.smithy import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.Model +import software.amazon.smithy.model.shapes.EnumShape +import software.amazon.smithy.model.shapes.IntEnumShape import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.shapes.Shape +import software.amazon.smithy.model.shapes.StringShape import software.amazon.smithy.model.shapes.StructureShape import software.amazon.smithy.model.shapes.UnionShape +import software.amazon.smithy.model.traits.EnumTrait import software.amazon.smithy.model.traits.ErrorTrait import software.amazon.smithy.rust.codegen.client.smithy.generators.client.FluentClientDocs import software.amazon.smithy.rust.codegen.client.smithy.generators.client.FluentClientGenerator @@ -195,8 +199,9 @@ object ClientModuleProvider : ModuleProvider { override fun moduleForShape( context: ModuleProviderContext, shape: Shape, - ): RustModule.LeafModule = - when (shape) { + ): RustModule.LeafModule { + fun shouldNotBeRendered(): Nothing = PANIC("Shape ${shape.id} should not be rendered in any module") + return when (shape) { is OperationShape -> perOperationModule(context, shape) is StructureShape -> when { @@ -206,8 +211,18 @@ object ClientModuleProvider : ModuleProvider { else -> ClientRustModule.types } - else -> ClientRustModule.types + is EnumShape -> ClientRustModule.types + is StringShape -> { + if (shape.hasTrait()) { + ClientRustModule.types + } else { + shouldNotBeRendered() + } + } + + else -> shouldNotBeRendered() } + } override fun moduleForOperationError( context: ModuleProviderContext, diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt index fb709ac61e..138d78d672 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt @@ -7,9 +7,11 @@ package software.amazon.smithy.rust.codegen.client.smithy.customizations import software.amazon.smithy.model.traits.TitleTrait import software.amazon.smithy.rust.codegen.client.smithy.ClientCodegenContext +import software.amazon.smithy.rust.codegen.client.smithy.ClientRustModule import software.amazon.smithy.rust.codegen.core.rustlang.Writable import software.amazon.smithy.rust.codegen.core.rustlang.containerDocs import software.amazon.smithy.rust.codegen.core.rustlang.writable +import software.amazon.smithy.rust.codegen.core.smithy.DirectedWalker import software.amazon.smithy.rust.codegen.core.smithy.generators.LibRsCustomization import software.amazon.smithy.rust.codegen.core.smithy.generators.LibRsSection import software.amazon.smithy.rust.codegen.core.smithy.generators.ModuleDocSection @@ -30,6 +32,20 @@ class ClientDocsGenerator(private val codegenContext: ClientCodegenContext) : Li private fun crateLayout(): Writable = writable { + val hasTypesModule = DirectedWalker(codegenContext.model).walkShapes(codegenContext.serviceShape) + .any { + try { + codegenContext.symbolProvider.moduleForShape(it).name == ClientRustModule.types.name + } catch (ex: RuntimeException) { + // The shape should not be rendered in any module. + false + } + } + val typesModuleSentence = if (hasTypesModule) { + "These structs and enums live in [`types`](crate::types). " + } else { + "" + } val serviceName = codegenContext.serviceShape.getTrait()?.value ?: "the service" containerDocs( """ @@ -40,7 +56,7 @@ class ClientDocsGenerator(private val codegenContext: ClientCodegenContext) : Li either a successful output or a [`SdkError`](crate::error::SdkError). Some of these API inputs may be structs or enums to provide more complex structured information. - These structs and enums live in [`types`](crate::types). There are some simpler types for + ${typesModuleSentence}There are some simpler types for representing data such as date times or binary blobs that live in [`primitives`](crate::primitives). All types required to configure a client via the [`Config`](crate::Config) struct live From 4e98dda6daf2c32105b8a02b70cca3261b282907 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 15:18:33 +0200 Subject: [PATCH 10/24] Comment out clearing RUSTDOCFLAGS --- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index 8557e54da0..cd2ff0d68f 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -283,7 +283,7 @@ fun Project.registerCargoCommandsTasks(outputDir: File) { // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194#issuecomment-2147657902) // Clear `RUSTDOCFLAGS` because in the CI Docker image we bake in `-D warnings`, but we currently // generate docs with warnings. - environment("RUSTDOCFLAGS", "") +// environment("RUSTDOCFLAGS", "") commandLine("cargo", "doc", "--no-deps", "--document-private-items") } From e75a6e0ad461a381974aa972abd35e3011fb6add Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 16:53:54 +0200 Subject: [PATCH 11/24] Use [ instead of < in Smithy docs for misc.smithy --- codegen-core/common-test-models/misc.smithy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-core/common-test-models/misc.smithy b/codegen-core/common-test-models/misc.smithy index 10072543c9..42329ba1d2 100644 --- a/codegen-core/common-test-models/misc.smithy +++ b/codegen-core/common-test-models/misc.smithy @@ -9,7 +9,7 @@ use smithy.framework#ValidationException /// A service to test miscellaneous aspects of code generation where protocol /// selection is not relevant. If you want to test something protocol-specific, -/// add it to a separate `-extras.smithy`. +/// add it to a separate `[protocol]-extras.smithy`. @restJson1 @title("MiscService") service MiscService { From 741d9efa436ac86a3fbc7cd6cdbf787d02d441f4 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 16:54:23 +0200 Subject: [PATCH 12/24] Unions are generated in the types module --- .../smithy/rust/codegen/client/smithy/ClientRustModule.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt index ddc04afd00..2e41c36994 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt @@ -211,7 +211,7 @@ object ClientModuleProvider : ModuleProvider { else -> ClientRustModule.types } - is EnumShape -> ClientRustModule.types + is UnionShape, is EnumShape -> ClientRustModule.types is StringShape -> { if (shape.hasTrait()) { ClientRustModule.types From 1d249c2cc9f5fb0c2de37c2612ff45014be57e77 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 16:55:05 +0200 Subject: [PATCH 13/24] ./gradlew ktlintFormat --- .../codegen/client/smithy/ClientRustModule.kt | 1 - .../customizations/ClientDocsGenerator.kt | 26 ++++++++++--------- .../codegen/core/rustlang/RustWriterTest.kt | 7 +++-- .../ServerBuilderGeneratorCommon.kt | 5 ++-- 4 files changed, 22 insertions(+), 17 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt index 2e41c36994..ccd21edc6e 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/ClientRustModule.kt @@ -8,7 +8,6 @@ package software.amazon.smithy.rust.codegen.client.smithy import software.amazon.smithy.codegen.core.Symbol import software.amazon.smithy.model.Model import software.amazon.smithy.model.shapes.EnumShape -import software.amazon.smithy.model.shapes.IntEnumShape import software.amazon.smithy.model.shapes.OperationShape import software.amazon.smithy.model.shapes.Shape import software.amazon.smithy.model.shapes.StringShape diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt index 138d78d672..d90b04c1fe 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/customizations/ClientDocsGenerator.kt @@ -32,20 +32,22 @@ class ClientDocsGenerator(private val codegenContext: ClientCodegenContext) : Li private fun crateLayout(): Writable = writable { - val hasTypesModule = DirectedWalker(codegenContext.model).walkShapes(codegenContext.serviceShape) - .any { - try { - codegenContext.symbolProvider.moduleForShape(it).name == ClientRustModule.types.name - } catch (ex: RuntimeException) { - // The shape should not be rendered in any module. - false + val hasTypesModule = + DirectedWalker(codegenContext.model).walkShapes(codegenContext.serviceShape) + .any { + try { + codegenContext.symbolProvider.moduleForShape(it).name == ClientRustModule.types.name + } catch (ex: RuntimeException) { + // The shape should not be rendered in any module. + false + } } + val typesModuleSentence = + if (hasTypesModule) { + "These structs and enums live in [`types`](crate::types). " + } else { + "" } - val typesModuleSentence = if (hasTypesModule) { - "These structs and enums live in [`types`](crate::types). " - } else { - "" - } val serviceName = codegenContext.serviceShape.getTrait()?.value ?: "the service" containerDocs( """ diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt index 0b834d5205..63fb4076a7 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt @@ -101,11 +101,14 @@ class RustWriterTest { @Test fun `normalize HTML`() { - val output = normalizeHtml(""" + val output = + normalizeHtml( + """ Link without href attribute
Some text with [brackets]
] mismatched [ is escaped too - """) + """, + ) output shouldContain "Link without href attribute" output shouldContain "Some text with \\[brackets\\]" output shouldContain "\\] mismatched \\[ is escaped too" diff --git a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt index d17be36d09..1035928204 100644 --- a/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt +++ b/codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt @@ -114,7 +114,8 @@ fun generateFallbackCodeToDefaultValue( if ((targetShape is DocumentShape && (node is BooleanNode || node is NumberNode)) || targetShape is BooleanShape || targetShape is NumberShape || - targetShape is EnumShape) { + targetShape is EnumShape + ) { writer.rustTemplate(".unwrap_or(#{DefaultValue:W})", "DefaultValue" to defaultValue) } else { // Values for the Rust types of the rest of the shapes might require heap allocations, @@ -214,7 +215,7 @@ private fun defaultValue( rustTemplate( """#{SmithyTypes}::Document::Array(#{Vec}::new())""", *preludeScope, - "SmithyTypes" to types + "SmithyTypes" to types, ) } From 738f241c37c5d691c7a4ede3cb19c34f7518d105 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 17:19:32 +0200 Subject: [PATCH 14/24] Fix doc warning when generating unions with unit variants --- .../rust/codegen/core/smithy/generators/UnionGenerator.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt index e25fccdeb6..335edf4271 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt @@ -200,7 +200,8 @@ private fun RustWriter.renderAsVariant( ) { if (member.isTargetUnit()) { rust( - "/// Tries to convert the enum instance into [`$variantName`], extracting the inner `()`.", + "/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner `()`.", + unionSymbol ) rust("/// Returns `Err(&Self)` if it can't be converted.") rustBlockTemplate("pub fn as_$funcNamePart(&self) -> #{Result}<(), &Self>", *preludeScope) { From a6ed02b70cdf586957f8cf6187616a4a280ee362 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 17:31:34 +0200 Subject: [PATCH 15/24] Fix doc warning when referring to client methods with Rust reserved names --- .../client/smithy/generators/client/FluentClientDocs.kt | 2 +- .../smithy/generators/client/FluentClientGenerator.kt | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt index 4660da8fd0..c8d3b3b6b0 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt @@ -76,7 +76,7 @@ object FluentClientDocs { if (operation != null && member != null) { val operationSymbol = symbolProvider.toSymbol(operation) val memberSymbol = symbolProvider.toSymbol(member) - val operationFnName = FluentClientGenerator.clientOperationFnName(operation, symbolProvider) + val operationFnName = FluentClientGenerator.clientOperationFnDocsName(operation, symbolProvider) docsTemplate( """ ## Using the `Client` diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt index cc5d78fe65..f7a7ff3b07 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientGenerator.kt @@ -59,7 +59,13 @@ class FluentClientGenerator( fun clientOperationFnName( operationShape: OperationShape, symbolProvider: RustSymbolProvider, - ): String = RustReservedWords.escapeIfNeeded(symbolProvider.toSymbol(operationShape).name.toSnakeCase()) + ): String = RustReservedWords.escapeIfNeeded(clientOperationFnDocsName(operationShape, symbolProvider)) + + /** When using the function name in Rust docs, there's no need to escape Rust reserved words. **/ + fun clientOperationFnDocsName( + operationShape: OperationShape, + symbolProvider: RustSymbolProvider, + ): String = symbolProvider.toSymbol(operationShape).name.toSnakeCase() fun clientOperationModuleName( operationShape: OperationShape, From d700dd41bce24b84d7b49c18a4044ca81dead225 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 17:32:17 +0200 Subject: [PATCH 16/24] Docs adjustment --- .../client/smithy/generators/client/FluentClientDocs.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt index c8d3b3b6b0..7570a7ce3a 100644 --- a/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt +++ b/codegen-client/src/main/kotlin/software/amazon/smithy/rust/codegen/client/smithy/generators/client/FluentClientDocs.kt @@ -34,8 +34,8 @@ object FluentClientDocs { or identity resolver to be configured. The config is used to customize various aspects of the client, such as: - - [the underlying HTTP client](crate::config::Builder::http_client) - - [Retry](crate::config::Builder::retry_config) + - [The underlying HTTP client](crate::config::Builder::http_client) + - [Retries](crate::config::Builder::retry_config) - [Timeouts](crate::config::Builder::timeout_config) - [... and more](crate::config::Builder) From dc67bc358e72be1c9ae8130a9ebeacb7dacd7aa0 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 17:43:04 +0200 Subject: [PATCH 17/24] Allow broken intra doc links in server codegen integration tests temporarily --- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 18 ++++++++++++------ codegen-server-test/build.gradle.kts | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index cd2ff0d68f..51b3837d6a 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -257,7 +257,7 @@ fun Project.registerModifyMtimeTask() { } } -fun Project.registerCargoCommandsTasks(outputDir: File) { +fun Project.registerCargoCommandsTasks(outputDir: File, allowBrokenIntraDocLinks: Boolean = false) { val dependentTasks = listOfNotNull( "assemble", @@ -280,11 +280,17 @@ fun Project.registerCargoCommandsTasks(outputDir: File) { this.tasks.register(Cargo.DOCS.toString) { dependsOn(dependentTasks) workingDir(outputDir) - // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194#issuecomment-2147657902) - // Clear `RUSTDOCFLAGS` because in the CI Docker image we bake in `-D warnings`, but we currently - // generate docs with warnings. -// environment("RUSTDOCFLAGS", "") - commandLine("cargo", "doc", "--no-deps", "--document-private-items") + val args = mutableListOf( + "--no-deps", + "--document-private-items", + ) + if (allowBrokenIntraDocLinks) { + // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194#issuecomment-2147657902) + // TODO(https://github.com/smithy-lang/smithy-rs/pull/3648) This is the _only_ reason why we need to allow + // broken intra doc links. Once it lands we can remove this escape hatch and deny _all_ warnings. + args += "-A rustdoc::broken-intra-doc-links" + } + commandLine("cargo", "doc", args) } this.tasks.register(Cargo.CLIPPY.toString) { diff --git a/codegen-server-test/build.gradle.kts b/codegen-server-test/build.gradle.kts index 4134cdd039..a8e50e0847 100644 --- a/codegen-server-test/build.gradle.kts +++ b/codegen-server-test/build.gradle.kts @@ -102,7 +102,7 @@ tasks["smithyBuild"].dependsOn("generateSmithyBuild") tasks["assemble"].finalizedBy("generateCargoWorkspace", "generateCargoConfigToml") project.registerModifyMtimeTask() -project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile) +project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile, allowBrokenIntraDocLinks = true) tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString }) From cfbcba8dc5340cd584cca64de220cddfad4fbe88 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 17:56:44 +0200 Subject: [PATCH 18/24] Fix cargo doc invocation --- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index 51b3837d6a..967dabf586 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -290,7 +290,7 @@ fun Project.registerCargoCommandsTasks(outputDir: File, allowBrokenIntraDocLinks // broken intra doc links. Once it lands we can remove this escape hatch and deny _all_ warnings. args += "-A rustdoc::broken-intra-doc-links" } - commandLine("cargo", "doc", args) + commandLine("cargo", "doc", *args.toTypedArray()) } this.tasks.register(Cargo.CLIPPY.toString) { From 7d99365f66ddaf89c546f07d0aeb295920075d09 Mon Sep 17 00:00:00 2001 From: david-perez Date: Wed, 5 Jun 2024 18:08:02 +0200 Subject: [PATCH 19/24] ./gradlew ktlintFormat --- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 14 +++++++++----- .../core/smithy/generators/UnionGenerator.kt | 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index 967dabf586..0f1aab85e7 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -257,7 +257,10 @@ fun Project.registerModifyMtimeTask() { } } -fun Project.registerCargoCommandsTasks(outputDir: File, allowBrokenIntraDocLinks: Boolean = false) { +fun Project.registerCargoCommandsTasks( + outputDir: File, + allowBrokenIntraDocLinks: Boolean = false, +) { val dependentTasks = listOfNotNull( "assemble", @@ -280,10 +283,11 @@ fun Project.registerCargoCommandsTasks(outputDir: File, allowBrokenIntraDocLinks this.tasks.register(Cargo.DOCS.toString) { dependsOn(dependentTasks) workingDir(outputDir) - val args = mutableListOf( - "--no-deps", - "--document-private-items", - ) + val args = + mutableListOf( + "--no-deps", + "--document-private-items", + ) if (allowBrokenIntraDocLinks) { // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194#issuecomment-2147657902) // TODO(https://github.com/smithy-lang/smithy-rs/pull/3648) This is the _only_ reason why we need to allow diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt index 335edf4271..5ec29d8f17 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/UnionGenerator.kt @@ -201,7 +201,7 @@ private fun RustWriter.renderAsVariant( if (member.isTargetUnit()) { rust( "/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner `()`.", - unionSymbol + unionSymbol, ) rust("/// Returns `Err(&Self)` if it can't be converted.") rustBlockTemplate("pub fn as_$funcNamePart(&self) -> #{Result}<(), &Self>", *preludeScope) { From c35dc14595db6d9e95bd3652d3d2c0d8004f0515 Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 6 Jun 2024 12:09:14 +0200 Subject: [PATCH 20/24] allow broken intra doc links in server codegen integration tests --- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index 0f1aab85e7..2d9f5d317b 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -292,7 +292,8 @@ fun Project.registerCargoCommandsTasks( // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194#issuecomment-2147657902) // TODO(https://github.com/smithy-lang/smithy-rs/pull/3648) This is the _only_ reason why we need to allow // broken intra doc links. Once it lands we can remove this escape hatch and deny _all_ warnings. - args += "-A rustdoc::broken-intra-doc-links" + args += "--config" + args += """build.rustdocflags = ["-A", "rustdoc::broken-intra-doc-links"]""" } commandLine("cargo", "doc", *args.toTypedArray()) } From 26036340c611653f3719ef59984a29821057c3a0 Mon Sep 17 00:00:00 2001 From: david-perez Date: Thu, 6 Jun 2024 12:12:47 +0200 Subject: [PATCH 21/24] Formatting --- .../smithy/rust/codegen/core/rustlang/RustWriterTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt index 63fb4076a7..861288041e 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/RustWriterTest.kt @@ -104,10 +104,10 @@ class RustWriterTest { val output = normalizeHtml( """ - Link without href attribute -
Some text with [brackets]
- ] mismatched [ is escaped too - """, + Link without href attribute +
Some text with [brackets]
+ ] mismatched [ is escaped too + """, ) output shouldContain "Link without href attribute" output shouldContain "Some text with \\[brackets\\]" From 81df348916879f6e151cd187b9c222d7c44f8ecf Mon Sep 17 00:00:00 2001 From: david-perez Date: Fri, 21 Jun 2024 12:56:28 +0200 Subject: [PATCH 22/24] Remove escape hatch to tolerate broken intra doc links --- buildSrc/src/main/kotlin/CodegenTestCommon.kt | 12 +----------- codegen-server-test/build.gradle.kts | 2 +- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/buildSrc/src/main/kotlin/CodegenTestCommon.kt b/buildSrc/src/main/kotlin/CodegenTestCommon.kt index a005005d32..3c025288ac 100644 --- a/buildSrc/src/main/kotlin/CodegenTestCommon.kt +++ b/buildSrc/src/main/kotlin/CodegenTestCommon.kt @@ -258,10 +258,7 @@ fun Project.registerModifyMtimeTask() { } } -fun Project.registerCargoCommandsTasks( - outputDir: File, - allowBrokenIntraDocLinks: Boolean = false, -) { +fun Project.registerCargoCommandsTasks(outputDir: File) { val dependentTasks = listOfNotNull( "assemble", @@ -289,13 +286,6 @@ fun Project.registerCargoCommandsTasks( "--no-deps", "--document-private-items", ) - if (allowBrokenIntraDocLinks) { - // TODO(https://github.com/smithy-lang/smithy-rs/issues/3194#issuecomment-2147657902) - // TODO(https://github.com/smithy-lang/smithy-rs/pull/3648) This is the _only_ reason why we need to allow - // broken intra doc links. Once it lands we can remove this escape hatch and deny _all_ warnings. - args += "--config" - args += """build.rustdocflags = ["-A", "rustdoc::broken-intra-doc-links"]""" - } commandLine("cargo", "doc", *args.toTypedArray()) } diff --git a/codegen-server-test/build.gradle.kts b/codegen-server-test/build.gradle.kts index 7df4cd0de0..509b921877 100644 --- a/codegen-server-test/build.gradle.kts +++ b/codegen-server-test/build.gradle.kts @@ -111,7 +111,7 @@ tasks["smithyBuild"].dependsOn("generateSmithyBuild") tasks["assemble"].finalizedBy("generateCargoWorkspace", "generateCargoConfigToml") project.registerModifyMtimeTask() -project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile, allowBrokenIntraDocLinks = true) +project.registerCargoCommandsTasks(layout.buildDirectory.dir(workingDirUnderBuildDir).get().asFile) tasks["test"].finalizedBy(cargoCommands(properties).map { it.toString }) From 2c9d9f916ba97f38798cc557285aea4e24a9aa57 Mon Sep 17 00:00:00 2001 From: david-perez Date: Fri, 21 Jun 2024 15:18:26 +0200 Subject: [PATCH 23/24] Fix Python --- .../python/smithy/generators/PythonServerUnionGenerator.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt index ef5a043a50..f00a41cb8b 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt @@ -154,7 +154,7 @@ class PythonServerUnionGenerator( ) { if (member.isTargetUnit()) { writer.rust( - "/// Tries to convert the union instance into [`$variantName`].", + "/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner `()`.", ) writer.rust("/// :rtype None:") writer.rustBlockTemplate("pub fn as_$funcNamePart(&self) -> #{pyo3}::PyResult<()>", "pyo3" to pyo3) { From 99f73ec83266e8ccba960b4f2b8e24b0de3ee9db Mon Sep 17 00:00:00 2001 From: david-perez Date: Fri, 21 Jun 2024 15:29:19 +0200 Subject: [PATCH 24/24] Fix Python now for real --- .../python/smithy/generators/PythonServerUnionGenerator.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt index f00a41cb8b..227d38df48 100644 --- a/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt +++ b/codegen-server/python/src/main/kotlin/software/amazon/smithy/rust/codegen/server/python/smithy/generators/PythonServerUnionGenerator.kt @@ -155,6 +155,7 @@ class PythonServerUnionGenerator( if (member.isTargetUnit()) { writer.rust( "/// Tries to convert the enum instance into [`$variantName`](#T::$variantName), extracting the inner `()`.", + unionSymbol, ) writer.rust("/// :rtype None:") writer.rustBlockTemplate("pub fn as_$funcNamePart(&self) -> #{pyo3}::PyResult<()>", "pyo3" to pyo3) {