Repository navigation
feat: gate semantic coercions behind explicit per-field consent - #7
Closed
Baoyuan-Xing wants to merge 1 commit into
Closed
Baoyuan-Xing wants to merge 1 commit into
Baoyuan-Xing wants to merge 1 commit into
Conversation
The coercion risk classes describe two different kinds of danger. Truncation, precision loss, runtime checks, and lossless widenings are value-level effects a caller can reason about globally: "I accept narrower numbers" is a coherent blanket policy, so the global switch (`coerce_all` / `--coerce`) enabling them wholesale is sound. Semantic coercions are not like that. Timestamp → int64 and Date32 → int32 impose an epoch/unit interpretation the source type does not carry; signed ↔ unsigned reinterprets bit patterns; utf8 ↔ bytes crosses a meaning boundary. Whether each is correct depends on the specific field — which unit downstream readers expect, whether ids are genuinely non-negative. A global flag cannot express that judgment, and enabling these silently invites the worst kind of bug: encodes that succeed while meaning something else. This change makes the global switch enable every risk class EXCEPT Semantic. Semantic coercions now require explicit per-field consent — a `[(apb.apb).coerce = true]` annotation or an explicit binding — where the decision is visible in the proto and reviewed with it. Mechanically: `resolve_type_check` takes a `CoercionConsent` (None / Global / Explicit) instead of a bool; inference maps the per-field annotation to Explicit and `coerce_all` to Global; explicit bindings map `coerce: true` to Explicit. A new `TypeErrorReason::SemanticCoercionNotExplicit` tells callers exactly what to do: semantic coercion requires an explicit per-field (apb.apb).coerce annotation; not enabled by global coercion Validation reports distinguish the same case. Behavior of explicit bindings and annotations is unchanged; the only new rejection is Global-consent × Semantic-risk, which previously succeeded silently.
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.
Why
The risk classes describe two different kinds of danger, and the global
--coerceswitch currently treats them the same.Truncation / precision loss / runtime check / lossless are value-level effects a caller can accept globally — "I'm fine with narrower numbers" is a coherent blanket policy.
Semantic coercions change what the bytes mean:
Timestamp → int64andDate32 → int32impose an epoch/unit interpretation the source type doesn't carry (millis? micros? days?); signed ↔ unsigned reinterprets bit patterns; utf8 ↔ bytes crosses a meaning boundary. Whether each is right is a per-field design decision — a global flag can't express it, and enabling them silently invites encodes that succeed while meaning something else entirely. (Motivating case: a pipeline running--coercefor legitimate int-narrowing would silently encode aTIMESTAMPcolumn into anint64proto field with a unit nobody chose.)What
--coerce/coerce_allnow enables every risk class exceptSemantic. Semantic coercions require explicit per-field consent —[(apb.apb).coerce = true]on the field, or an explicit binding — where the decision is visible in the proto and reviewed with it.resolve_type_checktakesCoercionConsent { None | Global | Explicit }instead of a boolExplicit,coerce_all→Global; explicit bindings withcoerce: true→ExplicitTypeErrorReason::SemanticCoercionNotExplicitwith an actionable message: "semantic coercion requires an explicit per-field (apb.apb).coerce annotation; not enabled by global coercion"Compatibility
coerce_all× Semantic-risk pairs now reject at plan time with the actionable error, where they previously coerced silently — which is the pointresolve_type_check's signature changes (bool → enum); internal callers updatedTests
194/194 (4 new): consent matrix at the type-check level (Global enables non-semantic, rejects all four semantic families; Explicit enables all), plus an inference-level test that
coerce_allfails a utf8→bytes name-match with an error directing to the annotation. Clippy clean.