Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix several warnings and reduce Rust target clobbering #1069

Merged
merged 4 commits into from
Jan 14, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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#", "::")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there may be other places where we make doc links that need to be audited (but maybe those aren't dynamic?)

Nevermind, looks like you got em


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"