Skip to content

codegen, text: remove vestigial Option/Result wraps and redundant clones#123

Draft
iainmcgin wants to merge 1 commit into
mainfrom
codegen-unnecessary-wraps
Draft

codegen, text: remove vestigial Option/Result wraps and redundant clones#123
iainmcgin wants to merge 1 commit into
mainfrom
codegen-unnecessary-wraps

Conversation

@iainmcgin
Copy link
Copy Markdown
Collaborator

Follow-up to #121: addresses the remaining actionable findings from the code-style audit — clippy::unnecessary_wraps (4 sites) and clippy::redundant_clone (2 sites), both nursery lints.

Changes

buffa-codegen/src/message.rs::singular_skip_predicate — returned Option<&'static str> but every match arm was Some(...). Now returns &'static str; the caller skip_serializing_predicate wraps it for the one branch where the value is conditional on field shape (the other branches already produce their own None/Some). A doc comment records that singular fields always have a default-value predicate.

buffa-codegen/src/view.rs::repeated_to_owned — returned Result<TokenStream, CodeGenError> but the body was a single infallible match. The signature implied a failure mode it didn't have, and the caller's ? was propagating an error that could never occur. Now returns TokenStream directly, which makes the asymmetry between it and the genuinely-fallible sibling singular_to_owned honest.

buffa/src/text/token.rs::emit / ::emit_eof — returned Result<Token<'a>, ParseError> purely so the FSM's return self.emit(...) tail calls didn't need an Ok(...) wrap. The 12 call sites now wrap explicitly, which makes the success/failure partition of parse_next visible at every return: Ok(self.emit(...)) always succeeds, Err(self.err_here(...)) always fails, self.parse_*() delegates. Before, the success path's Result typing was implicit while the error path's was explicit.

buffa-codegen/src/message.rs::compute_field_info — two bytes_type.clone() calls in mutually-exclusive if/else if branches alongside a third branch that already moved the value. Both clones removed; each execution path moves bytes_type at most once.

Notes

No change to generated output — task gen-wkt-types produced no drift. task lint and task test both pass. Net 47/-45.

Three classes of `clippy::unnecessary_wraps` and `clippy::redundant_clone`
findings (nursery group) in hand-written sources:

- `singular_skip_predicate` returned `Option<&'static str>` but every
  arm was `Some`. It now returns `&'static str` and the caller wraps the
  one branch where the value is conditional on field shape. A doc
  comment records the invariant.

- `repeated_to_owned` returned `Result<TokenStream, CodeGenError>` but
  the body was a single infallible `match`. The vestigial `Result`
  meant the caller's `?` was propagating an error that could never
  occur, and the signature implied a failure mode it didn't have. It
  now returns `TokenStream` directly, mirroring the asymmetry between
  it and the genuinely-fallible `singular_to_owned`.

- The tokenizer's `emit` and `emit_eof` helpers returned
  `Result<Token<'a>, ParseError>` purely so the FSM's `return self.emit(...)`
  tail calls didn't need an `Ok(...)` wrap. The 12 call sites now wrap
  explicitly, which makes the success/failure partition of `parse_next`
  visible at every return: `Ok(self.emit(...))` always succeeds,
  `Err(self.err_here(...))` always fails, `self.parse_*()` delegates.

- Two `bytes_type.clone()` calls in `compute_field_info` sat in
  mutually-exclusive `if`/`else if` branches alongside a third branch
  that already moved the value. Both clones were redundant; each
  execution path moves `bytes_type` at most once.

No change to generated output. `task lint`, `task test`, and
`task gen-wkt-types` all pass with no drift.
@github-actions
Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant