Skip to content

Conversation

@quinnj
Copy link
Member

@quinnj quinnj commented Jan 14, 2026

Summary

  • Fix @omit_null and @omit_empty macros to work correctly with parametric types
  • Previously, @omit_null struct Foo{T} ... end generated JSON.omit_null(::Type{Foo{T}}) = true which only matched when the exact type parameter matched
  • Now generates JSON.omit_null(::Type{<:Foo}) = true which correctly matches all instantiations (e.g., Foo{Int}, Foo{String}, etc.)

Test plan

  • All existing tests pass
  • Manual verification with parametric types:
    @omit_null struct MyParametric{T}
        value::Union{T,Nothing}
    end
    JSON.json(MyParametric(1))  # should omit null fields for any T

🤖 Generated with Claude Code

Previously, for parametric types like `struct Foo{T}`, the macros
generated `JSON.omit_null(::Type{Foo{T}}) = true` which only matched
when the exact type parameter matched. Now they generate
`JSON.omit_null(::Type{<:Foo}) = true` which correctly matches all
instantiations (e.g., Foo{Int}, Foo{String}, etc.).

Changes:
- Add _make_omit_type_sig helper to generate appropriate type signatures
- Modify _extract_type_name to return (base_type, is_parametric) tuple
- Update all call sites in _omit_macro_impl to use new signature format

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@codecov
Copy link

codecov bot commented Jan 14, 2026

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.26%. Comparing base (8d648bd) to head (5c9682f).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/write.jl 83.33% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
- Coverage   90.27%   90.26%   -0.02%     
==========================================
  Files           7        7              
  Lines        1358     1366       +8     
==========================================
+ Hits         1226     1233       +7     
- Misses        132      133       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Tests that the macros work correctly for all type instantiations
when applied to parametric struct definitions (e.g., Foo{Int},
Foo{String} both work when @omit_null struct Foo{T} is used).

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@quinnj quinnj merged commit 7479829 into master Jan 14, 2026
9 of 11 checks passed
@quinnj quinnj deleted the fix-omit-macros-parametric-types branch January 14, 2026 07:27
quinnj referenced this pull request Jan 14, 2026
Co-Authored-By: Claude Opus 4.5 <[email protected]>
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