fix: strip trailing whitespace on codegenned new lines#4639
Merged
aws-sdk-rust-ci merged 2 commits intosmithy-lang:mainfrom May 1, 2026
Merged
fix: strip trailing whitespace on codegenned new lines#4639aws-sdk-rust-ci merged 2 commits intosmithy-lang:mainfrom
aws-sdk-rust-ci merged 2 commits intosmithy-lang:mainfrom
Conversation
rcoh
approved these changes
May 1, 2026
Member
Author
|
Here's the fix for the rustfmt-side-bug, though it will take time to release even if merged: rust-lang/rustfmt#6881 |
vcjana
approved these changes
May 1, 2026
Member
Author
|
The semver checks failure is expected:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Part of the solution for #4634
Generated Rust server code contains whitespace-only lines that cause
cargo fmtto fail witherror[internal]: left behind trailing whitespace. This reproduces on the stocksmithy-rust-quickstarttemplate with no modifications.Description
Smithy's
AbstractCodeWriterprepends the current indentation to every line it writes, including blank lines. When codegen templates produce empty lines (viajoin("\n")separators between writables, or empty string interpolations like$boxIt/$boxErr), the writer turns them into whitespace-only lines.This change strips trailing whitespace (spaces and tabs before a newline) from all lines in
RustWriter.toString()using a single regex replace. Blank lines are preserved; only the invisible indentation on them is removed. This is a global fix that catches all current and future instances rather than patching individual templates.The alternative would be to patch each template that produces blank lines (the
join("\n")separator inSmithyValidationExceptionDecorator, the$boxIt/$boxErrinterpolations inUnconstrainedUnionGenerator, etc.). That approach is fragile since any new template could reintroduce the problem. ThetoString()approach is a single chokepoint that all generated output passes through.The performance cost is negligible: the regex (
[ \t]+\n) is pre-compiled as acompanion objectval, and it runs a simple character class scan with no backtracking.toString()is called once per output file duringflushWriters()(plus once per inline module writer). In thecodegen-server-testsuite (~2500 generated.rsfiles, ~18MB total), this adds no measurable overhead compared to the model traversal, template rendering, andcargo fmtinvocation that dominate the build.This is a partial fix: it eliminates all trailing whitespace produced by codegen, which resolves the whitespace-only blank lines described in the issue. However, both reproducers in the issue also generate
pub(crate)tuple structs with long type paths (e.g. constrained map/collection wrappers), andrustfmtitself reintroduces trailing whitespace when wrapping those fields. That meanscargo fmtwill still fail on a freshly generated workspace for models that produce these types. That is an upstreamrustfmtbug (rust-lang/rustfmt#6880). The codegen pipeline already catches and logscargo fmtfailures, so those cases are non-blocking.Testing
Added a regression test in
RustWriterTestthat reproduces both patterns from the issue (join separator blank lines and empty string interpolation blank lines inside indented blocks) and asserts no trailing whitespace in the output. Fullcodegen-coretest suite passes.Verified end-to-end with a clean
codegen-server-test:assemblebuild (~2500 generated.rsfiles). All codegen-produced trailing whitespace is eliminated. The only remaining trailing whitespace in generated files comes fromrustfmtreintroducing it onpub(crate)tuple struct fields (the upstream bug).Benchmarked
codegen-server-test:clean assembleover 10 runs each: 25.2s +/- 0.6s with the fix vs 24.9s +/- 0.7s without. Within noise, no measurable performance impact.Checklist
.changelogdirectory, specifying "client," "server," or both in theapplies_tokey.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.