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

Improve setproperties error message #19

Merged
merged 3 commits into from
Oct 3, 2019
Merged

Improve setproperties error message #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
90fa8fe
Improve setproperties error message
jw3126 Oct 2, 2019
2a004c1
remove assert_hasfields
jw3126 Oct 2, 2019
9515d4a
test setproperties error messages
jw3126 Oct 3, 2019
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
43 changes: 24 additions & 19 deletions src/ConstructionBase.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,32 +39,37 @@ struct NamedTupleConstructor{names} end
end
end

function assert_hasfields(T, fnames)
for fname in fnames
if !(fname in fieldnames(T))
msg = "$T has no field $fname"
throw(ArgumentError(msg))
end
end
end

function setproperties(obj; kw...)
setproperties(obj, (;kw...))
end

@generated function setproperties(obj, patch::NamedTuple)
assert_hasfields(obj, fieldnames(patch))
args = map(fieldnames(obj)) do fn
if fn in fieldnames(patch)
:(patch.$fn)
else
:(obj.$fn)
if issubset(fieldnames(patch), fieldnames(obj))
args = map(fieldnames(obj)) do fn
if fn in fieldnames(patch)
:(patch.$fn)
else
:(obj.$fn)
end
end
return Expr(:block,
Expr(:meta, :inline),
Expr(:call,:(constructorof($obj)), args...)
)
else
:(setproperties_unknown_field_error(obj, patch))
end
Expr(:block,
Expr(:meta, :inline),
Expr(:call,:(constructorof($obj)), args...)
)
end

function setproperties_unknown_field_error(obj, patch)
O = typeof(obj)
P = typeof(patch)
msg = """
Failed to assign properties $(fieldnames(P)) to object with fields $(fieldnames(O)).
You may want to overload
ConstructionBase.setproperties(obj::$O, patch::NamedTuple)
"""
throw(ArgumentError(msg))
end


Expand Down
14 changes: 12 additions & 2 deletions test/runtests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,18 @@ end
@test setproperties(o, (a=2, b=3.0)) === AB(2,3.0)
@test setproperties(o, a=2, b=3.0) === AB(2,3.0)

@test_throws ArgumentError setproperties(o, (a=2, c=3.0))
@test_throws ArgumentError setproperties(o, a=2, c=3.0)
res = @test_throws ArgumentError setproperties(o, (a=2, this_field_does_not_exist=3.0))
msg = sprint(showerror, res.value)
Copy link
Member Author

@jw3126 jw3126 Oct 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Is accessing the error object like this is considered a public API?

Copy link
Member

Choose a reason for hiding this comment

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

I asked to make it so a while ago JuliaLang/julia#31854

@test occursin("this_field_does_not_exist", msg)
@test occursin("overload", msg)
@test occursin("ConstructionBase.setproperties", msg)

res = @test_throws ArgumentError setproperties(o, a=2, this_field_does_not_exist=3.0)
msg = sprint(showerror, res.value)
@test occursin("this_field_does_not_exist", msg)
@test occursin("overload", msg)
@test occursin("ConstructionBase.setproperties", msg)

@test setproperties(Empty(), NamedTuple()) === Empty()
@test setproperties(Empty()) === Empty()

Expand Down