From df162fe5f449f80507ebe124ffea5ff1f55bc66a Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 24 Jul 2026 22:44:11 -0700 Subject: [PATCH 1/4] Kotlin: emit every optional model array as List? = null MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING (Kotlin). The model generator previously special-cased only RichTextAttachment optional arrays as nullable, defaulting every other optional array to `List = emptyList()`. That sentinel violates SPEC.md's Optional Fields rule (an empty list is not a substitute for absence) and left Kotlin the lone SDK that couldn't distinguish an absent array from a present-but-empty one — Go (nil), Swift/TS (optional), Python (NotRequired), Ruby (nil) all preserve it. ModelEmitter.resolvePropertyType's array branch now drops the `itemType == "RichTextAttachment"` condition, so EVERY optional array emits `List? = null`; required arrays stay a plain `List`. Regenerated: 15 models, ~23 fields flip to nullable (assignees, steps, completionSubscribers, participants, subscribers, lists, wormholes, responses, types, recentDeliveries, dock, dots, days, added/removed/notified person id lists, attachments). Cascade (the only non-null derefs the compiler flagged): - TodosService.fieldsFromTodo: `todo.assignees.orEmpty().map` / `todo.completionSubscribers.orEmpty().map` (preserves the always-send-empties semantics of the writable ID lists). - WormholesServiceTest: `assertNotNull(cardTable.wormholes)` before use. D-invariant: scripts/check-kotlin-optional-arrays.rb (wired into `make check`) pins the contract — every optional generated array is `List? = null`, every required array stays `List`, and none default to `= emptyList()`. Updates the rich-text-attachments-coverage note now that the nullable-array behavior is general, not a RichTextAttachment special case. --- Makefile | 10 ++- .../basecamp/sdk/generator/ModelEmitter.kt | 20 +++-- .../sdk/generated/models/CampfireLine.kt | 2 +- .../com/basecamp/sdk/generated/models/Card.kt | 6 +- .../sdk/generated/models/CardColumn.kt | 2 +- .../basecamp/sdk/generated/models/CardStep.kt | 2 +- .../sdk/generated/models/CardTable.kt | 6 +- .../sdk/generated/models/ClientApproval.kt | 2 +- .../sdk/generated/models/EventDetails.kt | 6 +- .../sdk/generated/models/HillChart.kt | 2 +- .../basecamp/sdk/generated/models/Project.kt | 2 +- .../sdk/generated/models/QuestionSchedule.kt | 2 +- .../sdk/generated/models/ScheduleEntry.kt | 2 +- .../sdk/generated/models/Subscription.kt | 2 +- .../basecamp/sdk/generated/models/Template.kt | 2 +- .../com/basecamp/sdk/generated/models/Todo.kt | 6 +- .../basecamp/sdk/generated/models/Webhook.kt | 4 +- .../com/basecamp/sdk/services/TodosService.kt | 4 +- .../com/basecamp/sdk/WormholesServiceTest.kt | 11 +-- scripts/check-kotlin-optional-arrays.rb | 78 +++++++++++++++++++ scripts/check-kotlin-optional-arrays.sh | 14 ++++ .../rich-text-attachments-coverage.md | 4 +- 22 files changed, 143 insertions(+), 46 deletions(-) create mode 100755 scripts/check-kotlin-optional-arrays.rb create mode 100755 scripts/check-kotlin-optional-arrays.sh diff --git a/Makefile b/Makefile index d54185389..9d215b884 100644 --- a/Makefile +++ b/Makefile @@ -743,7 +743,7 @@ tools: # Spec-shape lints #------------------------------------------------------------------------------ -.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity +.PHONY: check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays # Verify every bucket-scoped GET list operation has a flat-path counterpart # (or is justified in spec/bucket-scoped-allowlist.txt). Cross-project SDK @@ -765,6 +765,12 @@ check-deprecation-parity: rb-build validate-api-gaps: @./scripts/validate-api-gaps.sh +# D-invariant: every optional generated Kotlin array is `List? = null`, every +# required array stays `List`, and none default to the `= emptyList()` +# sentinel — pinning the optional-array presence contract against regression. +kt-check-optional-arrays: + @./scripts/check-kotlin-optional-arrays.sh + #------------------------------------------------------------------------------ # Combined targets #------------------------------------------------------------------------------ @@ -787,7 +793,7 @@ generate: @echo "==> Generation complete" # Run all checks (Smithy + Go + TypeScript + Ruby + Kotlin + Swift + Python + Behavior Model + Conformance + Provenance + Actions lint) -check: lint-actions sync-spec-version-check smithy-check behavior-model-check provenance-check sync-api-version-check go-check-drift go-check-wrapper-drift auth-routable-check kt-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity +check: lint-actions sync-spec-version-check smithy-check behavior-model-check provenance-check sync-api-version-check go-check-drift go-check-wrapper-drift auth-routable-check kt-check-drift go-check ts-check rb-check kt-check swift-check py-check conformance check-bucket-flat-parity validate-api-gaps check-deprecation-parity kt-check-optional-arrays @echo "==> All checks passed" # Clean all build artifacts diff --git a/kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/ModelEmitter.kt b/kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/ModelEmitter.kt index b755d3b86..22e84b6b0 100644 --- a/kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/ModelEmitter.kt +++ b/kotlin/generator/src/main/kotlin/com/basecamp/sdk/generator/ModelEmitter.kt @@ -191,17 +191,15 @@ class ModelEmitter(private val api: OpenApiParser) { "string" -> if (isRequired) "String" else "String?" "array" -> { val itemType = resolveArrayItemType(schema["items"]?.jsonObject) - // Rich-text companion arrays (RichTextAttachmentList) carry a - // documented cross-SDK presence contract (SPEC.md §10): an - // OPTIONAL one must stay nullable so an absent array (a - // non-matching projection type or a webhook-sourced item) is - // distinct from a present-but-empty one, matching how Go (nil), - // Swift/TypeScript (optional), Python (NotRequired), and Ruby - // (nil) already decode it. A bare `List = emptyList()` would - // be a sentinel for absence, which SPEC.md's Optional Fields - // rule forbids. Other optional arrays keep the empty-list - // convention (no such presence contract). - if (!isRequired && itemType == "RichTextAttachment") "List<$itemType>?" else "List<$itemType>" + // Every OPTIONAL array is nullable (`List? = null`) so an + // absent array stays distinct from a present-but-empty one, + // per SPEC.md's Optional Fields rule (a sentinel — here + // `= emptyList()` — is not a substitute for absence). This + // aligns Kotlin with the other five SDKs, all of which already + // preserve the distinction: Go (nil pointer), Swift/TypeScript + // (optional), Python (NotRequired), Ruby (nil). REQUIRED arrays + // are always emitted, so they stay a plain non-null `List`. + if (!isRequired) "List<$itemType>?" else "List<$itemType>" } "object" -> if (isRequired) "JsonObject" else "JsonObject?" else -> if (isRequired) "JsonElement" else "JsonElement?" diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CampfireLine.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CampfireLine.kt index 64b8fc621..5c79bde16 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CampfireLine.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CampfireLine.kt @@ -27,7 +27,7 @@ data class CampfireLine( val creator: Person, @SerialName("bookmark_url") val bookmarkUrl: String? = null, val content: String? = null, - val attachments: List = emptyList(), + val attachments: List? = null, @SerialName("boosts_count") val boostsCount: Int = 0, @SerialName("boosts_url") val boostsUrl: String? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Card.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Card.kt index 1bcd82524..b1eace173 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Card.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Card.kt @@ -38,9 +38,9 @@ data class Card( @SerialName("comments_url") val commentsUrl: String? = null, @SerialName("completion_url") val completionUrl: String? = null, val completer: Person? = null, - val assignees: List = emptyList(), - @SerialName("completion_subscribers") val completionSubscribers: List = emptyList(), - val steps: List = emptyList(), + val assignees: List? = null, + @SerialName("completion_subscribers") val completionSubscribers: List? = null, + val steps: List? = null, @SerialName("boosts_count") val boostsCount: Int = 0, @SerialName("boosts_url") val boostsUrl: String? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardColumn.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardColumn.kt index b6e6c21fe..38bef1b6f 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardColumn.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardColumn.kt @@ -32,6 +32,6 @@ data class CardColumn( @SerialName("cards_count") val cardsCount: Int = 0, @SerialName("comments_count") val commentsCount: Int = 0, @SerialName("cards_url") val cardsUrl: String? = null, - val subscribers: List = emptyList(), + val subscribers: List? = null, @SerialName("on_hold") val onHold: CardColumnOnHold? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardStep.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardStep.kt index 442ff237e..e24f93452 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardStep.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardStep.kt @@ -31,6 +31,6 @@ data class CardStep( val completed: Boolean = false, @SerialName("completed_at") val completedAt: String? = null, val completer: Person? = null, - val assignees: List = emptyList(), + val assignees: List? = null, @SerialName("completion_url") val completionUrl: String? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardTable.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardTable.kt index 2e765a267..e3f7cc62c 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardTable.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/CardTable.kt @@ -26,7 +26,7 @@ data class CardTable( val creator: Person, @SerialName("bookmark_url") val bookmarkUrl: String? = null, @SerialName("subscription_url") val subscriptionUrl: String? = null, - val subscribers: List = emptyList(), - val lists: List = emptyList(), - val wormholes: List = emptyList() + val subscribers: List? = null, + val lists: List? = null, + val wormholes: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ClientApproval.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ClientApproval.kt index 52d084528..ec3d4518e 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ClientApproval.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ClientApproval.kt @@ -35,5 +35,5 @@ data class ClientApproval( @SerialName("replies_url") val repliesUrl: String? = null, @SerialName("approval_status") val approvalStatus: String? = null, val approver: Person? = null, - val responses: List = emptyList() + val responses: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/EventDetails.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/EventDetails.kt index d9512cf9a..5528ad8eb 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/EventDetails.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/EventDetails.kt @@ -12,7 +12,7 @@ import kotlinx.serialization.json.JsonObject */ @Serializable data class EventDetails( - @SerialName("added_person_ids") val addedPersonIds: List = emptyList(), - @SerialName("removed_person_ids") val removedPersonIds: List = emptyList(), - @SerialName("notified_recipient_ids") val notifiedRecipientIds: List = emptyList() + @SerialName("added_person_ids") val addedPersonIds: List? = null, + @SerialName("removed_person_ids") val removedPersonIds: List? = null, + @SerialName("notified_recipient_ids") val notifiedRecipientIds: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/HillChart.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/HillChart.kt index fc57948b6..554e05c9d 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/HillChart.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/HillChart.kt @@ -17,5 +17,5 @@ data class HillChart( @SerialName("updated_at") val updatedAt: String? = null, @SerialName("app_update_url") val appUpdateUrl: String? = null, @SerialName("app_versions_url") val appVersionsUrl: String? = null, - val dots: List = emptyList() + val dots: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Project.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Project.kt index e7cc5383a..ed5f7b94f 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Project.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Project.kt @@ -26,7 +26,7 @@ data class Project( @SerialName("end_date") val endDate: String? = null, @SerialName("clients_enabled") val clientsEnabled: Boolean = false, @SerialName("bookmark_url") val bookmarkUrl: String? = null, - val dock: List = emptyList(), + val dock: List? = null, val bookmarked: Boolean = false, @SerialName("client_company") val clientCompany: ClientCompany? = null, @Deprecated("This shape is deprecated since 2024-01: Use Client Visibility feature instead") diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/QuestionSchedule.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/QuestionSchedule.kt index 8b76611a3..e4493f46c 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/QuestionSchedule.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/QuestionSchedule.kt @@ -13,7 +13,7 @@ import kotlinx.serialization.json.JsonObject @Serializable data class QuestionSchedule( val frequency: String? = null, - val days: List = emptyList(), + val days: List? = null, val hour: Int = 0, val minute: Int = 0, @SerialName("week_instance") val weekInstance: Int = 0, diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ScheduleEntry.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ScheduleEntry.kt index 7453dc3b8..6dc366b2d 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ScheduleEntry.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/ScheduleEntry.kt @@ -35,7 +35,7 @@ data class ScheduleEntry( @SerialName("all_day") val allDay: Boolean = false, @SerialName("starts_at") val startsAt: String? = null, @SerialName("ends_at") val endsAt: String? = null, - val participants: List = emptyList(), + val participants: List? = null, @SerialName("boosts_count") val boostsCount: Int = 0, @SerialName("boosts_url") val boostsUrl: String? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Subscription.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Subscription.kt index 951315775..a6dd7b49f 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Subscription.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Subscription.kt @@ -15,5 +15,5 @@ data class Subscription( val subscribed: Boolean, val count: Int, val url: String, - val subscribers: List = emptyList() + val subscribers: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Template.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Template.kt index dcbf8c044..576ada3a1 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Template.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Template.kt @@ -20,5 +20,5 @@ data class Template( val description: String? = null, val url: String? = null, @SerialName("app_url") val appUrl: String? = null, - val dock: List = emptyList() + val dock: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Todo.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Todo.kt index 4bba09258..a0773850c 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Todo.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Todo.kt @@ -36,10 +36,10 @@ data class Todo( val completed: Boolean = false, @SerialName("starts_on") val startsOn: String? = null, @SerialName("due_on") val dueOn: String? = null, - val assignees: List = emptyList(), - @SerialName("completion_subscribers") val completionSubscribers: List = emptyList(), + val assignees: List? = null, + @SerialName("completion_subscribers") val completionSubscribers: List? = null, @SerialName("completion_url") val completionUrl: String? = null, @SerialName("boosts_count") val boostsCount: Int = 0, @SerialName("boosts_url") val boostsUrl: String? = null, - val steps: List = emptyList() + val steps: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Webhook.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Webhook.kt index 03f8f0208..8d2fd39b9 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Webhook.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated/models/Webhook.kt @@ -19,6 +19,6 @@ data class Webhook( val url: String, @SerialName("app_url") val appUrl: String, val active: Boolean = false, - val types: List = emptyList(), - @SerialName("recent_deliveries") val recentDeliveries: List = emptyList() + val types: List? = null, + @SerialName("recent_deliveries") val recentDeliveries: List? = null ) diff --git a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/TodosService.kt b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/TodosService.kt index 2baf32522..c59d495e0 100644 --- a/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/TodosService.kt +++ b/kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/services/TodosService.kt @@ -94,8 +94,8 @@ class TodosService(client: AccountClient) : private fun fieldsFromTodo(todo: Todo): TodoFields = TodoFields( content = todo.content, description = todo.description ?: "", - assigneeIds = todo.assignees.map { it.id }, - completionSubscriberIds = todo.completionSubscribers.map { it.id }, + assigneeIds = todo.assignees.orEmpty().map { it.id }, + completionSubscriberIds = todo.completionSubscribers.orEmpty().map { it.id }, dueOn = todo.dueOn ?: "", startsOn = todo.startsOn ?: "", ) diff --git a/kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/WormholesServiceTest.kt b/kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/WormholesServiceTest.kt index b9e589f23..1b4ac58a7 100644 --- a/kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/WormholesServiceTest.kt +++ b/kotlin/sdk/src/commonTest/kotlin/com/basecamp/sdk/WormholesServiceTest.kt @@ -276,11 +276,12 @@ class WormholesServiceTest { val account = client.forAccount("12345") val cardTable = account.cardTables.get(cardTableId = 1069479345) - assertEquals(2, cardTable.wormholes.size) - assertTrue(cardTable.wormholes[0].linked) - assertNotNull(cardTable.wormholes[0].destinationUrl) - assertTrue(!cardTable.wormholes[1].linked) - assertNull(cardTable.wormholes[1].destinationUrl) + val wormholes = assertNotNull(cardTable.wormholes) + assertEquals(2, wormholes.size) + assertTrue(wormholes[0].linked) + assertNotNull(wormholes[0].destinationUrl) + assertTrue(!wormholes[1].linked) + assertNull(wormholes[1].destinationUrl) client.close() } diff --git a/scripts/check-kotlin-optional-arrays.rb b/scripts/check-kotlin-optional-arrays.rb new file mode 100755 index 000000000..048baaa10 --- /dev/null +++ b/scripts/check-kotlin-optional-arrays.rb @@ -0,0 +1,78 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# D-invariant guard for the Kotlin generator's optional-array contract. +# +# Every OPTIONAL generated array must be `List? = null` so an absent array +# stays distinct from a present-but-empty one (SPEC.md Optional Fields rule), +# aligning Kotlin with the other five SDKs. Every REQUIRED array stays a plain +# non-null `List`. No generated array may default to the `= emptyList()` +# sentinel that this fix removed. +# +# Concretely, this scans every generated Kotlin declaration of a `List<...>` +# property and enforces: +# * a property with a default MUST be nullable with a `= null` default +# (so `= emptyList()`, or any other non-null default, fails); +# * a nullable array that carries a default MUST default to exactly `null`. +# A required array (`List` with no default) and a nullable request param +# (`List?` with no default) both pass. +# +# Pins the D fix against regression. Wired into `make check`. + +PROJECT_ROOT = File.expand_path("..", __dir__) +GENERATED_DIR = File.join( + PROJECT_ROOT, + "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated" +) + +errors = [] + +files = Dir.glob(File.join(GENERATED_DIR, "**", "*.kt")).sort +if files.empty? + warn "ERROR: no generated Kotlin files found under #{GENERATED_DIR}" + exit 2 +end + +files.each do |path| + rel = path.sub("#{PROJECT_ROOT}/", "") + File.foreach(path).with_index(1) do |line, lineno| + # Match an array-typed property declaration: capture everything after the + # first `: List<`. Generated properties are one per line. + next unless line =~ /:\s*(List<.*)$/ + + decl = Regexp.last_match(1).strip.sub(/[,)]\s*$/, "").strip + + type_part, default_part = + if decl.include?("=") + left, right = decl.split("=", 2) + [left.strip, right.strip] + else + [decl, nil] + end + + nullable = type_part.end_with?("?") + + next if default_part.nil? # required array, or nullable param w/o default — OK + + if default_part == "emptyList()" + errors << "#{rel}:#{lineno}: optional array uses the forbidden " \ + "`= emptyList()` sentinel — must be `List? = null`" + elsif !nullable + errors << "#{rel}:#{lineno}: non-null array `#{type_part}` carries a " \ + "default (`= #{default_part}`) — an optional array must be " \ + "nullable (`List? = null`)" + elsif default_part != "null" + errors << "#{rel}:#{lineno}: nullable array defaults to " \ + "`#{default_part}` — must default to `null`" + end + end +end + +if errors.empty? + puts "==> Kotlin optional-array invariant clean — #{files.size} generated files scanned" + exit 0 +else + warn "Kotlin optional-array invariant failed:" + errors.each { |e| warn " - #{e}" } + exit 1 +end diff --git a/scripts/check-kotlin-optional-arrays.sh b/scripts/check-kotlin-optional-arrays.sh new file mode 100755 index 000000000..d20dea601 --- /dev/null +++ b/scripts/check-kotlin-optional-arrays.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# D-invariant guard: every optional generated Kotlin array is `List? = null`, +# every required array stays `List`, and none default to `= emptyList()`. +# See scripts/check-kotlin-optional-arrays.rb. +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +if ! command -v ruby >/dev/null 2>&1; then + echo "ERROR: ruby is required for check-kotlin-optional-arrays" >&2 + exit 2 +fi + +exec ruby "$SCRIPT_DIR/check-kotlin-optional-arrays.rb" diff --git a/spec/api-gaps/rich-text-attachments-coverage.md b/spec/api-gaps/rich-text-attachments-coverage.md index 3381ae326..cebdadc42 100644 --- a/spec/api-gaps/rich-text-attachments-coverage.md +++ b/spec/api-gaps/rich-text-attachments-coverage.md @@ -135,8 +135,8 @@ present-empty, populated — round-trip faithfully in both directions: Go `*[]RichTextAttachment` (nil pointer omitted, non-nil pointer to `[]` re-encodes as `[]`), Swift `[RichTextAttachment]?`, TypeScript `?: …[]`, Python `NotRequired[…]`, Ruby nil-vs-`[]` (`parse_array`), and Kotlin `List? = null` -(the model generator emits a nullable list for optional `RichTextAttachmentList` -members rather than the empty-list default it uses for other optional arrays). The +(the model generator emits `List? = null` for *every* optional array, so an +absent array is distinct from a present-but-empty one). The 14 concrete `@required` members are always present, so they stay a plain non-optional list in each SDK. From 71d20d937f687c01be6168e16ccad85191cfcad6 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 24 Jul 2026 22:44:18 -0700 Subject: [PATCH 2/4] Rubric 1B.4: optional-field note covers all six SDKs + Go's encode-only edge Go already meets the SPEC decode bar (nil vs non-nil-empty slice), so a blanket *[]T sweep would be an ergonomic regression for a re-encode-only edge case. Rewrite the 1B.4 note to name each SDK's optional-field representation (Go pointers, TS optional chaining, Ruby nilable, Python NotRequired, Swift optionals, Kotlin now-nullable) and to record Go's accepted omitempty encode-only collapse; *[]T stays only where rich-text projections need the two-way wire distinction. --- rubric-audit.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rubric-audit.json b/rubric-audit.json index af8c27d8e..8476a02dc 100644 --- a/rubric-audit.json +++ b/rubric-audit.json @@ -13,7 +13,7 @@ }, "1B.4": { "pass": true, - "note": "Optional fields use pointers (Go), optional chaining (TS), nilable (Ruby)" + "note": "Optional fields preserve absent-vs-present across all six SDKs: Go pointers (*T / *[]T), TS optional chaining (?: T), Ruby nilable (nil vs value), Python NotRequired[T], Swift optionals (T?), Kotlin nullable (T? = null, incl. List? = null for optional arrays). Go accepts one encode-only edge: an optional slice with `omitempty` re-encodes a non-nil empty slice as absent (the decode distinction nil-vs-empty is preserved; only re-serialization collapses it), a deliberate ergonomic trade-off rather than the blanket *[]T sweep. *[]T is used only where rich-text projections need the wire distinction round-tripped in both directions." }, "1B.5": { "pass": true, From 57617323ac458a12ea665f05ae3dbcc4003eaca1 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 24 Jul 2026 23:10:08 -0700 Subject: [PATCH 3/4] Address review: prove required arrays non-null, honest 1B.4, wire D guard into CI Addresses the external review on #433: - Invariant now proves both halves (blocker): check-kotlin-optional-arrays.rb cross-checks each generated model array against its OpenAPI component's `required` set. A REQUIRED array must be non-null `List` (no `?`, no default); an OPTIONAL array must be `List? = null`. So a required array mistakenly emitted as `List?` is now caught, not silently accepted. Request/param and non-component supporting types keep the structural check (no `= emptyList()`; defaulted arrays must be `?= null`). - rubric-audit 1B.4 is honest (blocker): set to pass:false + issue #436. The earlier note falsely claimed every optional field preserves absence across all six SDKs, but Kotlin optional non-array scalars still use zero-value sentinels (Boolean = false, Int = 0, Long = 0L), which SPEC.md's Optional Fields rule explicitly forbids. #436 tracks that broader (array-scoped-fix-excluded) debt. - Wire the optional-array guard into CI (blocker): a step in the Kotlin test job runs the invariant; it was only in `make check`, which CI never invoked. --- .github/workflows/test.yml | 3 + rubric-audit.json | 5 +- scripts/check-kotlin-optional-arrays.rb | 104 +++++++++++++++++++----- 3 files changed, 89 insertions(+), 23 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 096dd0f57..a5b02daec 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -316,6 +316,9 @@ jobs: fi echo "Generated code is up to date" + - name: Check optional-array invariant + run: ruby ../scripts/check-kotlin-optional-arrays.rb + - name: Run Kotlin conformance tests run: ./gradlew :conformance:run diff --git a/rubric-audit.json b/rubric-audit.json index 8476a02dc..fb0d78ef4 100644 --- a/rubric-audit.json +++ b/rubric-audit.json @@ -12,8 +12,9 @@ "note": "Request/response types generated from OpenAPI schema" }, "1B.4": { - "pass": true, - "note": "Optional fields preserve absent-vs-present across all six SDKs: Go pointers (*T / *[]T), TS optional chaining (?: T), Ruby nilable (nil vs value), Python NotRequired[T], Swift optionals (T?), Kotlin nullable (T? = null, incl. List? = null for optional arrays). Go accepts one encode-only edge: an optional slice with `omitempty` re-encodes a non-nil empty slice as absent (the decode distinction nil-vs-empty is preserved; only re-serialization collapses it), a deliberate ergonomic trade-off rather than the blanket *[]T sweep. *[]T is used only where rich-text projections need the wire distinction round-tripped in both directions." + "pass": false, + "issue": 436, + "note": "Partially met. Optional OBJECT/REF/ARRAY and nullable-typed fields preserve absence across all six SDKs: Go pointers (*T / *[]T), TS optional chaining (?: T), Ruby nilable, Python NotRequired[T], Swift optionals (T?), Kotlin nullable (T? = null, incl. List? = null for optional arrays after #433, pinned by check-kotlin-optional-arrays). NOT yet met for optional non-array SCALARS: Kotlin emits them with zero-value sentinels (Boolean = false, Int = 0, Long = 0L), which SPEC.md's Optional Fields rule explicitly forbids (0/'' are not substitutes for absence). Go/Ruby/Swift additionally collapse optional scalar/nil zero-values on RE-ENCODE only (decode faithful; omitempty / compact / synthesized-encoder — documented in SPEC.md §10). Tracked in #436. Go's *[]T is used only where rich-text projections need the wire distinction round-tripped both directions; not a blanket sweep." }, "1B.5": { "pass": true, diff --git a/scripts/check-kotlin-optional-arrays.rb b/scripts/check-kotlin-optional-arrays.rb index 048baaa10..ec912f473 100755 --- a/scripts/check-kotlin-optional-arrays.rb +++ b/scripts/check-kotlin-optional-arrays.rb @@ -9,21 +9,38 @@ # non-null `List`. No generated array may default to the `= emptyList()` # sentinel that this fix removed. # -# Concretely, this scans every generated Kotlin declaration of a `List<...>` -# property and enforces: -# * a property with a default MUST be nullable with a `= null` default -# (so `= emptyList()`, or any other non-null default, fails); -# * a nullable array that carries a default MUST default to exactly `null`. -# A required array (`List` with no default) and a nullable request param -# (`List?` with no default) both pass. +# Two levels of enforcement: +# +# * Schema-checked (generated/models/.kt whose basename is an +# OpenAPI component): each array property is cross-checked against the +# component's `required` set — a REQUIRED array MUST be non-null `List` +# (no `?`, no default); an OPTIONAL array MUST be `List? = null`. This +# proves both halves of the guarantee, so a required array mistakenly +# emitted as `List?` is caught, not silently accepted. +# * Structural (everything else — request/param types in generated/services, +# supporting model types not present as components): a property with a +# default MUST be `List? = null` (so `= emptyList()` or any non-null +# default fails). Requiredness can't be derived here, so a bare `List` +# or nullable-without-default is left alone. # # Pins the D fix against regression. Wired into `make check`. +require "json" + PROJECT_ROOT = File.expand_path("..", __dir__) GENERATED_DIR = File.join( PROJECT_ROOT, "kotlin/sdk/src/commonMain/kotlin/com/basecamp/sdk/generated" ) +MODELS_DIR = File.join(GENERATED_DIR, "models") +OPENAPI_FILE = File.join(PROJECT_ROOT, "openapi.json") + +unless File.file?(OPENAPI_FILE) + warn "ERROR: openapi.json not found at #{OPENAPI_FILE}" + exit 2 +end + +components = JSON.parse(File.read(OPENAPI_FILE)).dig("components", "schemas") || {} errors = [] @@ -33,26 +50,71 @@ exit 2 end +# Parse one property line into [wire_name, type_part, default_part] or nil when +# the line isn't an array-typed `val`. `wire_name` is the @SerialName value when +# present, else the Kotlin field name (the generator only adds @SerialName when +# the camelCase name differs from the wire name, so this recovers the wire name). +def parse_array_property(line) + return nil unless line =~ /:\s*(List<.*)$/ + + decl = Regexp.last_match(1).strip.sub(/[,)]\s*$/, "").strip + field = line[/\bval\s+(\w+)\s*:/, 1] + return nil unless field + + serial = line[/@SerialName\("([^"]+)"\)/, 1] + wire = serial || field + + type_part, default_part = + if decl.include?("=") + left, right = decl.split("=", 2) + [left.strip, right.strip] + else + [decl, nil] + end + + [wire, type_part, default_part] +end + files.each do |path| rel = path.sub("#{PROJECT_ROOT}/", "") + component_name = + if path.start_with?("#{MODELS_DIR}/") + base = File.basename(path, ".kt") + base if components.key?(base) + end + component = component_name ? components[component_name] : nil + required_fields = component ? (component["required"] || []) : nil + known_props = component ? (component["properties"] || {}) : nil + File.foreach(path).with_index(1) do |line, lineno| - # Match an array-typed property declaration: capture everything after the - # first `: List<`. Generated properties are one per line. - next unless line =~ /:\s*(List<.*)$/ - - decl = Regexp.last_match(1).strip.sub(/[,)]\s*$/, "").strip - - type_part, default_part = - if decl.include?("=") - left, right = decl.split("=", 2) - [left.strip, right.strip] - else - [decl, nil] - end + parsed = parse_array_property(line) + next unless parsed + wire, type_part, default_part = parsed nullable = type_part.end_with?("?") + has_default = !default_part.nil? + + # Schema-checked path: the field is a known property of an OpenAPI component. + if required_fields && known_props&.key?(wire) + if required_fields.include?(wire) + if nullable || has_default + errors << "#{rel}:#{lineno}: `#{wire}` is REQUIRED in schema " \ + "`#{component_name}` but is emitted as `#{type_part}" \ + "#{has_default ? " = #{default_part}" : ''}` — a required " \ + "array must be non-null `List` with no default" + end + elsif !nullable || default_part != "null" + errors << "#{rel}:#{lineno}: `#{wire}` is OPTIONAL in schema " \ + "`#{component_name}` but is emitted as `#{type_part}" \ + "#{has_default ? " = #{default_part}" : ''}` — an optional " \ + "array must be `List? = null`" + end + next + end - next if default_part.nil? # required array, or nullable param w/o default — OK + # Structural path: requiredness unknown (request/param types, supporting + # non-component models). Only a defaulted array is constrained. + next unless has_default if default_part == "emptyList()" errors << "#{rel}:#{lineno}: optional array uses the forbidden " \ From b2d031da212f954b5be9db1b692c0c89edf831e8 Mon Sep 17 00:00:00 2001 From: Jeremy Daer Date: Fri, 24 Jul 2026 23:49:39 -0700 Subject: [PATCH 4/4] Address review round 2: UTF-8 reads, correct Go optional-scalar decode claim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #433 second round: - Locale robustness: read openapi.json and the generated .kt files as explicit UTF-8 in check-kotlin-optional-arrays.rb (both File.read and File.foreach) — LC_ALL=C otherwise reads US-ASCII and chokes on UTF-8 bytes. - rubric-audit 1B.4 + #436 corrected: Go optional NON-POINTER scalars collapse absence on DECODE (an absent field and an explicit zero both unmarshal to the zero value); only pointer fields (*T) preserve it. The earlier note wrongly called Go decode uniformly faithful. Ruby/Swift decode faithfully but drop nil on re-encode. Still pass:false, tracked in #436. --- rubric-audit.json | 2 +- scripts/check-kotlin-optional-arrays.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/rubric-audit.json b/rubric-audit.json index fb0d78ef4..eb5ab945a 100644 --- a/rubric-audit.json +++ b/rubric-audit.json @@ -14,7 +14,7 @@ "1B.4": { "pass": false, "issue": 436, - "note": "Partially met. Optional OBJECT/REF/ARRAY and nullable-typed fields preserve absence across all six SDKs: Go pointers (*T / *[]T), TS optional chaining (?: T), Ruby nilable, Python NotRequired[T], Swift optionals (T?), Kotlin nullable (T? = null, incl. List? = null for optional arrays after #433, pinned by check-kotlin-optional-arrays). NOT yet met for optional non-array SCALARS: Kotlin emits them with zero-value sentinels (Boolean = false, Int = 0, Long = 0L), which SPEC.md's Optional Fields rule explicitly forbids (0/'' are not substitutes for absence). Go/Ruby/Swift additionally collapse optional scalar/nil zero-values on RE-ENCODE only (decode faithful; omitempty / compact / synthesized-encoder — documented in SPEC.md §10). Tracked in #436. Go's *[]T is used only where rich-text projections need the wire distinction round-tripped both directions; not a blanket sweep." + "note": "Partially met. Optional OBJECT/REF/ARRAY and nullable-typed fields preserve absence across all six SDKs: Go pointers (*T / *[]T), TS optional chaining (?: T), Ruby nilable, Python NotRequired[T], Swift optionals (T?), Kotlin nullable (T? = null, incl. List? = null for optional arrays after #433, pinned by check-kotlin-optional-arrays). NOT yet met for optional non-array SCALARS: Kotlin emits them with zero-value sentinels (Boolean = false, Int = 0, Long = 0L) — a static-type violation of SPEC.md's Optional Fields rule (0/'' are not substitutes for absence). Go optional NON-POINTER scalars collapse absence on DECODE too (an absent field and an explicit zero both unmarshal to the zero value); only Go pointer fields (*T) preserve it. Ruby/Swift decode faithfully but drop nil on RE-ENCODE (compact / synthesized encoder). All documented in SPEC.md §10 and tracked in #436. Go's *[]T is used only where rich-text projections need the wire distinction round-tripped both directions; not a blanket sweep." }, "1B.5": { "pass": true, diff --git a/scripts/check-kotlin-optional-arrays.rb b/scripts/check-kotlin-optional-arrays.rb index ec912f473..ececf0f6d 100755 --- a/scripts/check-kotlin-optional-arrays.rb +++ b/scripts/check-kotlin-optional-arrays.rb @@ -40,7 +40,9 @@ exit 2 end -components = JSON.parse(File.read(OPENAPI_FILE)).dig("components", "schemas") || {} +# Read as UTF-8 regardless of process locale (LC_ALL=C would otherwise read as +# US-ASCII and choke on the spec's / generated code's UTF-8 bytes). +components = JSON.parse(File.read(OPENAPI_FILE, encoding: "UTF-8")).dig("components", "schemas") || {} errors = [] @@ -86,7 +88,7 @@ def parse_array_property(line) required_fields = component ? (component["required"] || []) : nil known_props = component ? (component["properties"] || {}) : nil - File.foreach(path).with_index(1) do |line, lineno| + File.foreach(path, encoding: "UTF-8").with_index(1) do |line, lineno| parsed = parse_array_property(line) next unless parsed