False-positive unbound argument for struct constructors with unions

[staticfloat]

If I have a struct with a union type that contains the only instance of a parametric typevar in it, Aqua complains that my struct's constructor has an unbound typevar:

struct Foo{T}
    x::Union{Nothing,Vector{T}}
end

Results in:

Unbound type parameters: Test Failed at /Users/sabae/.julia/packages/Aqua/9p8ck/src/unbound_args.jl:28
  Expression: isempty(unbounds)
   Evaluated: isempty(Method[Example.Foo(x::Union{Nothing, Vector{T}}) where T @ Example ~/.julia/dev/Example/src/Example.jl:20])

[topolarity]

I think Aqua is complaining that Foo(nothing) results in T being unbound (so that the constructor fails), which is true.

You probably need to separate the Foo(x::Vector) and Foo{T}(::Nothing) constructors:

struct Foo{T}
     x::Union{Nothing,Vector{T}}

     Foo(v::Vector{T}) where T = new{T}(v)
     Foo{T}(v::Vector{T}) where T = new{T}(v)
     Foo{T}(::Nothing) where {T} = new{T}(nothing)
 end

[staticfloat]

Hmmm, that's a little annoying that the default constructor that Julia gives you fails this check though. Too bad.

[topolarity]

Agreed - seems like it might be worth an exception in the Aqua ruleset

[lgoettgens]

Using the example from above, I can easily get a real error (see below), so I think that Aqua reports a genuine issue here.

julia> struct Foo{T}
           x::Union{Nothing,Vector{T}}
       end

julia> Foo(nothing)
ERROR: UndefVarError: `T` not defined
Stacktrace:
 [1] Foo(x::Nothing)
   @ Main ./REPL[1]:2
 [2] top-level scope
   @ REPL[2]:1

[Tortar]

Can something be done in Julia itself to improve this?

[topolarity]

Yes, the error is real but the question is whether it's worth reporting.

This gives an UndefVarError, but if the default constructor were changed to "fix" this, it would still give a MethodError. It's not clear to me why Aqua considers one worth reporting, but not the other.

[Tortar]

but couldn't all needed constructors be created so that all cases work out of the box?

[lgoettgens]

but couldn't all needed constructors be created so that all cases work out of the box?

This would be fixed, if julia wouldn't create inner constructors without explicit type parameters at all. In the example in #265 (comment), I see no way how julia could automatically resolve this. What a user want's can be wastly different based on the context.
If, however, you would only have a constructor with type parameters, i.e. call it as Foo{String}(nothing), this would be fine. But that would be a breaking change in the julia language.

[gdalle]

I am having (what I think is) a similar problem with an NTuple wrapper. Here's the code for MyPackage:

module MyPackage

struct MyStruct{N,T}
    elements::NTuple{N,T}
end

end

and here's the error:

julia> Aqua.test_all(MyPackage)
Skipping Base.active_repl
Skipping Base.active_repl_backend
Test Summary:    | Pass  Total  Time
Method ambiguity |    1      1  1.6s
Unbound type parameters detected:
[1] MyPackage.MyStruct(elements::Tuple{Vararg{T, N}}) where {N, T} @ MyPackage /tmp/MyPackage/src/MyPackage.jl:6
Unbound type parameters: Test Failed at /home/gdalle/.julia/packages/Aqua/tHrmY/src/unbound_args.jl:37
  Expression: isempty(unbounds)
   Evaluated: isempty(Method[MyPackage.MyStruct(elements::Tuple{Vararg{T, N}}) where {N, T} @ MyPackage /tmp/MyPackage/src/MyPackage.jl:6])