Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add check for all missing arguments to schema. #124

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+68 −5
Open

Add check for all missing arguments to schema. #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
dbccd7e
working `all-missing` check
haberdashPI Sep 5, 2024
9895a37
fix 1.6 compat issue
haberdashPI Sep 10, 2024
064bb0b
wip
haberdashPI Sep 11, 2024
e5b6417
allow explicit `missing` arguments to be passed without warning
haberdashPI Sep 11, 2024
b6522fa
more tests / support for empty empty `(;)`
haberdashPI Sep 12, 2024
9fbdaac
fix some quoting issues, improve test coverage
haberdashPI Sep 12, 2024
dab96ea
fix `NamedTuple` constructor
haberdashPI Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 42 additions & 5 deletions src/schemas.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,31 @@ end

_schema_version_from_record_type(::Nothing) = nothing

struct DefaultArgument end
is_default(::Any) = false
is_default(::DefaultArgument) = true
default_to(::DefaultArgument, default) = default
default_to(x, ::Any) = x

abstract type CheckArgsFlag end
struct CheckArgs <: CheckArgsFlag end
struct DontCheckArgs <: CheckArgsFlag end

function _check_empty_args(::DontCheckArgs, _, args...)
return default_to.(args, missing)
end

function _check_empty_args(::CheckArgs, schema_name, args...)
if all(is_default, args)
@warn "The were no arguments passed to `$schema_name`. "*
"Are you sure you don't mean `$(schema_name)SchemaVersion`?"*
" If this is what you really meant you can call "*
"`$schema_name(; arg1=missing, arg2=...)` to avoid this warning."*
" Future (breaking) versions of Legolas may make this warning an error."
end
return default_to.(args, missing)
end

# Note also that this function's implementation is allowed to "observe" `Legolas.declared_fields(parent)`
# (if a parent exists), but is NOT allowed to "observe" `Legolas.declaration(parent)`, since the latter
# includes the parent's declared field RHS statements. We cannot interpolate/incorporate these statements
Expand Down Expand Up @@ -592,7 +617,7 @@ function _generate_record_type_definitions(schema_version::SchemaVersion, record
end

# generate `parent_record_application`
field_kwargs = [Expr(:kw, n, :missing) for n in keys(record_fields)]
field_kwargs = [Expr(:kw, n, :($Legolas.DefaultArgument())) for n in keys(record_fields)]
parent_record_application = nothing
parent = Legolas.parent(schema_version)
if !isnothing(parent)
Expand All @@ -607,13 +632,20 @@ function _generate_record_type_definitions(schema_version::SchemaVersion, record

# generate `inner_constructor_definitions` and `outer_constructor_definitions`
R = record_type_symbol
kwargs_from_row = [Expr(:kw, n, :(get(row, $(Base.Meta.quot(n)), missing))) for n in keys(record_fields)]
kwargs_from_row = [Expr(:kw, n, :(get(row, $(Base.Meta.quot(n)), $Legolas.DefaultArgument()))) for n in keys(record_fields)]
outer_constructor_definitions = quote
$R(::NamedTuple{(), Tuple{}}) = $R($Legolas.DontCheckArgs())
$R(row) = $R(; $(kwargs_from_row...))
end
result = gensym("result")
if isempty(type_param_defs)
inner_constructor_definitions = quote
function $R(; $(field_kwargs...))
# IMPLEMENTATION NOTE: `$R(; $(field_kwargs...))` overlaps with `$R()` so if we
# tried to implement the warning for empty arguments by defining `$R()` we would
# be defining the same method twice.
function $R(flag::$Legolas.CheckArgsFlag=$Legolas.CheckArgs(); $(field_kwargs...))
$(result) = $Legolas._check_empty_args(flag, $(string(R)), $(keys(record_fields)...))
$((:($key = $(result)[$i]) for (i, key) in enumerate(keys(record_fields)))...)
$parent_record_application
$(field_assignments...)
$(constraints...)
Expand All @@ -623,13 +655,17 @@ function _generate_record_type_definitions(schema_version::SchemaVersion, record
else
type_param_names = [p.args[1] for p in type_param_defs]
inner_constructor_definitions = quote
function $R{$(type_param_names...)}(; $(field_kwargs...)) where {$(type_param_names...)}
function $R{$(type_param_names...)}(flag::$Legolas.CheckArgsFlag=$Legolas.CheckArgs(); $(field_kwargs...)) where {$(type_param_names...)}
$(result) = $Legolas._check_empty_args(flag, $(string(R)), $(keys(record_fields)...))
$((:($key = $(result)[$i]) for (i, key) in enumerate(keys(record_fields)))...)
$parent_record_application
$(parametric_field_assignments...)
$(constraints...)
return new{$(type_param_names...)}($(keys(record_fields)...))
end
function $R(; $(field_kwargs...))
function $R(flag::$Legolas.CheckArgsFlag=$Legolas.CheckArgs(); $(field_kwargs...))
$(result) = $Legolas._check_empty_args(flag, $(string(R)), $(keys(record_fields)...))
$((:($key = $(result)[$i]) for (i, key) in enumerate(keys(record_fields)))...)
$parent_record_application
$(field_assignments...)
$(constraints...)
Expand All @@ -638,6 +674,7 @@ function _generate_record_type_definitions(schema_version::SchemaVersion, record
end
outer_constructor_definitions = quote
$outer_constructor_definitions
$R{$(type_param_names...)}(::NamedTuple) where {$(type_param_names...)} = $R{$(type_param_names...)}($Legolas.DontCheckArgs())
$R{$(type_param_names...)}(row) where {$(type_param_names...)} = $R{$(type_param_names...)}(; $(kwargs_from_row...))
end
end
Expand Down
26 changes: 26 additions & 0 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,10 @@ end
a::Integer = replace(a, ' ' => '-')
end

@version FieldErrorV4 begin
x::Union{String,Missing}
end

@testset "Legolas record constructor error handling" begin
@testset "field constructor error" begin
ex_stack = try
Expand Down Expand Up @@ -770,6 +774,28 @@ end
e = ArgumentError("Invalid value set for field `a`, expected Integer, got a value of type String (\"foo-bar\")")
@test_throws e FieldErrorV3(; a="foo bar")
end

@testset "appropriate warnings with zero-argument constructor" begin
msg = r"no arguments passed.*FieldErrorV.*are you sure.*FieldErrorV(1|4)SchemaVersion"i

@test_logs (:warn, msg) FieldErrorV1()
@test_logs FieldErrorV1(; a=missing)
@test_logs FieldErrorV1((;))

@test_logs (:warn, msg) FieldErrorV4()
@test_logs FieldErrorV4(; x=missing)
@test_logs FieldErrorV4((;))

@test_logs (:warn, msg) begin
@test_throws ArgumentError FieldErrorV1{String,Int}()
end
@test_logs begin
@test_throws ArgumentError FieldErrorV1{String,Int}(; a=missing)
end
@test_logs begin
@test_throws ArgumentError FieldErrorV1{String,Int}((;))
end
end
end

#####
Expand Down
Loading