Skip to content

Commit 045dcb1

Browse files
author
Fahad Zubair
committed
Generate server code compliant with the linter
1 parent ad97759 commit 045dcb1

File tree

5 files changed

+56
-12
lines changed

5 files changed

+56
-12
lines changed

buildSrc/src/main/kotlin/CodegenTestCommon.kt

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,11 @@ fun Project.registerGenerateCargoConfigTomlTask(outputDir: File) {
286286
// is completed, warnings can be prohibited in rustdoc by setting `rustdocflags` to `-D warnings`.
287287
doFirst {
288288
outputDir.resolve(".cargo").mkdirs()
289-
// TODO(MSRV1.82 follow-up): Restore `"--deny", "warnings"` once lints are fixed in the server runtime crates
290289
outputDir.resolve(".cargo/config.toml")
291290
.writeText(
292291
"""
293292
[build]
294-
rustflags = ["--cfg", "aws_sdk_unstable"]
293+
rustflags = ["--deny", "warnings", "--cfg", "aws_sdk_unstable"]
295294
""".trimIndent(),
296295
)
297296
}

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGenerator.kt

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -351,9 +351,25 @@ class ServerBuilderGenerator(
351351
if (!constrainedTypeHoldsFinalType(member)) varExpr = "($varExpr).into()"
352352

353353
if (wrapInMaybeConstrained) {
354-
conditionalBlock("input.map(##[allow(clippy::redundant_closure)] |v| ", ")", conditional = symbol.isOptional()) {
355-
conditionalBlock("Box::new(", ")", conditional = hasBox) {
356-
rust("$maybeConstrainedVariant($varExpr)")
354+
// TODO(https://github.com/rust-lang/rust-clippy/issues/14789):
355+
// Temporary fix for `#[allow(clippy::redundant_closure)]` not working in `rustc` version 1.82.
356+
// The issue occurs specifically when we generate code in the form:
357+
// ```rust
358+
// input.map(|v| MaybeConstrained(v))
359+
// ```
360+
// This pattern triggers a clippy warning about redundant closures, even with the allow attribute.
361+
// For this specific pattern, we directly use the constructor function reference:
362+
// ```rust
363+
// input.map(MaybeConstrained)
364+
// ```
365+
// Other cases like `Box::new(v)` or `v.into()` are valid closure usages and remain unchanged.
366+
if (symbol.isOptional() && varExpr == "v") {
367+
rust("input.map($maybeConstrainedVariant)")
368+
} else {
369+
conditionalBlock("input.map(##[allow(clippy::redundant_closure)] |v| ", ")", conditional = symbol.isOptional()) {
370+
conditionalBlock("Box::new(", ")", conditional = hasBox) {
371+
rust("$maybeConstrainedVariant($varExpr)")
372+
}
357373
}
358374
}
359375
} else {

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/generators/ServerBuilderGeneratorCommon.kt

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import software.amazon.smithy.model.shapes.LongShape
2727
import software.amazon.smithy.model.shapes.MapShape
2828
import software.amazon.smithy.model.shapes.MemberShape
2929
import software.amazon.smithy.model.shapes.NumberShape
30+
import software.amazon.smithy.model.shapes.Shape
3031
import software.amazon.smithy.model.shapes.ShortShape
3132
import software.amazon.smithy.model.shapes.StringShape
3233
import software.amazon.smithy.model.shapes.TimestampShape
@@ -84,8 +85,23 @@ fun generateFallbackCodeToDefaultValue(
8485
symbolProvider: RustSymbolProvider,
8586
publicConstrainedTypes: Boolean,
8687
) {
87-
var defaultValue = defaultValue(model, runtimeConfig, symbolProvider, member)
8888
val targetShape = model.expectShape(member.target)
89+
// TODO(https://github.com/rust-lang/rust-clippy/issues/14789):
90+
// Temporary fix for `#[allow(clippy::redundant_closure)]` not working in `rustc` version 1.82.
91+
// The issue occurs specifically when we generate code in the form:
92+
// ```rust
93+
// .unwrap_or_else(HashMap::new())
94+
// ```
95+
// Instead of the linter suggested code:
96+
// ```rust
97+
// .unwrap_or_default()
98+
// ```
99+
if (isTargetListOrMap(targetShape, member)) {
100+
writer.rustTemplate(".unwrap_or_default()")
101+
return
102+
}
103+
104+
var defaultValue = defaultValue(model, runtimeConfig, symbolProvider, member)
89105
val targetSymbol = symbolProvider.toSymbol(targetShape)
90106
// We need an .into() conversion to create defaults for the server types. A larger scale refactoring could store this information in the
91107
// symbol, however, retrieving it in this manner works for the moment.
@@ -125,6 +141,22 @@ fun generateFallbackCodeToDefaultValue(
125141
}
126142
}
127143

144+
private fun isTargetListOrMap(
145+
targetShape: Shape?,
146+
member: MemberShape,
147+
): Boolean {
148+
if (targetShape is ListShape) {
149+
val node = member.expectTrait<DefaultTrait>().toNode()!!
150+
check(node is ArrayNode && node.isEmpty)
151+
return true
152+
} else if (targetShape is MapShape) {
153+
val node = member.expectTrait<DefaultTrait>().toNode()!!
154+
check(node is ObjectNode && node.isEmpty)
155+
return true
156+
}
157+
return false
158+
}
159+
128160
/**
129161
* Returns a writable to construct a Rust value of the correct type holding the modeled `@default` value on the
130162
* [member] shape.

tools/ci-build/Dockerfile

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -206,9 +206,8 @@ ENV PATH=/opt/cargo/bin:$PATH \
206206
RUST_STABLE_VERSION=${rust_stable_version} \
207207
RUST_NIGHTLY_VERSION=${rust_nightly_version} \
208208
CARGO_INCREMENTAL=0 \
209-
# TODO(MSRV1.82 follow-up): Restore them once lints are fixed in the server runtime crates
210-
# RUSTDOCFLAGS="-D warnings" \
211-
# RUSTFLAGS="-D warnings" \
209+
RUSTDOCFLAGS="-D warnings" \
210+
RUSTFLAGS="-D warnings" \
212211
LANG=en_US.UTF-8 \
213212
LC_ALL=en_US.UTF-8
214213
# SMITHY_RS_DOCKER_BUILD_IMAGE indicates to build scripts that they are being built inside of the Docker build image.

tools/ci-scripts/check-rust-runtimes

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ do
3131

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

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

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

0 commit comments

Comments
 (0)