Skip to content

Commit af7eee8

Browse files
committed
guard against trait type piracy in a dependent package
Prevent dependent packages from commiting type piracy easily and unintentionally when defining an AbstractTrees trait for all subtypes of a type. Specifically: the bottom type, `Union{}`, is a subtype of each type, so add a method for the bottom type for each AbstractTrees trait. This method will take precedence over sane methods in dependent packages, thus preventing spurious type piracy.
1 parent 5f09fa5 commit af7eee8

File tree

4 files changed

+90
-0
lines changed

4 files changed

+90
-0
lines changed

src/iteration.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,10 @@ abstract type TreeIterator{T} end
6363
_iterator_eltype(::NodeTypeUnknown) = EltypeUnknown()
6464
_iterator_eltype(::HasNodeType) = HasEltype()
6565

66+
Base.IteratorEltype(::Type{<:TreeIterator{Union{}}}) = throw(not_supported_exc)
6667
Base.IteratorEltype(::Type{<:TreeIterator{T}}) where {T} = _iterator_eltype(NodeType(T))
6768

69+
Base.eltype(::Type{<:TreeIterator{Union{}}}) = throw(not_supported_exc)
6870
Base.eltype(::Type{<:TreeIterator{T}}) where {T} = nodetype(T)
6971
Base.eltype(ti::TreeIterator) = eltype(typeof(ti))
7072

src/traits.jl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
const not_supported_exc = ArgumentError("not supported")
12

23
"""
34
ParentLinks(::Type{T})
@@ -42,6 +43,7 @@ the tree structure and cannot be inferred through a single node.
4243
"""
4344
struct ImplicitParents <: ParentLinks; end
4445

46+
ParentLinks(::Type{Union{}}) = throw(not_supported_exc)
4547
ParentLinks(::Type) = ImplicitParents()
4648
ParentLinks(tree) = ParentLinks(typeof(tree))
4749

@@ -84,6 +86,7 @@ from the tree structure.
8486
"""
8587
struct ImplicitSiblings <: SiblingLinks; end
8688

89+
SiblingLinks(::Type{Union{}}) = throw(not_supported_exc)
8790
SiblingLinks(::Type) = ImplicitSiblings()
8891
SiblingLinks(tree) = SiblingLinks(typeof(tree))
8992

@@ -126,6 +129,7 @@ class of indexable trees consisting of arrays.
126129
"""
127130
struct NonIndexedChildren <: ChildIndexing end
128131

132+
ChildIndexing(::Type{Union{}}) = throw(not_supported_exc)
129133
ChildIndexing(::Type) = NonIndexedChildren()
130134
ChildIndexing(node) = ChildIndexing(typeof(node))
131135

@@ -143,6 +147,7 @@ If the `childrentype` can be inferred from the type of the node alone, the type
143147
**OPTIONAL**: In most cases, [`childtype`](@ref) is used instead. If `childtype` is not defined it will fall back
144148
to `eltype ∘ childrentype`.
145149
"""
150+
childrentype(::Type{Union{}}) = throw(not_supported_exc)
146151
childrentype(::Type{T}) where {T} = Base._return_type(children, Tuple{T})
147152
childrentype(node) = typeof(children(node))
148153

@@ -159,6 +164,7 @@ If `childtype` can be inferred from the type of the node alone, the type `::Type
159164
can be type-stable. If `childrentype` is defined and can be known from the node type alone, this function will
160165
fall back to `eltype(childrentype(T))`. If this gives a correct result it's not necessary to define `childtype`.
161166
"""
167+
childtype(::Type{Union{}}) = throw(not_supported_exc)
162168
childtype(::Type{T}) where {T} = eltype(childrentype(T))
163169
childtype(node) = eltype(childrentype(node))
164170

@@ -172,6 +178,7 @@ traversal is type stable.
172178
173179
**OPTIONAL**: Type inference is used to attempt to
174180
"""
181+
childstatetype(::Type{Union{}}) = throw(not_supported_exc)
175182
childstatetype(::Type{T}) where {T} = Iterators.approx_iter_type(childrentype(T))
176183
childstatetype(node) = childstatetype(typeof(node))
177184

@@ -204,6 +211,7 @@ type.
204211
"""
205212
struct NodeTypeUnknown <: NodeType end
206213

214+
NodeType(::Type{Union{}}) = throw(not_supported_exc)
207215
NodeType(::Type) = NodeTypeUnknown()
208216
NodeType(node) = NodeType(typeof(node))
209217

@@ -214,5 +222,6 @@ NodeType(node) = NodeType(typeof(node))
214222
Returns a type which must be a parent type of all nodes in the tree connected to `node`. This can be used to,
215223
for example, specify the `eltype` of any `TreeIterator` on `node`.
216224
"""
225+
nodetype(::Type{Union{}}) = throw(not_supported_exc)
217226
nodetype(::Type) = Any
218227
nodetype(node) = nodetype(typeof(node))

test/runtests.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
using AbstractTrees, Test
22
using Aqua
33

4+
if Base.VERSION >= v"1.9"
5+
# tests use `parentmodule(::Method)`, only supported on v1.9 and up
6+
@testset "Traits" begin include("traits.jl") end
7+
end
48
@testset "Builtins" begin include("builtins.jl") end
59
@testset "Custom tree types" begin include("trees.jl") end
610
if Base.VERSION >= v"1.6"

test/traits.jl

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
module TestTraits
2+
3+
using AbstractTrees
4+
using Test
5+
6+
function is_owned_by(m::Module, n::Module)
7+
ret = false
8+
while m != Main
9+
if m == n
10+
ret = true
11+
break
12+
end
13+
m = parentmodule(m)
14+
end
15+
ret
16+
end
17+
18+
function is_owned_by(m::Method, n::Module)
19+
is_owned_by(parentmodule(m), n)
20+
end
21+
22+
function we_own_the_method(m::Method)
23+
is_owned_by(m, AbstractTrees)
24+
end
25+
26+
const traits = (
27+
ParentLinks, SiblingLinks, ChildIndexing, childrentype, childtype, AbstractTrees.childstatetype, NodeType, nodetype,
28+
)
29+
30+
const base_traits = (
31+
eltype, Base.IteratorEltype,
32+
)
33+
34+
struct T end
35+
36+
for func traits
37+
f = nameof(func)
38+
@eval begin
39+
function AbstractTrees.$f(::Type{<:T})
40+
# This method should not ever get called, it just serves to test dispatch/type piracy.
41+
throw(ArgumentError("this is not the method you're looking for"))
42+
end
43+
end
44+
end
45+
46+
for func base_traits
47+
f = nameof(func)
48+
@eval begin
49+
function Base.$f(::Type{<:AbstractTrees.TreeIterator{<:T}})
50+
# This method should not ever get called, it just serves to test dispatch/type piracy.
51+
throw(ArgumentError("this is not the method you're looking for"))
52+
end
53+
end
54+
end
55+
56+
@testset "Traits" begin
57+
@testset "traits should not make dependents vulnerable to commiting type piracy" begin
58+
@testset "AbstractTrees traits" begin
59+
@testset "func: $func" for func traits
60+
arg = Union{}
61+
@test_throws Exception func(arg)
62+
@test all(we_own_the_method, methods(func, Tuple{Type{arg}}))
63+
end
64+
end
65+
@testset "Base traits" begin
66+
@testset "func: $func" for func base_traits
67+
arg = AbstractTrees.TreeIterator{Union{}}
68+
@test_throws Exception func(arg)
69+
@test all(we_own_the_method, methods(func, Tuple{Type{arg}}))
70+
end
71+
end
72+
end
73+
end
74+
75+
end

0 commit comments

Comments
 (0)