From 4bbc677c5ad16a4900058c659abfa696d4b818ca Mon Sep 17 00:00:00 2001 From: John DiSanti Date: Thu, 13 Jan 2022 16:45:04 -0800 Subject: [PATCH] Fix several warnings and reduce Rust target clobbering (#1069) --- CHANGELOG.next.toml | 18 ++++++++++++++++++ aws/sdk-codegen-test/build.gradle.kts | 15 +++++++-------- aws/sdk/build.gradle.kts | 15 +++++++-------- codegen-server-test/build.gradle.kts | 15 +++++++-------- codegen-test/build.gradle.kts | 15 +++++++-------- .../smithy/rust/codegen/rustlang/RustWriter.kt | 8 +++++++- .../customizations/ClientDocsGenerator.kt | 2 +- .../customizations/RetryConfigDecorator.kt | 4 +++- .../smithy/generators/FluentClientDecorator.kt | 5 +++-- .../generators/protocol/ProtocolGenerator.kt | 3 ++- .../smithy/rust/codegen/testutil/Rust.kt | 5 +++-- gradle.properties | 9 +++++++++ tools/.cargo/config.toml | 4 ++++ 13 files changed, 78 insertions(+), 40 deletions(-) create mode 100644 tools/.cargo/config.toml diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index 9aef618b1e..257c4bb74a 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -28,3 +28,21 @@ message = "Upgraded Smithy to 1.16.1" references = ["smithy-rs#1053"] meta = { "breaking" = false, "tada" = false, "bug" = false } author = "jdisanti" + +[[smithy-rs]] +message = "Fix broken link to `RetryMode` in client docs" +references = ["smithy-rs#1069"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "jdisanti" + +[[smithy-rs]] +message = "Fix several doc links to raw identifiers (identifiers excaped with `r#`)" +references = ["smithy-rs#1069"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "jdisanti" + +[[smithy-rs]] +message = "Reduce dependency recompilation in local dev" +references = ["smithy-rs#1069"] +meta = { "breaking" = false, "tada" = false, "bug" = true } +author = "jdisanti" diff --git a/aws/sdk-codegen-test/build.gradle.kts b/aws/sdk-codegen-test/build.gradle.kts index eb5ec54239..751f2555bb 100644 --- a/aws/sdk-codegen-test/build.gradle.kts +++ b/aws/sdk-codegen-test/build.gradle.kts @@ -13,6 +13,8 @@ plugins { } val smithyVersion: String by project +val defaultRustFlags: String by project +val defaultRustDocFlags: String by project dependencies { implementation(project(":aws:sdk-codegen")) @@ -81,32 +83,29 @@ tasks["assemble"].finalizedBy("generateCargoWorkspace") tasks.register("cargoCheck") { workingDir("build/smithyprojections/sdk-codegen-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "check") dependsOn("assemble") } tasks.register("cargoTest") { workingDir("build/smithyprojections/sdk-codegen-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "test") dependsOn("assemble") } tasks.register("cargoDocs") { workingDir("build/smithyprojections/sdk-codegen-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTDOCFLAGS", defaultRustDocFlags) commandLine("cargo", "doc", "--no-deps") dependsOn("assemble") } tasks.register("cargoClippy") { workingDir("build/smithyprojections/sdk-codegen-test/") - // disallow warnings - commandLine("cargo", "clippy", "--", "-D", "warnings") + environment("RUSTFLAGS", defaultRustFlags) + commandLine("cargo", "clippy") dependsOn("assemble") } diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 8231e89f94..9a501487b9 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -19,6 +19,8 @@ plugins { } val smithyVersion: String by project +val defaultRustFlags: String by project +val defaultRustDocFlags: String by project val properties = PropertyRetriever(rootProject, project) val outputDir = buildDir.resolve("aws-sdk") @@ -288,32 +290,29 @@ tasks["assemble"].apply { tasks.register("cargoCheck") { workingDir(outputDir) - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "check", "--lib", "--tests", "--benches") dependsOn("assemble") } tasks.register("cargoTest") { workingDir(outputDir) - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "test") dependsOn("assemble") } tasks.register("cargoDocs") { workingDir(outputDir) - // disallow warnings - environment("RUSTDOCFLAGS", "-D warnings") + environment("RUSTDOCFLAGS", defaultRustDocFlags) commandLine("cargo", "doc", "--no-deps", "--document-private-items") dependsOn("assemble") } tasks.register("cargoClippy") { workingDir(outputDir) - // disallow warnings - commandLine("cargo", "clippy", "--", "-D", "warnings") + environment("RUSTFLAGS", defaultRustFlags) + commandLine("cargo", "clippy") dependsOn("assemble") } diff --git a/codegen-server-test/build.gradle.kts b/codegen-server-test/build.gradle.kts index 858be7715c..e323b1182d 100644 --- a/codegen-server-test/build.gradle.kts +++ b/codegen-server-test/build.gradle.kts @@ -12,6 +12,8 @@ tasks["jar"].enabled = false plugins { id("software.amazon.smithy").version("0.5.3") } val smithyVersion: String by project +val defaultRustFlags: String by project +val defaultRustDocFlags: String by project buildscript { val smithyVersion: String by project @@ -99,32 +101,29 @@ tasks["assemble"].finalizedBy("generateCargoWorkspace") tasks.register("cargoCheck") { workingDir("build/smithyprojections/codegen-server-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "check") dependsOn("assemble") } tasks.register("cargoTest") { workingDir("build/smithyprojections/codegen-server-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "test") dependsOn("assemble") } tasks.register("cargoDocs") { workingDir("build/smithyprojections/codegen-server-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTDOCFLAGS", defaultRustDocFlags) commandLine("cargo", "doc", "--no-deps") dependsOn("assemble") } tasks.register("cargoClippy") { workingDir("build/smithyprojections/codegen-server-test/") - // disallow warnings - commandLine("cargo", "clippy", "--", "-D", "warnings") + environment("RUSTFLAGS", defaultRustFlags) + commandLine("cargo", "clippy") dependsOn("assemble") } diff --git a/codegen-test/build.gradle.kts b/codegen-test/build.gradle.kts index 857d1f5438..74944ebf97 100644 --- a/codegen-test/build.gradle.kts +++ b/codegen-test/build.gradle.kts @@ -13,6 +13,8 @@ plugins { } val smithyVersion: String by project +val defaultRustFlags: String by project +val defaultRustDocFlags: String by project buildscript { val smithyVersion: String by project @@ -137,32 +139,29 @@ tasks["assemble"].finalizedBy("generateCargoWorkspace") tasks.register("cargoCheck") { workingDir("build/smithyprojections/codegen-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "check") dependsOn("assemble") } tasks.register("cargoTest") { workingDir("build/smithyprojections/codegen-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTFLAGS", defaultRustFlags) commandLine("cargo", "test") dependsOn("assemble") } tasks.register("cargoDocs") { workingDir("build/smithyprojections/codegen-test/") - // disallow warnings - environment("RUSTFLAGS", "-D warnings") + environment("RUSTDOCFLAGS", defaultRustDocFlags) commandLine("cargo", "doc", "--no-deps") dependsOn("assemble") } tasks.register("cargoClippy") { workingDir("build/smithyprojections/codegen-test/") - // disallow warnings - commandLine("cargo", "clippy", "--", "-D", "warnings") + environment("RUSTFLAGS", defaultRustFlags) + commandLine("cargo", "clippy") dependsOn("assemble") } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustWriter.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustWriter.kt index 78dca23ba3..0b310cf58c 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustWriter.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/rustlang/RustWriter.kt @@ -305,6 +305,12 @@ fun Writable.isEmpty(): Boolean { return writer.toString() == RustWriter.root().toString() } +/** + * Rustdoc doesn't support `r#` for raw identifiers. + * This function adjusts doc links to refer to raw identifiers directly. + */ +fun docLink(docLink: String): String = docLink.replace("::r##", "::").replace("::r#", "::") + class RustWriter private constructor( private val filename: String, val namespace: String, @@ -467,7 +473,7 @@ class RustWriter private constructor( inner class RustDocLinker : BiFunction { override fun apply(t: Any, u: String): String { return when (t) { - is Symbol -> "[`${t.name}`](${t.rustType().qualifiedName()})" + is Symbol -> "[`${t.name}`](${docLink(t.rustType().qualifiedName())})" else -> throw CodegenException("Invalid type provided to RustDocLinker ($t) expected Symbol") } } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/ClientDocsGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/ClientDocsGenerator.kt index dc90a27438..bf4a348ab6 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/ClientDocsGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/ClientDocsGenerator.kt @@ -28,7 +28,7 @@ private fun crateLayout(): Writable = writable { The entry point for most customers will be [`Client`]. [`Client`] exposes one method for each API offered by the service. - Some APIs require complex or nested arguments. These exist in [`model`]. + Some APIs require complex or nested arguments. These exist in [`model`](crate::model). Lastly, errors that can be returned by the service are contained within [`error`]. [`Error`] defines a meta error encompassing all possible errors that can be returned by the service. diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt index 6f9026bfe8..308b950775 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/customizations/RetryConfigDecorator.kt @@ -150,7 +150,9 @@ class RetryConfigProviderConfig(codegenContext: CodegenContext) : ConfigCustomiz class PubUseRetryConfig(private val runtimeConfig: RuntimeConfig) : LibRsCustomization() { override fun section(section: LibRsSection): Writable { return when (section) { - is LibRsSection.Body -> writable { rust("pub use #T::RetryConfig;", smithyTypesRetry(runtimeConfig)) } + is LibRsSection.Body -> writable { + rust("pub use #T::RetryConfig;", smithyTypesRetry(runtimeConfig)) + } else -> emptySection } } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt index 275dd45846..cea0dc7d9f 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/FluentClientDecorator.kt @@ -23,6 +23,7 @@ import software.amazon.smithy.rust.codegen.rustlang.Writable import software.amazon.smithy.rust.codegen.rustlang.asArgument import software.amazon.smithy.rust.codegen.rustlang.asOptional import software.amazon.smithy.rust.codegen.rustlang.asType +import software.amazon.smithy.rust.codegen.rustlang.docLink import software.amazon.smithy.rust.codegen.rustlang.docs import software.amazon.smithy.rust.codegen.rustlang.documentShape import software.amazon.smithy.rust.codegen.rustlang.escape @@ -177,7 +178,7 @@ class GenericFluentClient(codegenContext: CodegenContext) : FluentClientCustomiz /// - A retry policy (`R`) that dictates the behavior for requests that /// fail and should (potentially) be retried. The default type is /// generally what you want, as it implements a well-vetted retry - /// policy implemented in [`retry::Standard`](crate::retry::Standard). + /// policy implemented in [`RetryMode::Standard`](aws_smithy_types::retry::RetryMode::Standard). /// /// To construct a client, you will generally want to call /// [`Client::with_config`], which takes a [`#{client}::Client`] (a @@ -574,6 +575,6 @@ fun generateShapeMemberDocs(writer: RustWriter, symbolProvider: SymbolProvider, else -> "(undocumented)" } - "[`$name($member)`]($structName::$name): $docs" + "[`$name($member)`](${docLink("$structName::$name")}): $docs" } } diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/protocol/ProtocolGenerator.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/protocol/ProtocolGenerator.kt index 920c6d69a9..787e78a2bc 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/protocol/ProtocolGenerator.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/smithy/generators/protocol/ProtocolGenerator.kt @@ -11,6 +11,7 @@ import software.amazon.smithy.rust.codegen.rustlang.Attribute import software.amazon.smithy.rust.codegen.rustlang.CargoDependency import software.amazon.smithy.rust.codegen.rustlang.RustWriter import software.amazon.smithy.rust.codegen.rustlang.asType +import software.amazon.smithy.rust.codegen.rustlang.docLink import software.amazon.smithy.rust.codegen.rustlang.rust import software.amazon.smithy.rust.codegen.rustlang.rustBlock import software.amazon.smithy.rust.codegen.rustlang.rustBlockTemplate @@ -170,7 +171,7 @@ open class ProtocolGenerator( /// Operation shape for `$operationName`. /// /// This is usually constructed for you using the the fluent builder returned by - /// [`$fluentBuilderName`](crate::client::Client::$fluentBuilderName). + /// [`$fluentBuilderName`](${docLink("crate::client::Client::$fluentBuilderName")}). /// /// See [`crate::client::fluent_builders::$operationName`] for more details about the operation. """ diff --git a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt index 9523ff76b7..fc4c1a36fd 100644 --- a/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt +++ b/codegen/src/main/kotlin/software/amazon/smithy/rust/codegen/testutil/Rust.kt @@ -182,9 +182,10 @@ fun TestWriterDelegator.compileAndTest(runClippy: Boolean = false) { } catch (e: Exception) { // cargo fmt errors are useless, ignore } - "cargo test".runCommand(baseDir, mapOf("RUSTFLAGS" to "-A dead_code")) + val env = mapOf("RUSTFLAGS" to "-A dead_code") + "cargo test".runCommand(baseDir, env) if (runClippy) { - "cargo clippy".runCommand(baseDir) + "cargo clippy".runCommand(baseDir, env) } } diff --git a/gradle.properties b/gradle.properties index dbfac21b46..fab36db761 100644 --- a/gradle.properties +++ b/gradle.properties @@ -21,3 +21,12 @@ kotlinVersion=1.4.21 # testing/utility ktlintVersion=0.40.0 kotestVersion=4.2.6 + +# rustc +defaultRustFlags=-D warnings + +# TODO(https://github.com/awslabs/smithy-rs/issues/1068): Once doc normalization +# is completed, warnings can be prohibited in rustdoc. +# +# defaultRustDocFlags=-D warnings +defaultRustDocFlags= diff --git a/tools/.cargo/config.toml b/tools/.cargo/config.toml new file mode 100644 index 0000000000..2bf2a2e3e4 --- /dev/null +++ b/tools/.cargo/config.toml @@ -0,0 +1,4 @@ +[build] +# Tools shouldn't share `target` with the rest of the project to +# avoid thrash from differing compiler flags +target-dir = "target"