Skip to content

Commit

Permalink
TOML: fix converted tabular data printing (JuliaLang#41009)
Browse files Browse the repository at this point in the history
Previously the printing pass for converted data only works for
non-tabular data (like simple number literals), and it doesn't work for
`Dict` or `Array`s. Rather it leads to runtime error because we don't
pass over the same `by` keyword  argument through recursive calls and it
may not be assigned:
```julia
julia> struct MyStruct
           a::Int
       end

julia> data = Dict(:foo => MyStruct(1))
Dict{Symbol, MyStruct} with 1 entry:
  :foo => MyStruct(1)

julia> TOML.print(data; softed=true) do x
           x isa MyStruct && return Dict(:bar => x.a)
       end
ERROR: MethodError: no method matching print(::var"JuliaLang#38#39", ::Dict{Symbol, MyStruct}; softed=true)
You may have intended to import Base.print
Closest candidates are:
  print(::Union{Nothing, Function}, ::AbstractDict; sorted, by) at /Users/aviatesk/julia/julia/stdlib/TOML/src/print.jl:130 got unsupported keyword argument "softed"
  print(::Union{Nothing, Function}, ::IO, ::AbstractDict; sorted, by) at /Users/aviatesk/julia/julia/stdlib/TOML/src/print.jl:129 got unsupported keyword argument "softed"
  print(::IO, ::AbstractDict; sorted, by) at /Users/aviatesk/julia/julia/stdlib/TOML/src/print.jl:131 got unsupported keyword argument "softed"
Stacktrace:
 [1] kwerr(::NamedTuple{(:softed,), Tuple{Bool}}, ::Function, ::Function, ::Dict{Symbol, MyStruct})
   @ Base ./error.jl:163
 [2] top-level scope
   @ none:1
```

<details><summary>Originally reported by JET:</summary>
julia> using JET, Pkg

julia> @report_call Pkg.project()
═════ 3 possible errors found ═════
┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/API.jl:103 Pkg.API.EnvCache()
│┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/Types.jl:285 #self#(Pkg.Types.nothing)
││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/Types.jl:288 Pkg.Types.read_project(project_file)
│││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/project.jl:138 Pkg.Types.sprint(Pkg.Types.showerror, e)
││││┌ @ strings/io.jl:106 Base.#sprint#412(Core.tuple(Base.nothing, 0, #self#, f), args...)
│││││┌ @ strings/io.jl:112 f(Core.tuple(s), args...)
││││││┌ @ toml_parser.jl:326 Base.TOML.point_to_line(Base.getproperty(err, :str), pos, pos, io)
│││││││ for 1 of union split cases, no matching method found for call signatures (Tuple{typeof(Base.TOML.point_to_line), Nothing, Int64, Int64, IOBuffer})): Base.TOML.point_to_line(Base.getproperty(err::Base.TOML.ParserError, :str::Symbol)::Union{Nothing, String}, pos::Int64, pos::Int64, io::IOBuffer)
││││││└──────────────────────
│││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/project.jl:142 Pkg.Types.Project(raw)
││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/project.jl:124 Base.setproperty!(project, :targets, Pkg.Types.read_project_targets(Pkg.Types.get(raw, "targets", Pkg.Types.nothing), project))
│││││┌ @ Base.jl:35 Base.convert(Base.fieldtype(Base.typeof(x), f), v)
││││││┌ @ abstractdict.jl:523 _(x)
│││││││┌ @ dict.jl:104 Base.setindex!(h, v, k)
││││││││┌ @ dict.jl:382 Base.convert(_, v0)
│││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/LinearAlgebra/src/factorization.jl:58 _(f)
││││││││││ no matching method found for call signature (Tuple{Type{Vector{String}}, LinearAlgebra.Factorization}): _::Type{Vector{String}}(f::LinearAlgebra.Factorization)
│││││││││└─────────────────────────────────────────────────────────────────────────────────────────────────
││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/Types.jl:305 Pkg.Types.write_env_usage(manifest_file, "manifest_usage.toml")
│││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/Types.jl:436 Pkg.Types.sprint(JuliaLang#35)
││││┌ @ strings/io.jl:106 Base.#sprint#412(Core.tuple(Base.nothing, 0, #self#, f), args...)
│││││┌ @ strings/io.jl:112 f(Core.tuple(s), args...)
││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/Pkg/src/Types.jl:437 TOML.print(io, Pkg.Types.Dict(Pkg.Types.=>(Core.getfield(#self#, :source_file), Base.vect(Pkg.Types.Dict(Pkg.Types.=>("time", Pkg.Types.now()))))))
│││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:131 TOML.Internals.Printer.#print#16(false, TOML.Internals.Printer.identity, #self#, io, a)
││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:131 Core.kwfunc(TOML.Internals.Printer._print)(Core.apply_type(Core.NamedTuple, (:sorted, :by))(Core.tuple(sorted, by)), TOML.Internals.Printer._print, TOML.Internals.Printer.nothing, io, a)
│││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:76 #s848(_2, _3, f, io, a, Base.getindex(TOML.Internals.Printer.String))
││││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:76 TOML.Internals.Printer.#_print#11(indent, first_block, sorted, by, _3, f, io, a, ks)
│││││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:88 Core.kwfunc(TOML.Internals.Printer.printvalue)(Core.apply_type(Core.NamedTuple, (:sorted,))(Core.tuple(sorted)), TOML.Internals.Printer.printvalue, f, io, value)
││││││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:25 TOML.Internals.Printer.#printvalue#1(sorted, _3, f, io, value)
│││││││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:29 Core.kwfunc(TOML.Internals.Printer._print)(Core.apply_type(Core.NamedTuple, (:sorted,))(Core.tuple(sorted)), TOML.Internals.Printer._print, f, io, x)
││││││││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:76 #s848(_2, _3, f, io, a, Base.getindex(TOML.Internals.Printer.String))
│││││││││││││││┌ @ /Users/aviatesk/julia/julia/usr/share/julia/stdlib/v1.7/TOML/src/print.jl:76 Core.throw(Core.UndefKeywordError(:by))
││││││││││││││││ UndefKeywordError: keyword argument by not assigned
│││││││││││││││└────────────────────────────────────────────────────────────────────────────────
Pkg.API.ProjectInfo
</details>

With this PR, everything should work:
> After
```julia
julia> TOML.print(data; sorted=true) do x
           x isa MyStruct && return Dict(:bar => x.a)
       end
[foo]
bar = 1
```
  • Loading branch information
aviatesk committed May 30, 2021
1 parent 2280efe commit 311ff56
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 32 deletions.
69 changes: 37 additions & 32 deletions stdlib/TOML/src/print.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import Dates

import Base: @invokelatest
import ..isvalid_barekey_char

function printkey(io::IO, keys::Vector{String})
Expand All @@ -20,46 +21,36 @@ function printkey(io::IO, keys::Vector{String})
end

const MbyFunc = Union{Function, Nothing}
const TOMLValue = Union{AbstractVector, AbstractDict, Dates.DateTime, Dates.Time, Dates.Date, Bool, Integer, AbstractFloat, String}
function printvalue(f::MbyFunc, io::IO, value::AbstractVector; sorted=false)
const TOMLValue = Union{AbstractVector, AbstractDict, Dates.DateTime, Dates.Time, Dates.Date, Bool, Integer, AbstractFloat, AbstractString}
function printvalue(f::MbyFunc, io::IO, value::AbstractVector; sorted=false, by=identity)
Base.print(io, "[")
for (i, x) in enumerate(value)
i != 1 && Base.print(io, ", ")
if isa(x, AbstractDict)
_print(f, io, x; sorted)
_print(f, io, x; sorted, by)
else
printvalue(f, io, x; sorted)
printvalue(f, io, x; sorted, by)
end
end
Base.print(io, "]")
end
function printvalue(f::MbyFunc, io::IO, value; sorted)
if f === nothing
error("type `$(typeof(value))` is not a valid TOML type, pass a conversion function to `TOML.print`")
end
toml_value = f(value)
if !(toml_value isa TOMLValue)
error("TOML syntax function for type `$(typeof(value))` did not return a valid TOML type but a `$(typeof(toml_value))`")
end
Base.invokelatest(printvalue, f, io, toml_value; sorted)
end
printvalue(f::MbyFunc, io::IO, value::AbstractDict; sorted) =
_print(f, io, value; sorted)
printvalue(f::MbyFunc, io::IO, value::Dates.DateTime; sorted) =
printvalue(f::MbyFunc, io::IO, value::AbstractDict; sorted=false, by=identity) =
_print(f, io, value; sorted, by)
printvalue(f::MbyFunc, io::IO, value::Dates.DateTime; _...) =
Base.print(io, Dates.format(value, Dates.dateformat"YYYY-mm-dd\THH:MM:SS.sss\Z"))
printvalue(f::MbyFunc, io::IO, value::Dates.Time; sorted) =
printvalue(f::MbyFunc, io::IO, value::Dates.Time; _...) =
Base.print(io, Dates.format(value, Dates.dateformat"HH:MM:SS.sss"))
printvalue(f::MbyFunc, io::IO, value::Dates.Date; sorted) =
printvalue(f::MbyFunc, io::IO, value::Dates.Date; _...) =
Base.print(io, Dates.format(value, Dates.dateformat"YYYY-mm-dd"))
printvalue(f::MbyFunc, io::IO, value::Bool; sorted) =
printvalue(f::MbyFunc, io::IO, value::Bool; _...) =
Base.print(io, value ? "true" : "false")
printvalue(f::MbyFunc, io::IO, value::Integer; sorted) =
printvalue(f::MbyFunc, io::IO, value::Integer; _...) =
Base.print(io, Int64(value)) # TOML specifies 64-bit signed long range for integer
printvalue(f::MbyFunc, io::IO, value::AbstractFloat; sorted) =
printvalue(f::MbyFunc, io::IO, value::AbstractFloat; _...) =
Base.print(io, isnan(value) ? "nan" :
isinf(value) ? string(value > 0 ? "+" : "-", "inf") :
Float64(value)) # TOML specifies IEEE 754 binary64 for float
printvalue(f::MbyFunc, io::IO, value::AbstractString; sorted) = Base.print(io, "\"", escape_string(value), "\"")
printvalue(f::MbyFunc, io::IO, value::AbstractString; _...) = Base.print(io, "\"", escape_string(value), "\"")

is_table(value) = isa(value, AbstractDict)
is_array_of_tables(value) = isa(value, AbstractArray) &&
Expand All @@ -70,8 +61,8 @@ function _print(f::MbyFunc, io::IO, a::AbstractDict,
ks::Vector{String} = String[];
indent::Int = 0,
first_block::Bool = true,
sorted::Bool,
by::Function,
sorted::Bool = false,
by::Function = identity,
)
akeys = keys(a)
if sorted
Expand All @@ -82,11 +73,25 @@ function _print(f::MbyFunc, io::IO, a::AbstractDict,
for key in akeys
value = a[key]
is_tabular(value) && continue
Base.print(io, ' '^4max(0,indent-1))
printkey(io, [String(key)])
Base.print(io, " = ") # print separator
printvalue(f, io, value; sorted)
Base.print(io, "\n") # new line?
if !isa(value, TOMLValue)
if f === nothing
error("type `$(typeof(value))` is not a valid TOML type, pass a conversion function to `TOML.print`")
end
toml_value = f(value)
if !(toml_value isa TOMLValue)
error("TOML syntax function for type `$(typeof(value))` did not return a valid TOML type but a `$(typeof(toml_value))`")
end
value = toml_value
end
if is_tabular(value)
_print(f, io, Dict(key => value); indent, first_block, sorted, by)
else
Base.print(io, ' '^4max(0,indent-1))
printkey(io, [String(key)])
Base.print(io, " = ") # print separator
printvalue(f, io, value; sorted, by)
Base.print(io, "\n") # new line?
end
first_block = false
end

Expand All @@ -105,7 +110,7 @@ function _print(f::MbyFunc, io::IO, a::AbstractDict,
Base.print(io,"]\n")
end
# Use runtime dispatch here since the type of value seems not to be enforced other than as AbstractDict
Base.invokelatest(_print, f, io, value, ks; indent = indent + header, first_block = header, sorted, by)
@invokelatest _print(f, io, value, ks; indent = indent + header, first_block = header, sorted, by)
pop!(ks)
elseif is_array_of_tables(value)
# print array of tables
Expand All @@ -119,7 +124,7 @@ function _print(f::MbyFunc, io::IO, a::AbstractDict,
Base.print(io,"]]\n")
# TODO, nicer error here
!isa(v, AbstractDict) && error("array should contain only tables")
Base.invokelatest(_print, f, io, v, ks; indent = indent + 1, sorted, by)
@invokelatest _print(f, io, v, ks; indent = indent + 1, sorted, by)
end
pop!(ks)
end
Expand Down
22 changes: 22 additions & 0 deletions stdlib/TOML/test/print.jl
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,34 @@ struct MyStruct
a::Int
end
@test_throws ErrorException toml_str(Dict("foo" => MyStruct(1)))
# simple value
@test toml_str(Dict("foo" => MyStruct(1))) do x
x isa MyStruct && return x.a
end == """
foo = 1
"""

# tabular values
@test toml_str(Dict("foo" => MyStruct(1)); sorted=true) do x
x isa MyStruct && return [x.a]
end == """
foo = [1]
"""
@test toml_str(Dict("foo" => MyStruct(1)); sorted=true) do x
x isa MyStruct && return Dict(:bar => x.a)
end == """
[foo]
bar = 1
"""

# validation against the usual case
@test toml_str(Dict("foo" => MyStruct(1)); sorted=true) do x
x isa MyStruct && return [x.a]
end == toml_str(Dict("foo" => [1]); sorted=true)
@test toml_str(Dict("foo" => MyStruct(1)); sorted=true) do x
x isa MyStruct && return Dict(:bar => x.a)
end == toml_str(Dict("foo" => Dict(:bar => 1)); sorted=true)

@test toml_str(Dict("b" => SubString("foo"))) == "b = \"foo\"\n"

@testset "empty dict print" begin
Expand Down

0 comments on commit 311ff56

Please sign in to comment.