Commit a0d46cd
authored
fix: strip trailing whitespace on codegenned new lines (#4639)
## Motivation and Context
Part of the solution for
#4634
Generated Rust server code contains whitespace-only lines that cause
`cargo fmt` to fail with `error[internal]: left behind trailing
whitespace`. This reproduces on the stock `smithy-rust-quickstart`
template with no modifications.
## Description
Smithy's `AbstractCodeWriter` prepends the current indentation to every
line it writes, including blank lines. When codegen templates produce
empty lines (via `join("\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 in
`SmithyValidationExceptionDecorator`, the `$boxIt`/`$boxErr`
interpolations in `UnconstrainedUnionGenerator`, etc.). That approach is
fragile since any new template could reintroduce the problem. The
`toString()` approach is a single chokepoint that all generated output
passes through.
The performance cost is negligible: the regex (`[ \t]+\n`) is
pre-compiled as a `companion object` val, and it runs a simple character
class scan with no backtracking. `toString()` is called once per output
file during `flushWriters()` (plus once per inline module writer). In
the `codegen-server-test` suite (~2500 generated `.rs` files, ~18MB
total), this adds no measurable overhead compared to the model
traversal, template rendering, and `cargo fmt` invocation 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), and `rustfmt` itself reintroduces trailing whitespace when
wrapping those fields. That means `cargo fmt` will still fail on a
freshly generated workspace for models that produce these types. That is
an upstream `rustfmt` bug
([rust-lang/rustfmt#6880](rust-lang/rustfmt#6880)).
The codegen pipeline already catches and logs `cargo fmt` failures, so
those cases are non-blocking.
## Testing
Added a regression test in `RustWriterTest` that 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. Full `codegen-core` test suite
passes.
Verified end-to-end with a clean `codegen-server-test:assemble` build
(~2500 generated `.rs` files). All codegen-produced trailing whitespace
is eliminated. The only remaining trailing whitespace in generated files
comes from `rustfmt` reintroducing it on `pub(crate)` tuple struct
fields (the upstream bug).
Benchmarked `codegen-server-test:clean assemble` over 10 runs each:
25.2s +/- 0.6s with the fix vs 24.9s +/- 0.7s without. Within noise, no
measurable performance impact.
## Checklist
- [x] For changes to the smithy-rs codegen or runtime crates, I have
created a changelog entry Markdown file in the `.changelog` directory,
specifying "client," "server," or both in the `applies_to` key.
----
_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._1 parent 777c24b commit a0d46cd
3 files changed
Lines changed: 49 additions & 0 deletions
File tree
- .changelog
- codegen-core/src
- main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang
- test/kotlin/software/amazon/smithy/rust/codegen/core/rustlang
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
Lines changed: 3 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
598 | 598 | | |
599 | 599 | | |
600 | 600 | | |
| 601 | + | |
| 602 | + | |
601 | 603 | | |
602 | 604 | | |
603 | 605 | | |
| |||
925 | 927 | | |
926 | 928 | | |
927 | 929 | | |
| 930 | + | |
928 | 931 | | |
929 | 932 | | |
930 | 933 | | |
| |||
Lines changed: 33 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
228 | 228 | | |
229 | 229 | | |
230 | 230 | | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
231 | 264 | | |
0 commit comments