Skip to content

Commit

Permalink
Provide nice error messages when constructing record fields (#83)
Browse files Browse the repository at this point in the history 
* Provide nice error messages when constructing record fields

* Set project version to 0.5.8

* Show alternative exception when types match

* Julia 1.6 compatibility

* Simplify error message

* Allow Compat version 4

* Quote field names

* Handle parametric conversion

* Misspelling

* Use human-readable parametric variables
  • Loading branch information
@omus
omus authored Apr 12, 2023
1 parent 61c700a commit f0a9c27
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 14 deletions.
6 changes: 4 additions & 2 deletions Project.toml
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name = "Legolas"
uuid = "741b9549-f6ed-4911-9fbf-4a1c0c97f0cd"
authors = ["Beacon Biosignals, Inc."]
version = "0.5.7"
version = "0.5.8"

[deps]
Arrow = "69666777-d1a9-59fb-9406-91d4454c9d45"
Expand All @@ -10,14 +10,16 @@ UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"

[compat]
Arrow = "2"
Compat = "3.34, 4"
DataFrames = "1"
Tables = "1.4"
julia = "1.6"

[extras]
Compat = "34da2185-b29b-5c13-b0c7-acf172513d20"
DataFrames = "a93c6f00-e57d-5684-b7b6-d8193f3e46c0"
Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
UUIDs = "cf7118a7-6976-5b1a-9a39-7adc72f591a4"

[targets]
test = ["Test", "DataFrames", "UUIDs"]
test = ["Compat", "DataFrames", "Test", "UUIDs"]
10 changes: 5 additions & 5 deletions examples/tour.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ fields_with_x = (; fields..., x="x")
foo = FooV1(; a=1.0, b="hi", d=[1, 2, 3])
@test isequal(NamedTuple(foo), (a=1.0, b="hi", c=missing, d=[1, 2, 3]))

# Providing the non-compliantly-typed field `d::Int`, inducing a `MethodError`:
@test_throws MethodError FooV1(; a=1.0, b="hi", d=2)
# Providing the non-compliantly-typed field `d::Int`, inducing an `ArgumentError`:
@test_throws ArgumentError FooV1(; a=1.0, b="hi", d=2)

# Implicitly providing the non-compliantly-typed field `d::Missing`, inducing a `MethodError`:
@test_throws MethodError FooV1(; a=1.0, b="hi")
# Implicitly providing the non-compliantly-typed field `d::Missing`, inducing an `ArgumentError`:
@test_throws ArgumentError FooV1(; a=1.0, b="hi")

#####
##### Custom Field Assignments
Expand Down Expand Up @@ -243,7 +243,7 @@ end
# assignments before applying the child's field assignments. Notice how `BazV1` applies the
# constraints/transformations of both `example.baz@1` and `example.bar@1`:
@test NamedTuple(BazV1(; x=200, y=:hi)) == (x=127, y="hi", z="hi_127", k=6)
@test_throws MethodError BazV1(; y=:hi) # `example.baz@1` does not allow `x::Missing`
@test_throws ArgumentError BazV1(; y=:hi) # `example.baz@1` does not allow `x::Missing`

# `BazV1`'s inner constructor definition is roughly equivalent to:
#
Expand Down
54 changes: 49 additions & 5 deletions src/schemas.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -477,21 +477,60 @@ function _generate_record_type_definitions(schema_version::SchemaVersion, record
names_of_parameterized_fields = Symbol[]
field_definitions = Expr[]
field_assignments = Expr[]
parametric_field_assignments = Expr[]
for (fname, ftype) in pairs(record_fields)
fsym = Base.Meta.quot(fname)
T = Base.Meta.quot(ftype)
fdef = :($fname::$T)
info = get(declared_field_infos, fname, nothing)
if !isnothing(info)
fcatch = quote
if $fname isa $(info.type)
throw(ArgumentError("Invalid value set for field `$($fsym)` ($(repr($(fname))))"))
else
throw(ArgumentError("Invalid value set for field `$($fsym)`, expected $($(info.type)), got a value of type $(typeof($fname)) ($(repr($(fname))))"))
end
end
if info.parameterize
T = gensym(string(fname, "_T"))
# As we disallow the use of fields which start with an underscore then the
# following parameter should not conflict with any user defined fields.
# Additionally, as these are static parameters users will not be able to
# overwrite them in custom field assignments.
T = Symbol('_', join(titlecase.(split(string(fname), '_'))))
push!(type_param_defs, :($T <: $(info.type)))
push!(names_of_parameterized_fields, fname)
fdef = :($fname::$T)
fstmt = :($fname = $(info.statement.args[2]))
fstmt = quote
try
$fname = $(info.statement.args[2])
catch
$fcatch
end
if !($fname isa $(info.type))
throw(TypeError($(Base.Meta.quot(record_type_symbol)), "field `$($fsym)`", $(info.type), $fname))
end
end
fstmt_par = quote
try
$fname = $(info.statement.args[2])
$fname = convert($T, $fname)::$T
catch
$fcatch
end
end
else
fstmt = :($fname = convert($T, $(info.statement.args[2]))::$T)
fstmt = quote
try
$fname = $(info.statement.args[2])
$fname = convert($T, $fname)::$T
catch
$fcatch
end
end
fstmt_par = fstmt
end
push!(field_assignments, fstmt)
push!(parametric_field_assignments, fstmt_par)
end
push!(field_definitions, fdef)
end
Expand Down Expand Up @@ -527,7 +566,7 @@ function _generate_record_type_definitions(schema_version::SchemaVersion, record
inner_constructor_definitions = quote
function $R{$(type_param_names...)}(; $(field_kwargs...)) where {$(type_param_names...)}
$parent_record_application
$(field_assignments...)
$(parametric_field_assignments...)
return new{$(type_param_names...)}($(keys(record_fields)...))
end
function $R(; $(field_kwargs...))
Expand Down Expand Up @@ -703,7 +742,12 @@ macro version(record_type, required_fields_block)
end
end
if !allunique(f.name for f in required_field_infos)
msg = string("cannot have duplicate field names in `@version` declaration; recieved: ", [f.name for f in required_field_infos])
msg = string("cannot have duplicate field names in `@version` declaration; received: ", [f.name for f in required_field_infos])
return :(throw(SchemaVersionDeclarationError($msg)))
end
invalid_field_names = filter!(fname -> startswith(string(fname), '_'), [f.name for f in required_field_infos])
if !isempty(invalid_field_names)
msg = string("cannot have field name which start with an underscore in `@version` declaration: ", invalid_field_names)
return :(throw(SchemaVersionDeclarationError($msg)))
end
declared_field_names_types = Expr(:tuple, (:($(f.name) = $(esc(f.type))) for f in required_field_infos)...)
Expand Down
117 changes: 115 additions & 2 deletions test/runtests.jl
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Compat: current_exceptions
using Legolas, Test, DataFrames, Arrow, UUIDs
using Legolas: SchemaVersion, @schema, @version, SchemaVersionDeclarationError, RequiredFieldInfo

Expand Down Expand Up @@ -265,7 +266,8 @@ end
@test_throws SchemaVersionDeclarationError("provided record type symbol is malformed: ChildVTwo") @version(ChildVTwo > ParentV2, begin x end)
@test_throws SchemaVersionDeclarationError("provided record type expression is malformed: BobV1 > DaveV1 > JoeV1") @version(BobV1 > DaveV1 > JoeV1, begin x end)
@test_throws SchemaVersionDeclarationError("provided record type expression is malformed: BobV1 < DaveV1") @version(BobV1 < DaveV1, begin x end)
@test_throws SchemaVersionDeclarationError("cannot have duplicate field names in `@version` declaration; recieved: $([:x, :y, :x, :z])") @version(ChildV2, begin x; y; x; z end)
@test_throws SchemaVersionDeclarationError("cannot have duplicate field names in `@version` declaration; received: $([:x, :y, :x, :z])") @version(ChildV2, begin x; y; x; z end)
@test_throws SchemaVersionDeclarationError("cannot have field name which start with an underscore in `@version` declaration: $([:_X])") @version(ChildV2, begin x; X; _X end)
@test_throws SchemaVersionDeclarationError("cannot extend from another version of the same schema") @version(ChildV2 > ChildV1, begin x end)
@test_throws SchemaVersionDeclarationError("declared field types violate parent's field types") @version(NewV1 > ParentV1, begin y::Int end)
@test_throws SchemaVersionDeclarationError("declared field types violate parent's field types") @version(NewV1 > ChildV1, begin y::Int end)
Expand Down Expand Up @@ -411,7 +413,7 @@ end
@test typeof(ParamV1{Int}(; i=1.0)) === ParamV1{Int}
@test_throws TypeError ParamV1{Float64}(; i=1)
@test_throws TypeError ParamV1(; i=1.0)
@test_throws InexactError ParamV1{Int}(; i=1.1)
@test_throws ArgumentError ParamV1{Int}(; i=1.1)
end
end

Expand Down Expand Up @@ -490,3 +492,114 @@ end

c = ConstrainedFieldV1(field = "1")
@test c.field == 1

#####
##### record error reporting
#####

@schema "test.field-error" FieldError

function _validate(x)
x in ("a", "b", "c") || throw(ArgumentError("Must be a, b, or c"))
return x
end

@version FieldErrorV1 begin
a::Union{String,Missing} = Legolas.lift(_validate, a)
b::(<:Union{String,Missing}) = Legolas.lift(_validate, b)
c::Union{Integer,Missing}
d::(<:Union{Integer,Missing})
end

_num_calls = Ref{Int}(0)
@version FieldErrorV2 begin
a::Integer = begin
_num_calls[] += 1
a isa Function ? a() : a
end
end

@version FieldErrorV3 begin
a::Integer = replace(a, ' ' => '-')
end

@testset "Legolas record constructor error handling" begin
@testset "field constructor error" begin
ex_stack = try
FieldErrorV1(; a="invalid")
catch
current_exceptions()
end

@test length(ex_stack) == 2
@test sprint(showerror, ex_stack[1].exception) == "ArgumentError: Must be a, b, or c"
@test sprint(showerror, ex_stack[2].exception) == "ArgumentError: Invalid value set for field `a` (\"invalid\")"

ex_stack = try
FieldErrorV1(; b="invalid")
catch
current_exceptions()
end

@test length(ex_stack) == 2
@test sprint(showerror, ex_stack[1].exception) == "ArgumentError: Must be a, b, or c"
@test sprint(showerror, ex_stack[2].exception) == "ArgumentError: Invalid value set for field `b` (\"invalid\")"

ex_stack = try
FieldErrorV1(; c="3")
catch
current_exceptions()
end

@test length(ex_stack) == 2
@test startswith(sprint(showerror, ex_stack[1].exception), "MethodError: Cannot `convert` an object of type String to an object of type Integer")
@test sprint(showerror, ex_stack[2].exception) == "ArgumentError: Invalid value set for field `c`, expected Union{Missing, Integer}, got a value of type String (\"3\")"

ex_stack = try
FieldErrorV1(; d=4.0)
catch
current_exceptions()
end

@test length(ex_stack) == 1
@test sprint(showerror, ex_stack[1].exception) == "TypeError: in FieldErrorV1, in field `d`, expected Union{Missing, Integer}, got a value of type Float64"

ex_stack = try
FieldErrorV1{Missing,Int}(; d=4.5)
catch
current_exceptions()
end

@test length(ex_stack) == 2
@test typeof(ex_stack[1].exception) == InexactError
@test sprint(showerror, ex_stack[2].exception) == "ArgumentError: Invalid value set for field `d`, expected Union{Missing, Integer}, got a value of type Float64 (4.5)"

ex_stack = try
FieldErrorV1{AbstractString,Int}
catch
current_exceptions()
end

@test length(ex_stack) == 1
@test contains(sprint(showerror, ex_stack[1].exception), r"TypeError: in FieldErrorV1, in _B, expected _B<:Union{Missing, String}, got Type{AbstractString}")
end

@testset "one-time evaluation" begin
_num_calls[] = 0
FieldErrorV2(; a=1.0)
@test _num_calls[] == 1

_num_calls[] = 0
@test_throws ArgumentError FieldErrorV2(; a=() -> error("foo"))
@test _num_calls[] == 1

_num_calls[] = 0
@test_throws ArgumentError FieldErrorV2(; a="a")
@test _num_calls[] == 1
end

@testset "reports modifications" begin
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
end

2 comments on commit f0a9c27

@omus
Copy link
Member Author

@omus omus commented on f0a9c27 Apr 12, 2023

Choose a reason for hiding this comment

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

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

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

Registration pull request created: JuliaRegistries/General/81504

After the above pull request is merged, it is recommended that a tag is created on this repository for the registered package version.

This will be done automatically if the Julia TagBot GitHub Action is installed, or can be done manually through the github interface, or via:

git tag -a v0.5.8 -m "<description of version>" f0a9c279365a44be2ba0e68276b589360bf7ffc3
git push origin v0.5.8

Please sign in to comment.