Repository navigation
refactor(codegen): give POST Object a single owner in codegen v2 - #993
Conversation
PostObject has no shape in the upstream Smithy model, so the v1 generator special-cases it in several modules. Introduce codegen/src/v2/post_object.rs as the module that owns the operation, starting with a descriptor for its name, its DTO types, the operation it forks and its placeholder route. The v1 pipeline still generates everything itself, so the output of `just codegen` is unchanged. Assisted-by: DeepSeek Harness:deepseek-flash
The oir lookup, the aws-sdk conversion and the ops test family each special-cased PostObject on their own. Read the name, the DTO type names and the emitted test pieces from codegen/src/v2/post_object.rs instead; the emission positions are unchanged, so the generated files are byte for byte the same. Assisted-by: DeepSeek Harness:deepseek-flash
…o v2 The DTO fork, the POST-only fields, the mapping helpers, the operation injection and the operation module emitter lived in the v1 DTO and ops passes. Move them into codegen/src/v2/post_object.rs; the v1 passes keep their execution order and call positions, so the generated files are byte for byte the same. The v1 items v2 reads are re-exported or made crate-visible, which is a visibility change only. Assisted-by: DeepSeek Harness:deepseek-flash
…iptor The remaining generators compared an operation name against a literal. Use the module predicate instead, so the name lives only in the descriptor; the predicate returns the same value, so the generated files are unchanged. Every descriptor field is read now, so its dead-code allowance is gone. Assisted-by: DeepSeek Harness:deepseek-flash
The S3 trait default bridge, the access-hook notes and the proxy skip still compared the operation name against a literal in v1. Emit the first two from the v2 module and query its predicate for the proxy skip, so the name, the DTO type names and the emitted text all live in one module. The emitted text is unchanged, including the compatibility-layer documentation added for the bridge. Assisted-by: DeepSeek Harness:deepseek-flash
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🔵 Needs a closer look
Correctness hinges on the generated output being byte-identical and the strict codegen/clippy gates passing, which cannot be recomputed in this environment, and it regenerates code for the core crate.
0 open findings
What changed in this PR
This PR refactors the s3s-codegen build tool so the synthetic PostObject operation (which has no shape in the upstream Smithy model) has a single owner. Previously it was special-cased at many sites across seven v1 modules; now codegen/src/v2/post_object.rs holds a SyntheticOp descriptor plus one emitter function per generation concern, while the v1 modules keep the generation wiring and simply call into it. It is a structural/maintainability change with no intended effect on generated output — the artifacts are meant to stay byte-identical.
Changes:
- Add
codegen/src/v2/post_object.rswithSyntheticOp/descriptor()and emitter functions (inject_operation,codegen_fork_op,codegen_dto_clone,codegen_post_only_fields,codegen_trait_default,codegen_access_hook,codegen_mapping_helpers, test emitters). - Replace the seven literal
op.name == "PostObject"checks acrossv1withpost_object::is_synthetic(...)/is_synthetic_type(...), and remove the inlined emitter bodies fromops.rs,dto.rs,s3_trait.rs,access.rs,gen_tests/ops.rs. - Widen visibility (
pub(crate)) and re-export thev1items (rust,write_dir_file,OPS_GENERATED_DIR,codegen_file_header,RustTypes,Operation,Operations) that the new module consumes.
| File | Description |
|---|---|
| codegen/src/v2/post_object.rs | New single owner: descriptor + emitter functions for the synthetic operation |
| codegen/src/v2/mod.rs | Declares pub(crate) mod post_object |
| codegen/src/v1/mod.rs | Visibility change: pub(crate) mod rust, write_dir_file, and re-exports for v2 consumption |
| codegen/src/v1/ops.rs | Removes inlined fork-op/injection; delegates to post_object::*; pub(crate) on shared items |
| codegen/src/v1/dto.rs | Delegates DTO clone, POST-only fields, and mapping helpers to post_object::* |
| codegen/src/v1/s3_trait.rs | Default bridge emission moved to post_object::codegen_trait_default |
| codegen/src/v1/access.rs | Access-hook note emission moved to post_object::codegen_access_hook |
| codegen/src/v1/gen_tests/ops.rs | Test fixture/generated test moved to post_object::*; POST_OBJECT const removed |
| codegen/src/v1/oir.rs | Literal check replaced with post_object::is_synthetic |
| codegen/src/v1/aws_proxy.rs | Literal check replaced with post_object::is_synthetic |
| codegen/src/v1/aws_conv.rs | Match arms replaced with post_object::is_synthetic_type guard |
Verification performed: all moved emitter strings and the v1 emission call order match the originals; inject_operation is equivalent to the old assert-insert block; every SyntheticOp field and every new function is referenced (no dead code under -D warnings); the visibility/re-export chain resolves for crate::v2::post_object; and no references to the removed symbols (codegen_post_object_*, POST_OBJECT, literal op.name == "PostObject") remain.
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
POST Objectexists only in the generator, and it used to be special-cased at 75 sites across seven modules, so every change to the operation meant changing the whole generator. This gives it one owner:codegen/src/v2/post_object.rsholds a descriptor and one function per generation concern, and v1 keeps the wiring. The generated artifacts are byte-identical — 11 files change and 0 lines of generated output do.Changes
SyntheticOpwithdescriptor()) as the single place that states the operation's facts: its name, its input and output types, its placeholder Smithy shapes and its fork types.op.name == "PostObject") withv2::post_object::is_synthetic(&op.name), so a module asks what an operation is instead of naming it;codegen/srcno longer mentions the operation name in a condition.Behavior
None for users of the crates: the generator emits byte-identical files, which the identity check below enforces. The change matters for the next step of the issue, where the operation stops being synthetic — that edit now lands in one module instead of seven.
Tests
just codegentwice → exit 0 and 0, with 0 dirty files after each run.git diff --exit-code -- crates/s3s crates/s3s-aws→ exit 0: the generated output is byte-identical, including the baseline's comment wording.cargo test -p s3s --all-features --lib→ exit 0, 1031 passed, 0 failed, 5 ignored (on the branch rebased ontomainafter the wire baseline merged).cargo test -p s3s --all-features --test post_object_wire→ 12 passed, and the default feature set → 12 passed: the baseline suite that pins the operation's wire behaviour.cargo checkandcargo check --all-features→ exit 0;cargo clippy -p s3s-codegen --all-targets -- -D warnings→ exit 0.cargo fmt --all -- --check→ exit 0;cargo +nightly clippy --workspace --all-targets --all-features -- -D warnings→ exit 0.References
Part of #982.
Dependencies
The branch is based on
mainafter the wire baseline merged (#991); that baseline's suite is what shows the generated output is unchanged. The next change in the stack (moving the parsing into a hand-written layer) builds on this branch. No dependency or manifest changes.AI
Written with DeepSeek Harness (
deepseek-flash): it moved the emitters, rewrote the seven literal checks, kept the generated output byte-identical and ran the gates and the instrument control above. A human reviewed the diff and owns the change.