feat: add a checkRequired builder helper for uniform required-field validation#91
Merged
Conversation
SDK builders validated their mandatory fields with hand-rolled
`checkNotNull(field) { "..." }` calls, and the messages had drifted:
the Request builder threw "Method is required." / "URL is required."
(capitalized, trailing period) while the Response builder threw
"request is required" / "protocol is required" / "status is required"
(lowercase, no period). Callers that key off these messages, and the
builders codegen will eventually emit, had no single contract to rely
on.
Add `checkRequired(name, value)` in the `generics` package, alongside
the `Builder` interface. It returns the value as a non-null reference
when present (so it drops straight into builder constructor calls) and
throws IllegalStateException with the uniform message "<name> is
required" when absent. A Kotlin contract lets the compiler smart-cast
the result to non-null.
Wire it into the Request and Response builders as the reference usage,
standardizing both on the lowercase field-name form.
This is a public API addition (BuilderChecksKt.checkRequired); the
sdk-core .api snapshot is regenerated accordingly.
checkNotNull already declares returns() implies (value != null) and checkRequired returns a non-null T by signature, so the explicit contract only added smart-casting of the argument variable after the call. Neither builder call site relies on that (both consume the return value directly), so remove the contract and its ExperimentalContracts opt-in and collapse to a single expression.
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
1 participant
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.
Summary
Builders hand-rolled required-field validation with inconsistent messages (e.g.
"Method is required."vs"request is required"). This adds a single helper:It returns the value when present and throws
IllegalStateException("<name> is required")when null. The non-null guarantee comes from the return type, so call sites assign the result directly. TheRequestandResponsebuilders are wired onto it as the reference usage, standardizing their messages. New tests cover the present and missing cases; the helper is added to the API snapshot.Closes #48