Fix crash encoding non-map data for struct types - #192
Merged
Merged
Conversation
to_json/4's #sp_map{} clause had no is_map/1 guard, unlike the
sibling #sp_list{} (is_list) and #sp_rec{} (is_tuple) clauses. For a
struct type this called maps:get('__struct__', Data, undefined) on
non-map Data, raising badmap instead of returning a type_mismatch
error like every other type already does.
Also drops map_to_json/4's now-unreachable fallback clause, since
its only caller guarantees is_map(Data).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The fix is covered by regression tests and all reviewed changes are ready for approval.
Pull request overview
Fixes a crash when encoding non-map values against struct JSON types by returning structured type-mismatch errors.
Changes:
- Guards struct map encoding with
is_map/1. - Removes unreachable fallback logic.
- Adds regression coverage and changelog documentation.
File summaries
| File | Description |
|---|---|
test/elixir_struct_test.erl |
Tests non-map struct encoding errors. |
src/spectra_json.erl |
Guards map encoding and simplifies the helper. |
CHANGELOG.md |
Documents the fix. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
extract_struct_name/1 detects "struct" map types purely from a literal '__struct__' field in the type's abstract syntax, regardless of source language. The crash fixed in 1c3af00 was therefore reachable without any Elixir module or runtime involved, and reachable one level deeper too: list_to_json/4 recurses into the same unguarded clause for each element of a list of such types. struct_shaped_map_types.erl defines the type in plain Erlang, and struct_shaped_map_test.erl exercises it through spectra:encode/5 only (no Elixir), covering both the top-level and list-nested cases. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
andreashasse
marked this pull request as ready for review
September 14, 2026 09:30
The '__struct__' convention only exists to interoperate with Elixir structs -- nothing about ordinary Erlang code would ever produce a map type shaped that way on its own. Writing an Erlang type that mimics one doesn't demonstrate a real Erlang-only trigger for the bug, just a contrived one, so it doesn't add coverage worth keeping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
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.
Summary
to_json/4's#sp_map{}clause insrc/spectra_json.erlhad nois_map/1guard, unlike the sibling#sp_list{}(is_list) and#sp_rec{}(is_tuple) clauses above it.struct_name =/= undefined), this calledmaps:get('__struct__', Data, undefined)directly on non-mapData, raising a rawbadmaperror instead of reaching the catch-all clause that already returns a proper{error, [type_mismatch]}for every other type.from_json) were all unaffected —map_from_json/4already guardsis_map(Json)unconditionally, andmap_to_json/4had its ownis_map(Data)guard with a fallback for the non-struct case.when is_map(Data)to the#sp_map{}clause into_json/4, so non-map input for struct types now falls through to the existing catch-alltype_mismatchclause.map_to_json/4's ownis_map/1guard and fallback clause became unreachable as a result (its only caller now guarantees a map), so they were removed.Test plan
to_json_non_map_data_returns_error_testintest/elixir_struct_test.erl, encoding a struct type with a string, integer, list, and atom, asserting each returns{error, [#sp_error{type = type_mismatch}]}instead of crashing.{badmap, Data}crash when the guard is reverted, and passes with the fix.rebar3 eunit(757 tests, 0 failures),rebar3 dialyzer(clean, including the dead-code warning surfaced by removingmap_to_json/4's dead fallback),rebar3 xref,rebar3 fmt --check,rebar3 hank,rebar3 check_app_calls,rebar3 cover, andrebar3 proper(9/9 properties passed).🤖 Generated with Claude Code
https://claude.ai/code/session_01WK6VcSwrRa5XLKUrEhEi2T
Generated by Claude Code