Skip to content

fix(accessors): honor map slice defaults - #192

Merged
hanzei merged 2 commits into
stretchr:masterfrom
terminalchai:fix/map-slice-default
Oct 24, 2026
Merged

hanzei merged 2 commits into
stretchr:masterfrom
terminalchai:fix/map-slice-default

Conversation

@terminalchai

@terminalchai terminalchai commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

ObjxMapSlice(default) currently returns nil instead of the supplied default when an []any contains a non-map element. This also occurs with mixed arrays decoded from JSON. Honor the existing single-default convention in that failure branch, matching the accessor's documented behavior.

The JSON regression and two invalid-element cases cover the fix. Valid conversions and generated files remain unchanged. Full race-enabled tests pass, and CI-pinned golangci-lint 2.14.0 reports zero issues in an equivalent LF checkout.

AI tools assisted with implementation and independent automated code review.

@hanzei hanzei left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the fix is correct and needed.

One optional suggestion: the tests are heavier than a 3-line fix needs.

  • TestObjxMapSliceDefaultFromJSON covers the regression well by itself.
  • TestObjxMapSliceInvalidElementDefaults runs 18 subtests with 7 assertions each, including pointer-identity and no-mutation checks. One or two invalid-element cases would be enough.
  • TestObjxMapSliceValidInputsWithDefault only covers behaviour this PR doesn't change.

Trimming these down to the JSON test plus a couple of invalid-element cases would keep the diff small, but it isn't blocking.

@hanzei hanzei left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hanzei
hanzei merged commit 7d1f603 into stretchr:master Oct 24, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants