generator: make generated Go getters tolerate a nil receiver

[erikmiller-gusto]

Description of your changes

Follow-up to #160.

Problem

The generated accessors return their field directly:

func (o *XComponentDefinitionEnvStatus) GetProviderConfigRefs() *...ProviderConfigRefs {
	return o.ProviderConfigRefs
}

So walking nested getters panics as soon as an intermediate struct is nil:

cd.GetStatus().GetProviderConfigRefs().GetAws().GetName() // panic

goRemoveRequired makes every field of a generated model optional, so intermediate
nils are the normal case for a partially-populated resource, not an edge case — a
status subresource that the controller hasn't filled in yet hits this immediately.
Callers end up writing explicit nil checks at every hop, which is most of what the
accessors were meant to remove.

What this does

Guards each getter with a nil-receiver check, the way protobuf-generated getters do,
so a chain over absent fields yields the zero value instead of panicking:

func (o *XComponentDefinitionEnvStatus) GetProviderConfigRefs() *...ProviderConfigRefs {
	if o == nil {
		return nil
	}
	return o.ProviderConfigRefs
}

Nilable field types return nil directly. Anything else — a value field, a fixed-size
array — returns a declared zero value, so the guard is correct for field shapes the
generator doesn't currently emit but could.

Setters are deliberately left unguarded: a set on a nil receiver has nowhere to store
the value, so panicking is the honest behaviour rather than silently dropping a write.

What this does not do

Scalar getters keep returning pointers (GetName() *string, not string). Protobuf
returns values for scalars, but for Kubernetes APIs the nil-vs-empty distinction is
load-bearing — omitempty, patch semantics, and "unset" versus "explicitly empty" are
all observable. Collapsing that would lose information, and the pointer return keeps
GetX/SetX symmetric. Callers still need one nil check at the end of a chain, not one
per hop.

Compatibility

Getter signatures are unchanged, and this only widens the set of receivers a getter
accepts, so no code that works today breaks. #160 is also not in a release yet, so
there are no released consumers either way.

Testing

• TestAddAccessorsGuardsNilReceiver — asserts every getter opens with the nil guard,
  that setters do not, and that nilable fields return nil while a value field and a
  fixed-size array get a declared zero.
• The existing compile gate now also runs what it builds: the consumer chains
  getters over an empty resource, and the materialized module is go tested. Compiling
  alone would not have caught this, since the old code typechecks fine and only fails at
  runtime.
• Verified the new tests fail without the generator change — the runtime test panics with
  a nil pointer dereference, which is the bug being fixed.
• Also regenerated a real 24-XRD project (~58k LOC of models) with the patched binary and
  confirmed cd.GetStatus().GetProviderConfigRefs().GetAws().GetName() returns nil on an
  empty resource instead of panicking.

go test ./..., go vet ./..., go build ./... and gofmt -l . are all clean.

I have:

• Read and followed Crossplane's contribution process.
• Run ./nix.sh flake check to ensure this PR is ready for review.
• Added or updated unit tests.
• Linked a PR or a docs tracking issue to document this change.
• Added backport release-x.y labels to auto-backport this PR.

On the struck items: nix isn't available in the environment this was developed in, so
flake check hasn't been run — go test/go vet/gofmt were run instead, and I'd
appreciate CI or a reviewer confirming the flake. This changes the body of generated
methods rather than any user-facing command or help text, so there's nothing for
crossplane/docs to pick up. And it fixes an unreleased feature from #160, so it isn't a
backport candidate.

Need help with this checklist? See the cheat sheet.

[coderabbitai]

📝 Walkthrough

Walkthrough

Generated getters now guard nil receivers and return type-appropriate zero values. Tests verify generated guard structure, preserve unguarded setters, and confirm safe chained access through nil intermediate values.

Changes

Nil-safe accessor generation

Layer / File(s)	Summary
Generate nil-safe getters internal/schemas/generator/accessors.go	The generator classifies nilable types and emits getter guards that return nil or a zero-initialized value for nil receivers.
Validate getter guards and chaining internal/schemas/generator/accessors_test.go	Tests inspect generated getter guards, verify setters remain unguarded, and execute chained getters on empty resources without a panic.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TestGeneratedModels
  participant ChainOnEmpty
  participant GeneratedGetter
  TestGeneratedModels->>ChainOnEmpty: Execute chained getter test
  ChainOnEmpty->>GeneratedGetter: Call getters on nil intermediate values
  GeneratedGetter-->>ChainOnEmpty: Return nil without panic

Loading

Suggested reviewers: jcogilvie, phisco, tampakrap

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: generated Go getters now handle nil receivers safely, which is the primary objective of this PR.
Description check	✅ Passed	The description clearly explains the problem, solution, testing approach, and compatibility considerations, all directly related to the changeset.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Breaking Changes	✅ Passed	The breaking changes check applies only to files under 'apis/' or 'cmd/'. This PR modifies only 'internal/schemas/generator/accessors.go' and 'internal/schemas/generator/accessors_test.go', whi...
Feature Gate Requirement	✅ Passed	PR modifies implementation of existing feature already gated behind generateGoModelAccessors flag; no new experimental feature added.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

internal/schemas/generator/accessors_test.go (1)

419-438: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use table cases that assert the complete nil branch.

Thanks for adding coverage for nil receivers. Convert these checks into cases with reason, args, and want fields. Include return zero in the expected scalar and fixed-array branches. The current checks pass if a getter declares zero but returns a different value.

As per path instructions, **/*_test.go requires a table-driven test structure with args/want, case naming, and reason fields.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/schemas/generator/accessors_test.go` around lines 419 - 438, The
nil-receiver assertions in the accessor test should become table-driven cases
with case names plus reason, args, and want fields. Update the cases for GetBar,
GetCount, and GetFixed to compare the complete generated nil branch, explicitly
requiring return nil for the nilable field and var zero followed by return zero
for scalar and fixed-array fields; retain the SetBar guard assertion.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/schemas/generator/accessors_test.go`:
- Around line 419-438: The nil-receiver assertions in the accessor test should
become table-driven cases with case names plus reason, args, and want fields.
Update the cases for GetBar, GetCount, and GetFixed to compare the complete
generated nil branch, explicitly requiring return nil for the nilable field and
var zero followed by return zero for scalar and fixed-array fields; retain the
SetBar guard assertion.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 44608a88-35aa-4e42-b63b-f5081f06005b

📥 Commits

Reviewing files that changed from the base of the PR and between 8df0f92 and 992a1b3.

📒 Files selected for processing (2)

• internal/schemas/generator/accessors.go
• internal/schemas/generator/accessors_test.go

[adamwg]

One question, but assuming my assumption is correct, this lgtm. This will definitely help the ergonomics of the Go bindings 🙏

[adamwg]

Are the receivers guaranteed to be nil-able? I think they are because we mark every openapi field as optional, so they all become pointers - is that right?

[erikmiller-gusto]

yah