codegen: address style audit findings — clones, wildcards, unwrap#121
Draft
iainmcgin wants to merge 1 commit into
Draft
codegen: address style audit findings — clones, wildcards, unwrap#121iainmcgin wants to merge 1 commit into
iainmcgin wants to merge 1 commit into
Conversation
Three changes from a code-style audit of hand-written sources:
- `protoc-gen-buffa/src/main.rs`: build the `CodeGeneratorResponse` file
list by consuming `generated` with `.into_iter()` instead of cloning
every name and content string. The vector is the sole owner and is
not used afterward, so the clones were pure overhead on every plugin
invocation.
- `buffa-codegen/src/oneof.rs::to_snake_case`: replace
`c.to_lowercase().next().unwrap()` with `result.extend(c.to_lowercase())`.
Removes the unwrap and correctly preserves multi-codepoint lowercase
mappings instead of silently truncating them. Behavior is unchanged
for ASCII (the only thing protobuf identifiers contain).
- `buffa-codegen/src/{impl_message,impl_text,view}.rs`: replace nine
`_ => unreachable!()` arms over the protobuf `Type` enum with explicit
enumeration of the unhandled variants. Each match is now exhaustive,
so a future protobuf edition adding a `Type` variant produces a
compile error in codegen rather than a runtime panic. Two of the
replaced arms also had no diagnostic message at all.
No change to generated output — the unreachable arms never produce
tokens. Verified with `task gen-wkt-types` (no drift).
|
All contributors have signed the CLA ✍️ ✅ |
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.
Three changes from a code-style audit of hand-written sources, focused on iteration/ownership, error handling, and dispatch structure.
Changes
protoc-gen-buffa/src/main.rs— build theCodeGeneratorResponsefile list by consuminggeneratedwith.into_iter()instead of cloning every name and content string. The vector is the sole owner and is not used afterward, so the clones were pure overhead on every plugin invocation (2N string clones per protoc run).buffa-codegen/src/oneof.rs::to_snake_case— replacec.to_lowercase().next().unwrap()withresult.extend(c.to_lowercase()). Removes the unwrap and correctly preserves multi-codepoint lowercase mappings (e.g.İ→i\u{307}) instead of silently truncating them. Behavior is unchanged for ASCII, which is the only thing protobuf identifiers contain.buffa-codegen/src/{impl_message,impl_text,view}.rs— replace nine_ => unreachable!()arms over the protobufTypeenum with explicit enumeration of the unhandled variants. Each match is now exhaustive overType's 18 variants. A future protobuf edition that adds aTypevariant will produce a compile error inbuffa-codegenrather than a runtime panic during codegen, which is consistent with howwire_type_byteandwire_type_tokenalready fail. Two of the replaced arms also had no diagnostic message at all (view.rs:1021,impl_text.rs:716).Notes
No change to generated output — the
unreachable!()arms never produce tokens. Verified withtask gen-wkt-types(no drift).task lintandtask testboth pass.