Refactor getdocs code (#1842)

Co-authored-by: Morten Piibeleht <morten.piibeleht@gmail.com> (cherry picked from commit 373590a)
JuliaDocs · Jul 6, 2022 · 87ac790 · 87ac790
1 parent 9775246
commit 87ac790
Show file tree

Hide file tree

Showing 3 changed files with 171 additions and 30 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,8 @@
   - highlight.js has been updated from `v11.0.1` to `v11.5.1`.
   - KaTeX has been updated from `v0.13.11` to `v0.13.24`.
 
+* ![Bugfix][badge-bugfix] When including docstrings for an alias, Documenter now correctly tries to include the exactly matching docstring first, before checking for signature subtypes. ([#1842][github-1842])
+
 ## Version `v0.27.19`
 
 * ![Enhancement][badge-enhancement] Documenter can now build draft version of HTML documentation by passing `draft=true` to `makedocs`. Draft mode skips potentially expensive parts of the building process and can be useful to get faster feedback when writing documentation. Draft mode currently skips doctests, `@example`-, `@repl`-, `@eval`-, and `@setup`-blocks. Draft mode can be disabled (or enabled) on a per-page basis by setting `Draft = true` in an `@meta` block. ([#1836][github-1836])
@@ -1061,6 +1063,7 @@
 [github-1834]: https://github.com/JuliaDocs/Documenter.jl/pull/1834
 [github-1836]: https://github.com/JuliaDocs/Documenter.jl/pull/1836
 [github-1838]: https://github.com/JuliaDocs/Documenter.jl/pull/1838
+[github-1842]: https://github.com/JuliaDocs/Documenter.jl/pull/1842
 [github-1844]: https://github.com/JuliaDocs/Documenter.jl/pull/1844
 [github-1846]: https://github.com/JuliaDocs/Documenter.jl/pull/1846
 <!-- end of issue link definitions -->

diff --git a/src/DocSystem.jl b/src/DocSystem.jl
@@ -162,23 +162,19 @@ const CACHED = IdDict{Any,Any}()
 """
 $(SIGNATURES)
 
-Find all `DocStr` objects that match the provided arguments:
-
+Find all `DocStr` objects that match the provided arguments exactly.
 - `binding`: the name of the object.
 - `typesig`: the signature of the object. Default: `Union{}`.
-- `compare`: how to compare signatures? Exact (`==`) or subtypes (`<:`). Default: `<:`.
+- `compare`: how to compare signatures? (`==` (default), `<:` or `>:`)
 - `modules`: which modules to search through. Default: *all modules*.
-- `aliases`: check aliases of `binding` when nothing is found. Default: `true`.
 
-Returns a `Vector{DocStr}` ordered by definition order in `0.5` and by
-`type_morespecific` in `0.4`.
+Return a `Vector{DocStr}` ordered by definition order.
 """
-function getdocs(
+function getspecificdocs(
         binding::Docs.Binding,
-        typesig::Type = Union{};
+        typesig::Type = Union{},
         compare = (==),
         modules = Docs.modules,
-        aliases = true,
     )
     # Fall back to searching all modules if user provides no modules.
     modules = isempty(modules) ? Docs.modules : modules
@@ -200,28 +196,44 @@ function getdocs(
             end
         end
     end
-    if compare == (==)
-        # Exact matching of signatures:
-        #
-        # When we get a single match from using `==` as the comparision then we just return
-        # that one result.
-        #
-        # Otherwise we fallback to comparing signatures using `<:` to match, hopefully, a
-        # wider range of possible docstrings.
-        if length(results) == 1
-            results
-        else
-            getdocs(binding, typesig; compare = (<:), modules = modules, aliases = aliases)
-        end
-    else
-        # When nothing is found we check whether the `binding` is an alias of some other
-        # `Binding`. If so then we redo the search using that `Binding` instead.
-        if aliases && isempty(results) && (b = aliasof(binding)) != binding
-            getdocs(b, typesig; compare = compare, modules = modules)
-        else
-            results
+    results
+end
+
+"""
+$(SIGNATURES)
+
+Find all `DocStr` objects that somehow match the provided arguments.
+That is, if [`getspecificdocs`](@ref) fails, get docs for aliases of
+`binding` (unless `aliases` is set to `false). For `compare` being `==` also
+try getting docs for `<:`.
+"""
+function getdocs(
+        binding::Docs.Binding,
+        typesig::Type = Union{};
+        compare = (==),
+        modules = Docs.modules,
+        aliases = true,
+    )
+    # First, we try to find the docs that _exactly_ match the binding. If you
+    # have aliases, you can have a separate docstring attached to the alias.
+    results = getspecificdocs(binding, typesig, compare, modules)
+    # If we don't find anything, we'll loosen the function signature comparison
+    # to allow for subtypes, to find any signatures that would get called in
+    # dispatch for this method (i.e. supertype signatures).
+    if isempty(results) && compare == (==)
+        results = getspecificdocs(binding, typesig, (<:), modules)
+    end
+    # If we still can't find anything, `aliases` is set, and this binding is
+    # indeed an alias, we'll fetch the docstrings for the original object, first
+    # as an exact match (if relevant) and then also falling back to a subtype
+    # search.
+    if isempty(results) && aliases && (b = aliasof(binding)) != binding
+        results = getspecificdocs(b, typesig, compare, modules)
+        if isempty(results) && compare == (==)
+            results = getspecificdocs(b, typesig, (<:), modules)
         end
     end
+    results
 end
 
 """

diff --git a/test/docsystem.jl b/test/docsystem.jl
@@ -6,6 +6,29 @@ import Documenter: Documenter, DocSystem
 
 const alias_of_getdocs = DocSystem.getdocs # NOTE: won't get docstrings if in a @testset
 
+module TestDocstrings
+    "A"
+    struct A end
+    "A(x)"
+    A(x) = A()
+    B = A
+
+    "foo(::Number)"
+    foo(::Number) = nothing
+
+    "foo(::Float64)"
+    foo(::Float64) = nothing
+
+    const bar = foo
+    const baz = foo
+
+    "baz(::Number)"
+    baz(::Number)
+
+    "baz(::Float64)"
+    baz(::Float64)
+end
+
 @testset "DocSystem" begin
     ## Bindings.
     @test_throws ArgumentError DocSystem.binding(9000)
@@ -47,14 +70,18 @@ const alias_of_getdocs = DocSystem.getdocs # NOTE: won't get docstrings if in a
         d_2 = DocSystem.getdocs(b, Union{Tuple{Any}, Tuple{Any, Type}}; compare = (==)),
         d_3 = DocSystem.getdocs(b; modules = Module[Main]),
         d_4 = DocSystem.getdocs(DocSystem.binding(@__MODULE__, :alias_of_getdocs)),
-        d_5 = DocSystem.getdocs(DocSystem.binding(@__MODULE__, :alias_of_getdocs); aliases = false)
+        d_5 = DocSystem.getdocs(DocSystem.binding(@__MODULE__, :alias_of_getdocs); aliases = false),
+        d_6 = DocSystem.getdocs(b, Union{Tuple{Docs.Binding}, Tuple{Docs.Binding, Type}}; compare = (==)),
+        d_7 = DocSystem.getdocs(DocSystem.binding(@__MODULE__, :alias_of_getdocs), Union{Tuple{Docs.Binding}, Tuple{Docs.Binding, Type}})
 
         @test length(d_0) == 0
         @test length(d_1) == 2
         @test length(d_2) == 1
         @test length(d_3) == 0
         @test length(d_4) == 2
         @test length(d_5) == 0
+        @test length(d_6) == 1
+        @test length(d_7) == 1
 
         @test d_1[1].data[:binding] == b
         @test d_1[2].data[:binding] == b
@@ -67,14 +94,113 @@ const alias_of_getdocs = DocSystem.getdocs # NOTE: won't get docstrings if in a
         @test d_2[1].data[:typesig] == Union{Tuple{Any}, Tuple{Any, Type}}
         @test d_2[1].data[:module]  == DocSystem
 
+        @test d_6[1].data[:binding] == b
+        @test d_6[1].data[:typesig] == Union{Tuple{Docs.Binding}, Tuple{Docs.Binding, Type}}
+        @test d_6[1].data[:module]  == DocSystem
+
         @test d_1 == d_4
         @test d_1 != d_5
+        @test d_6 == d_7
     end
 
     ## `UnionAll`
     let b = DocSystem.binding(@__MODULE__, Meta.parse("f(x::T) where T"))
         @test b.var == :f
     end
+
+    # TestDocstrings
+    a_1 = DocSystem.getdocs(Docs.Binding(TestDocstrings, :A))
+    a_2 = DocSystem.getdocs(Docs.Binding(TestDocstrings, :A), Union{})
+    a_3 = DocSystem.getdocs(Docs.Binding(TestDocstrings, :A), Tuple{Any})
+    b_1 = DocSystem.getdocs(Docs.Binding(TestDocstrings, :B))
+    b_2 = DocSystem.getdocs(Docs.Binding(TestDocstrings, :B), Union{})
+    b_3 = DocSystem.getdocs(Docs.Binding(TestDocstrings, :B), Tuple{Any})
+    @test length(a_2) == 1
+    @test a_2[1].data[:typesig] == Union{}
+    # No signature fetches the docstring of the type (Union{}) in this case
+    @test a_1 == a_2
+    @test length(a_3) == 1
+    @test a_3[1].data[:typesig] == Tuple{Any}
+    # Make sure that for an alias we get consistent docstrings
+    @test b_1 == a_1
+    @test b_2 == a_2
+    @test b_3 == a_3
+
+    # Tests for method and alias fallback logic
+    foo_1 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :foo))
+    foo_2 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :foo), Tuple{Int})
+    foo_3 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :foo), Tuple{Float64})
+    foo_4 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :foo), Tuple{AbstractFloat})
+    foo_5 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :foo), Tuple{Number})
+    foo_6 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :foo), Tuple{Any})
+
+    @test length(foo_1) == 2 # should have fetched both docstrings
+
+    @test length(foo_5) == 1 # contains docstring for generic ::Number method
+    @test foo_5[1].data[:binding] == Docs.Binding(TestDocstrings, :foo)
+    @test foo_5[1].data[:typesig] == Tuple{Number}
+
+    @test isempty(foo_6) # this shouldn't match anything
+    @test foo_2 == foo_5 # ::Int dispatches to ::Number
+    @test foo_4 == foo_5 # ::AbstractFloat also dispatches to ::Number
+
+    @test foo_3 != foo_5 # foo(::Float64) has its own docstring
+    @test foo_3[1].data[:binding] == Docs.Binding(TestDocstrings, :foo)
+    @test foo_3[1].data[:typesig] == Tuple{Float64}
+
+    @test foo_2[1] ∈ foo_1
+    @test foo_3[1] ∈ foo_1
+
+    # setting 'compare' to subtype, will fetch both docstrings
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :foo), Tuple{Float64}, compare = (<:)) == foo_1
+
+    # bar is an alias, so falls back to foo
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar)) == foo_1
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Int}) == foo_2
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Float64}) == foo_3
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{AbstractFloat}) == foo_4
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Number}) == foo_5
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Any}) == foo_6
+    # unless we disable following aliases
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar); aliases = false) |> isempty
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Int}; aliases = false) |> isempty
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Float64}; aliases = false) |> isempty
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{AbstractFloat}; aliases = false) |> isempty
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Number}; aliases = false) |> isempty
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :bar), Tuple{Any}; aliases = false) |> isempty
+
+    # baz, while an alias of foo, has the same 'structure', but different docstrings..
+    baz_1 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz))
+    baz_2 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Int})
+    baz_3 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Float64})
+    baz_4 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{AbstractFloat})
+    baz_5 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Number})
+    baz_6 = Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Any})
+
+    @test length(baz_1) == 2 # should have fetched both docstrings
+
+    @test length(baz_5) == 1 # contains docstring for generic ::Number method
+    @test baz_5[1].data[:binding] == Docs.Binding(TestDocstrings, :baz)
+    @test baz_5[1].data[:typesig] == Tuple{Number}
+
+    @test isempty(baz_6) # this shouldn't match anything
+    @test baz_2 == baz_5 # ::Int dispatches to ::Number
+    @test baz_4 == baz_5 # ::AbstractFloat also dispatches to ::Number
+
+    @test baz_3 != baz_5 # baz(::Float64) has its own docstring
+    @test baz_3[1].data[:binding] == Docs.Binding(TestDocstrings, :baz)
+    @test baz_3[1].data[:typesig] == Tuple{Float64}
+
+    @test baz_2[1] ∈ baz_1
+    @test baz_3[1] ∈ baz_1
+
+    # .. even if we disable aliases
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz); aliases = false) == baz_1
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Int}; aliases = false) == baz_2
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Float64}; aliases = false) == baz_3
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{AbstractFloat}; aliases = false) == baz_4
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Number}; aliases = false) == baz_5
+    @test Documenter.DocSystem.getdocs(Docs.Binding(TestDocstrings, :baz), Tuple{Any}; aliases = false) == baz_6
 end
 
 end