test_unbound_args: add a functionality to ignore specific methods

[arnaud-ma]

With this PR, it is now possible to ignore some methods, given their signature. For example, taking the false positive in #86:

module abcd

f(::NTuple{N, T}) where {N,T} = (N,T)
f(::Tuple{}) = (0,Any)

end # module

It is possible to disable the false positive with:

Aqua.test_unbound_args(abcd, ignore = [(abcd.f, NTuple)])

This specific example has been added in the unit tests.

To improve flexibility, the possibility to call directly Aqua.test_unbound_args(unbounds_args) where unbounds_args is a collection of methods (probably got with Test.detect_unbound_args) is also added.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.86%. Comparing base (1fca5d9) to head (df69299).
Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #316      +/-   ##
==========================================
- Coverage   74.90%   74.86%   -0.04%     
==========================================
  Files          11       11              
  Lines         761      764       +3     
==========================================
+ Hits          570      572       +2     
- Misses        191      192       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cgarling]

The ability to ignore specific methods when testing for unbound arguments would be helpful.

[lgoettgens]

Thanks for working on this. Adding exceptions is indeed something that each of the test functions should have. Some detailed comments below

[lgoettgens]

This approach unfortunately does not work for callable objects.

Please make sure that this also works for cases like

struct S{U}
  s::U
end

(::S{U})(::NTuple{N,T}) where {N,T,U} = (N,T,U)
(::S{U})(::Tuple{}) where {U} = (0,Any,U)

and include such an example in the tests

[arnaud-ma]

All done!

[lgoettgens]

Hmm, I thinks this still isn't quite what we want here.
For custom structs, say struct S end, there are both function signatures containing S and Type{S} (which isn't even quite typeof(S)), since the former is for callable objects and the latter for constructors.

Furthermore, there are subtypes of Function, that are not a singleton type of function xxx, e.g. Base.ComposedFunction.

Maybe you have an idea that makes all of these cases work. If not, I think we just need the user to specify the exact signature, i.e. foo(x::Int, y::Float64) as (typeof(foo), Int, Float64).

[arnaud-ma]

Ah, I understand now. Thank you very much for your patience. After some research, I can't find any way to be 100% sure that we are dealing with a "simple" method.

But specifying the exact signature requires some knowledge from the user. To help the user, I think adding what method.sig returns in the error message can be a good idea. For example from this:

Unbound type parameters detected:
[1] f(x) where T @ Main.Foo ~/projets/forks/Aqua/Aqua.jl/a.jl:15
[2] (::Main.Foo.S{U})(::NTuple{N, T}) where {N, T, U} @ Main.Foo ~/projets/forks/Aqua/Aqua.jl/a.jl:8
[3] Main.Foo.S(::NTuple{N, T}) where {N, T} @ Main.Foo ~/projets/forks/Aqua/Aqua.jl/a.jl:11

to this:

Unbound type parameters detected:

[1] f(x) where T @ Main.Foo ~/projets/forks/Aqua/Aqua.jl/a.jl:15
    signature: Tuple{typeof(Main.Foo.f), Any} where T

[2] (::Main.Foo.S{U})(::NTuple{N, T}) where {N, T, U} @ Main.Foo ~/projets/forks/Aqua/Aqua.jl/a.jl:8
    signature: Tuple{Main.Foo.S{U}, NTuple{N, T}} where {N, T, U}

[3] Main.Foo.S(::NTuple{N, T}) where {N, T} @ Main.Foo ~/projets/forks/Aqua/Aqua.jl/a.jl:11
    signature: Tuple{Type{Main.Foo.S}, NTuple{N, T}} where {N, T}

What do you think about that?

[lgoettgens]

I am not a fan of requiring 3x of vertical space. Instead, I think we could make a function public that returns the list of methods, so that a user can query this for the signature themselves. But I would put that to a follow-up PR.
For this PR, it would be great if you could do the small adaptions needed for this, and adapt the docstring as well.

[jlapeyre]

If you are willing to rewrite your code, this may be a better workaround:
https://discourse.julialang.org/t/is-this-test-detect-unbound-args-result-valid-or-a-bug/96987

The Aqua documentation on test_unbound_args might also mention this. The fix recommended in the docs is not applicable in most situations.

julia> f(::Tuple{T, Vararg{T, NmOne}}) where {T, NmOne} = (NmOne + 1 ,T);

julia> f((1,2,3))
(3, Int64)

julia> import Test;

julia> Test.detect_unbound_args(Main)
0-element Vector{Method}