Fix for nested types #4
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request fixes handling of nested parameterized types in field type checking. The changes improve how the code counts and validates type parameters in struct fields, particularly for complex nested generic types like Vector{Dict{T1,T2}}.
Key changes:
- Updated type parameter counting to handle nested
UnionAlltypes and distinguish between free and constrained parameters - Changed from simple argument counting to recursive parameter analysis
- Replaced debug output (
@show,print) with proper@debugstatements
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| test/runtests.jl | Updated package name references from FullySpecifiedFieldTypesStaticTests to StructFieldParamsTesting |
| test/check_struct_fields_tests.jl | Added comprehensive test cases for nested types (e.g., Vector{Dict{T1,T2}}, Pair{Dict{K, Int64}, String}) |
| src/check_struct_fields.jl | Refactored parameter counting logic to recursively handle nested types and constrained parameters |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function _count_type_expr_params(expr::Expr) | ||
|
|
||
| _count_type_expr_params(mod::Module, @nospecialize(x)) = 0 | ||
| _count_type_expr_params(mod::Module, s::Symbol) = Int(!isdefined(mod, s)) |
There was a problem hiding this comment.
The logic Int(!isdefined(mod, s)) is unclear without explanation. Consider adding a comment explaining why undefined symbols count as 1 parameter while defined symbols count as 0, or using a more explicit conditional expression like isdefined(mod, s) ? 0 : 1 for better readability.
| _count_type_expr_params(mod::Module, s::Symbol) = Int(!isdefined(mod, s)) | |
| # If the symbol is not defined in the module, count it as a free type parameter (1); otherwise, count as 0. | |
| _count_type_expr_params(mod::Module, s::Symbol) = isdefined(mod, s) ? 0 : 1 |
| count += length(expr.args) - 1 | ||
| count = _count_type_expr_params(mod, expr) | ||
| elseif expr.head == :curly | ||
| count += sum(_count_type_expr_params(mod, arg) for arg in expr.args) |
There was a problem hiding this comment.
This sums parameters across all curly brace arguments including the type constructor name (first argument). For example, in Dict{K,V}, this counts parameters in Dict, K, and V. The first argument (type name) should likely be excluded. Consider using expr.args[2:end] instead of expr.args.
| count += sum(_count_type_expr_params(mod, arg) for arg in expr.args) | |
| count += sum(_count_type_expr_params(mod, arg) for arg in expr.args[2:end]) |
No description provided.