fix docstring search by signatures

[matthias314]

The problem

I think the signatures generated for docstrings create problems when type parameters are involved. This can be seen when searching docstrings based on signatures. My understanding of the intended behavior is that only docstrings matching the given signature should be displayed or, if none exists, all docstrings for the given function. This works well without type parameters, but not with them. I reported this already in #52669, but I didn’t get any response. The present PR intends to fix this issue. The underlying problem is somewhat subtle, so I try to explain it in detail.

EDIT: The builds fail because a change in doc/src/stdlib/SparseArrays.md is needed, see below.

Examples

help?> filter(iszero)

displays the two docstrings

filter(f, a)
filter(f)

although the first one doesn't match the signature.

help?> NamedTuple([:a => 1, :b => 2])

displays

NamedTuple{names}(args::Tuple)
NamedTuple{names,T}(args::Tuple)
NamedTuple{names}(nt::NamedTuple)
NamedTuple(itr)

although only the last one matches.

What is going wrong?

Here are the signatures used by the help system for the examples above:

julia> using Base.Docs: META, @var, signature

julia> function docsigs(f, M::Module)
           @eval keys(getfield($M, META)[@var $f].docs)
       end;

julia> docsigs(:filter, Base)
KeySet for a IdDict{Any, Any} with 4 entries. Keys:
  Tuple{Any, Base.SkipMissing{<:AbstractArray}}
  Tuple{Any, AbstractDict}
  Union{Tuple{N}, Tuple{T}, Tuple{Any, Array{T, N}}} where {T, N}
  Tuple{Any}

julia> docsigs(:NamedTuple, Base.BaseDocs)
KeySet for a IdDict{Any, Any} with 4 entries. Keys:
  Union{Tuple{Tuple}, Tuple{T}, Tuple{names}} where {names, T}
  Union{Tuple{Tuple}, Tuple{names}} where names
  Tuple{Any}
  Union{Tuple{NamedTuple}, Tuple{names}} where names

The third entry for filter comes from

julia/base/array.jl

Line 2875 in d68a04e

function filter(f, a::Array{T, N}) where {T, N}

as can be seen via

julia> signature(:( filter(f, a::Array{T, N}) where {T, N} ))
:((Union{Tuple{Any, Array{T, N}}, Tuple{N}, Tuple{T}} where N <: Any) where T <: Any)

The type parameters T and N have been added as signatures, which leads to wrong search results.

The first entry for NamedTuple comes from

julia/base/docs/basedocs.jl

Line 3306 in d68a04e

NamedTuple{names,T}(args::Tuple)

Here the same happens with the parameters names and T, even in the absence of a where clause:

julia> signature(:( NamedTuple{names,T}(args::Tuple) ))
:((Union{Tuple{Tuple}, Tuple{T}, Tuple{names}} where T <: Any) where names <: Any)

The problematic code

The signatures attached to docstrings are generated by the function Base.Docs.signature!. PR #21036 added among other things the following statements:

julia/base/docs/Docs.jl

Lines 91 to 96 in f65b8ab

	if isexpr(expr.args[1], :curly) && isempty(tv)
	append!(tv, tvar.(expr.args[1].args[2:end]))
	end
	for i = length(tv):-1:1
	push!(sig.args, :(Tuple{$(tv[i].args[1])}))
	end

The for loop adds type parameters from where clauses as additional signatures, which was the problem for filter. In the absence of where clauses, the if statement does the same with :curly expressions. We have seen this for NamedTuple.

Proposed solution

The PR deletes these two loops. As far as I can tell, searching docstrings by signatures then works as expected. However, docstrings attached to constructors like

NamedTuple{names}(args::Tuple)
NamedTuple{names,T}(args::Tuple)

cannot be distinguished anymore because the signatures of the arguments are the same. Attaching a docstring to the second call replaces the first one. At present they can be distinguished, and this may have been the motivation for #21036.

That having different docstrings for constructors differing only by a type parameter could have been the original motivation is also indicated by two tests in test/docs.jl. One checks that the signatures have the (now problematic) form, and the other one checks that different docstrings can be attached to constructors like the NamedTuple example above. I’ve modified the former test and marked the other one as @test_broken. NamedTuple is the only case where I have seen this problem. (I've compiled Julia and loaded all standard libs.) I've merged the corresponding docstrings into one. I have also changed two lines in the documentation.

Also, docstrings attached to constructor calls with parameters must now use a where clause if the parameter is used for the arguments. For example,

"doc" A{T}(x::T)

is currently allowed, but would have to be written

"doc" A{T}(x::T) where T

For this reason, one needs the following additional change to build the documentation. The file is from a different git repository, see here.

$ diff doc/src/stdlib/SparseArrays.md.orig doc/src/stdlib/SparseArrays.md
240c240
< permute!{Tv, Ti, Tp <: Integer, Tq <: Integer}(::SparseMatrixCSC{Tv,Ti}, ::SparseMatrixCSC{Tv,Ti}, ::AbstractArray{Tp,1}, ::AbstractArray{Tq,1})
---
> permute!{Tv, Ti, Tp, Tq}(::SparseMatrixCSC{Tv,Ti}, ::SparseMatrixCSC{Tv,Ti}, ::AbstractArray{Tp,1}, ::AbstractArray{Tq,1}) where {Tv, Ti, Tp <: Integer, Tq <: Integer}

With this change the documentation builds without problems.

Changing less?

(ADDED) One could avoid the changes just mentioned if one kept part of the code that I delete, namely the lines that treat :curly expressions like where clauses. This is problematic, however, because on the level of expressions free parameters cannot be distinguished from constant parameters. Consider the following example:

struct A{T} end
"doc Int" A{Int}(x::Int) = A{Int}()
"doc T" A{T}(x) where T = A{T}()

The signature for the first docstring would be converted to Tuple{Int} where Int, which is the same as Tuple{Any}. Hence help?> A{Int}(1.0) would display both docstrings although the first one doesn't match. (This is also the current behavior.)

Conclusion

I think that the current situation is not desirable and should be changed. Whether the proposed PR is the right way to go is up for debate. That docstrings can be attached to function calls (instead of method definitions) is not documented, I believe. In this sense one could say that the change made by this PR would be non-breaking. In any case, let me know what you think.