Skip to content

Commit bbd9096

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 bbd9096

File tree

3 files changed

+59
-0
lines changed

3 files changed

+59
-0
lines changed

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: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
module TestTraits
2+
3+
using AbstractTrees
4+
using Test
5+
6+
function we_own_the_module(m::Module)
7+
ret = false
8+
while m != Main
9+
if m == AbstractTrees
10+
ret = true
11+
break
12+
end
13+
m = parentmodule(m)
14+
end
15+
ret
16+
end
17+
18+
function we_own_the_method(m::Method)
19+
we_own_the_module(parentmodule(m))
20+
end
21+
22+
const traits = (
23+
ParentLinks, SiblingLinks, ChildIndexing, childrentype, childtype, AbstractTrees.childstatetype, NodeType, nodetype,
24+
)
25+
26+
struct T end
27+
28+
for func traits
29+
f = Symbol(func)
30+
@eval begin
31+
function AbstractTrees.$f(::Type{<:T})
32+
# This method should not ever get called, it just serves to test dispatch/type piracy.
33+
throw(ArgumentError("this is not the method you're looking for"))
34+
end
35+
end
36+
end
37+
38+
@testset "Traits" begin
39+
@testset "traits should not make dependents vulnerable to commiting type piracy" begin
40+
@testset "func: $func" for func traits
41+
@test all(we_own_the_method, methods(func, Tuple{Type{Union{}}}))
42+
end
43+
end
44+
end
45+
46+
end

0 commit comments

Comments
 (0)