XML.jl icon indicating copy to clipboard operation
XML.jl copied to clipboard

Inconsistent type for attributes where nodes have no attributes

Open TimG1964 opened this issue 9 months ago • 2 comments

The assumption in XML.jl seems to be that if a node has no attributes then XML.attributes(node) === nothing.

For example, this constructor seems to assume this:

    function Node(nodetype::NodeType, tag=nothing, attributes=nothing, value=nothing, children=nothing)
        new(nodetype,
            isnothing(tag) ? nothing : string(tag),
            isnothing(attributes) ? nothing : OrderedDict(string(k) => string(v) for (k, v) in pairs(attributes)),
            isnothing(value) ? nothing : string(value),
            isnothing(children) ? nothing :
                children isa Node ? [children] :
                children isa Vector{Node} ? children :
                children isa Vector ? map(Node, children) :
                children isa Tuple ? map(Node, collect(children)) :
                [Node(children)]
        )
    end

and these two functions do, too:

Base.haskey(o::Node, key::AbstractString) = isnothing(o.attributes) ? false : haskey(o.attributes, key)
Base.keys(o::Node) = isnothing(o.attributes) ? () : keys(o.attributes)

The constructor XML.h() relies on the main constructor function above.

However, it is easy to create nodes with no attributes where XML.attributes(node) !== nothing.

julia> using XML

julia> n1 = XML.Element("test1")
Node (depth=1) Element <test1>

julia> XML.attributes(n1)
OrderedCollections.OrderedDict{String, String}()

julia> n2 = XML.h.test2()
Node (depth=1) Element <test2>

julia> XML.attributes(n2)
OrderedCollections.OrderedDict{String, String}()

julia> length(XML.attributes(n1))
0

julia> length(XML.attributes(n2))
0

As a consequence, nodes with no attributes may have different values for XML.attributes(node), and then the test for equality, too, will fail. This is mentioned on Discourse, here.

I have only looked at the Element constructor so far, so I don't know if the other constructors share this issue. I guess they all use the main constructor function, though.

Sorry if I've mis-understood something fundamental! 🙂

TimG1964 avatar Feb 12 '25 10:02 TimG1964