Skip to content

Commit

Permalink
Fix several warnings and reduce Rust target clobbering (#1069)
Browse files Browse the repository at this point in the history
  • Loading branch information
jdisanti authored Jan 14, 2022
1 parent 7f74b87 commit 4bbc677
Show file tree
Hide file tree
Showing 13 changed files with 78 additions and 40 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.next.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
15 changes: 7 additions & 8 deletions aws/sdk-codegen-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down Expand Up @@ -81,32 +83,29 @@ tasks["assemble"].finalizedBy("generateCargoWorkspace")

tasks.register<Exec>("cargoCheck") {
workingDir("build/smithyprojections/sdk-codegen-test/")
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "check")
dependsOn("assemble")
}

tasks.register<Exec>("cargoTest") {
workingDir("build/smithyprojections/sdk-codegen-test/")
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "test")
dependsOn("assemble")
}

tasks.register<Exec>("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<Exec>("cargoClippy") {
workingDir("build/smithyprojections/sdk-codegen-test/")
// disallow warnings
commandLine("cargo", "clippy", "--", "-D", "warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "clippy")
dependsOn("assemble")
}

Expand Down
15 changes: 7 additions & 8 deletions aws/sdk/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -288,32 +290,29 @@ tasks["assemble"].apply {

tasks.register<Exec>("cargoCheck") {
workingDir(outputDir)
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "check", "--lib", "--tests", "--benches")
dependsOn("assemble")
}

tasks.register<Exec>("cargoTest") {
workingDir(outputDir)
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "test")
dependsOn("assemble")
}

tasks.register<Exec>("cargoDocs") {
workingDir(outputDir)
// disallow warnings
environment("RUSTDOCFLAGS", "-D warnings")
environment("RUSTDOCFLAGS", defaultRustDocFlags)
commandLine("cargo", "doc", "--no-deps", "--document-private-items")
dependsOn("assemble")
}

tasks.register<Exec>("cargoClippy") {
workingDir(outputDir)
// disallow warnings
commandLine("cargo", "clippy", "--", "-D", "warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "clippy")
dependsOn("assemble")
}

Expand Down
15 changes: 7 additions & 8 deletions codegen-server-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -99,32 +101,29 @@ tasks["assemble"].finalizedBy("generateCargoWorkspace")

tasks.register<Exec>("cargoCheck") {
workingDir("build/smithyprojections/codegen-server-test/")
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "check")
dependsOn("assemble")
}

tasks.register<Exec>("cargoTest") {
workingDir("build/smithyprojections/codegen-server-test/")
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "test")
dependsOn("assemble")
}

tasks.register<Exec>("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<Exec>("cargoClippy") {
workingDir("build/smithyprojections/codegen-server-test/")
// disallow warnings
commandLine("cargo", "clippy", "--", "-D", "warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "clippy")
dependsOn("assemble")
}

Expand Down
15 changes: 7 additions & 8 deletions codegen-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -137,32 +139,29 @@ tasks["assemble"].finalizedBy("generateCargoWorkspace")

tasks.register<Exec>("cargoCheck") {
workingDir("build/smithyprojections/codegen-test/")
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "check")
dependsOn("assemble")
}

tasks.register<Exec>("cargoTest") {
workingDir("build/smithyprojections/codegen-test/")
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "test")
dependsOn("assemble")
}

tasks.register<Exec>("cargoDocs") {
workingDir("build/smithyprojections/codegen-test/")
// disallow warnings
environment("RUSTFLAGS", "-D warnings")
environment("RUSTDOCFLAGS", defaultRustDocFlags)
commandLine("cargo", "doc", "--no-deps")
dependsOn("assemble")
}

tasks.register<Exec>("cargoClippy") {
workingDir("build/smithyprojections/codegen-test/")
// disallow warnings
commandLine("cargo", "clippy", "--", "-D", "warnings")
environment("RUSTFLAGS", defaultRustFlags)
commandLine("cargo", "clippy")
dependsOn("assemble")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -467,7 +473,7 @@ class RustWriter private constructor(
inner class RustDocLinker : BiFunction<Any, String, String> {
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")
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -574,6 +575,6 @@ fun generateShapeMemberDocs(writer: RustWriter, symbolProvider: SymbolProvider,
else -> "(undocumented)"
}

"[`$name($member)`]($structName::$name): $docs"
"[`$name($member)`](${docLink("$structName::$name")}): $docs"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
9 changes: 9 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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=
4 changes: 4 additions & 0 deletions tools/.cargo/config.toml
Original file line number Diff line number Diff line change
@@ -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"

0 comments on commit 4bbc677

Please sign in to comment.