Skip to content

Fix redundant closure warnings in Rust 1.82 #4125

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
3 changes: 1 addition & 2 deletions buildSrc/src/main/kotlin/CodegenTestCommon.kt
Original file line number Diff line number Diff line change
Expand Up @@ -286,12 +286,11 @@ fun Project.registerGenerateCargoConfigTomlTask(outputDir: File) {
// is completed, warnings can be prohibited in rustdoc by setting `rustdocflags` to `-D warnings`.
doFirst {
outputDir.resolve(".cargo").mkdirs()
// TODO(MSRV1.82 follow-up): Restore `"--deny", "warnings"` once lints are fixed in the server runtime crates
outputDir.resolve(".cargo/config.toml")
.writeText(
"""
[build]
rustflags = ["--cfg", "aws_sdk_unstable"]
rustflags = ["--deny", "warnings", "--cfg", "aws_sdk_unstable"]
""".trimIndent(),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,25 @@ class ServerBuilderGenerator(
if (!constrainedTypeHoldsFinalType(member)) varExpr = "($varExpr).into()"

if (wrapInMaybeConstrained) {
conditionalBlock("input.map(##[allow(clippy::redundant_closure)] |v| ", ")", conditional = symbol.isOptional()) {
conditionalBlock("Box::new(", ")", conditional = hasBox) {
rust("$maybeConstrainedVariant($varExpr)")
// TODO(https://github.com/rust-lang/rust-clippy/issues/14789):
// Temporary fix for `#[allow(clippy::redundant_closure)]` not working in `rustc` version 1.82.
// The issue occurs specifically when we generate code in the form:
// ```rust
// input.map(|v| MaybeConstrained(v))
// ```
// This pattern triggers a clippy warning about redundant closures, even with the allow attribute.
// For this specific pattern, we directly use the constructor function reference:
// ```rust
// input.map(MaybeConstrained)
// ```
// Other cases like `Box::new(v)` or `v.into()` are valid closure usages and remain unchanged.
if (symbol.isOptional() && varExpr == "v") {
rust("input.map($maybeConstrainedVariant)")
} else {
conditionalBlock("input.map(##[allow(clippy::redundant_closure)] |v| ", ")", conditional = symbol.isOptional()) {
conditionalBlock("Box::new(", ")", conditional = hasBox) {
rust("$maybeConstrainedVariant($varExpr)")
}
}
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import software.amazon.smithy.model.shapes.LongShape
import software.amazon.smithy.model.shapes.MapShape
import software.amazon.smithy.model.shapes.MemberShape
import software.amazon.smithy.model.shapes.NumberShape
import software.amazon.smithy.model.shapes.Shape
import software.amazon.smithy.model.shapes.ShortShape
import software.amazon.smithy.model.shapes.StringShape
import software.amazon.smithy.model.shapes.TimestampShape
Expand Down Expand Up @@ -84,8 +85,23 @@ fun generateFallbackCodeToDefaultValue(
symbolProvider: RustSymbolProvider,
publicConstrainedTypes: Boolean,
) {
var defaultValue = defaultValue(model, runtimeConfig, symbolProvider, member)
val targetShape = model.expectShape(member.target)
// TODO(https://github.com/rust-lang/rust-clippy/issues/14789):
// Temporary fix for `#[allow(clippy::redundant_closure)]` not working in `rustc` version 1.82.
// The issue occurs specifically when we generate code in the form:
// ```rust
// .unwrap_or_else(HashMap::new())
// ```
// Instead of the linter suggested code:
// ```rust
// .unwrap_or_default()
// ```
if (isTargetListOrMap(targetShape, member)) {
writer.rustTemplate(".unwrap_or_default()")
return
}

var defaultValue = defaultValue(model, runtimeConfig, symbolProvider, member)
val targetSymbol = symbolProvider.toSymbol(targetShape)
// We need an .into() conversion to create defaults for the server types. A larger scale refactoring could store this information in the
// symbol, however, retrieving it in this manner works for the moment.
Expand Down Expand Up @@ -125,6 +141,22 @@ fun generateFallbackCodeToDefaultValue(
}
}

private fun isTargetListOrMap(
targetShape: Shape?,
member: MemberShape,
): Boolean {
if (targetShape is ListShape) {
val node = member.expectTrait<DefaultTrait>().toNode()!!
check(node is ArrayNode && node.isEmpty)
return true
} else if (targetShape is MapShape) {
val node = member.expectTrait<DefaultTrait>().toNode()!!
check(node is ObjectNode && node.isEmpty)
return true
}
return false
}

/**
* Returns a writable to construct a Rust value of the correct type holding the modeled `@default` value on the
* [member] shape.
Expand Down
5 changes: 2 additions & 3 deletions tools/ci-build/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,8 @@ ENV PATH=/opt/cargo/bin:$PATH \
RUST_STABLE_VERSION=${rust_stable_version} \
RUST_NIGHTLY_VERSION=${rust_nightly_version} \
CARGO_INCREMENTAL=0 \
# TODO(MSRV1.82 follow-up): Restore them once lints are fixed in the server runtime crates
# RUSTDOCFLAGS="-D warnings" \
# RUSTFLAGS="-D warnings" \
RUSTDOCFLAGS="-D warnings" \
RUSTFLAGS="-D warnings" \
LANG=en_US.UTF-8 \
LC_ALL=en_US.UTF-8
# SMITHY_RS_DOCKER_BUILD_IMAGE indicates to build scripts that they are being built inside of the Docker build image.
Expand Down
4 changes: 1 addition & 3 deletions tools/ci-scripts/check-rust-runtimes
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ do

echo -e "## ${C_YELLOW}Running 'cargo doc' on ${runtime_path}...${C_RESET}"

# TODO(MSRV1.82 follow-up): Restore `-Dwarnings` once lints are fixed in aws-smithy-http-server-python:
# "error: unexpected `cfg` condition name: `addr_of`"
RUSTDOCFLAGS="--cfg docsrs" cargo +"${RUST_NIGHTLY_VERSION}" doc --no-deps --document-private-items --all-features
RUSTDOCFLAGS="--cfg docsrs -Dwarnings" cargo +"${RUST_NIGHTLY_VERSION}" doc --no-deps --document-private-items --all-features

echo -e "## ${C_YELLOW}Running 'cargo minimal-versions check' on ${runtime_path}...${C_RESET}"
# Print out the cargo tree with minimal versions for easier debugging
Expand Down
Loading