haskell / haddock

Haskell Documentation Tool
www.haskell.org/haddock/
BSD 2-Clause "Simplified" License
361 stars 242 forks source link

Cross-package hyperlinked sources links are broken #1628

Closed bgamari closed 3 months ago

bgamari commented 5 months ago

Consider case like the following:

-- in package `foo`
module Lib (hello) where

-- | Hello world
hello = print "hello"

-- in package `bar`
module Wombat (hello) where
import Lib (hello)

Running cabal haddock bar against a project containing both packages foo and bar will produce faulty documentation.

Specifically, the "Source" link in the generated HTML documentation for Wombat.hello points to something like dist-newstyle/build/x86_64-linux/ghc-9.9.20240122/foo-0.1.0.0/doc/html/foo/src.

The reason for this appears to be that Cabal passes the following to haddock:

--read-interface=file:///.../dist-newstyle/build/x86_64-linux/ghc-9.9.20240122/foo-0.1.0.0/doc/html/foo,file:///.../dist-newstyle/build/x86_64-linux/ghc-9.9.20240122/foo-0.1.0.0/doc/html/foo/src,visible,/.../dist-newstyle/build/x86_64-linux/ghc-9.9.20240122/foo-0.1.0.0/doc/html/foo/foo.haddock

The path used to locate the hyperlinked sources for foo is derived from the second component of this 4-tuple using Haddock.Backends.Xhtml.Utils.spliceURL. However, this function expects this string to be a pattern (e.g. containing the %{MODULE} token), not a path. It is unclear how this could have ever worked.

bgamari commented 5 months ago

I am honestly quite unsure of whether this issue is a bug in Cabal or Haddock; in general the intended behavior of this machinery is quite underspecified. I can see two ways to handle this:

bgamari commented 5 months ago

For instance, the following patch appears to avoid the issue:

diff --git a/haddock-api/src/Haddock/Options.hs b/haddock-api/src/Haddock/Options.hs
index 43c55fdc..27b9a3aa 100644
--- a/haddock-api/src/Haddock/Options.hs
+++ b/haddock-api/src/Haddock/Options.hs
@@ -416,7 +416,7 @@ readIfaceArgs flags = [ parseIfaceOption s | Flag_ReadInterface s <- flags ]
             (src, ',':rest') ->
               let src' = case src of
                     "" -> Nothing
-                    _  -> Just src
+                    _  -> Just (src ++ "/%M.html")
               in
               case break (==',') rest' of
                 (visibility, ',':file) | visibility == "hidden" ->

However, I have not yet convinced myself that this won't break the hyperlinked sources themselves.

bgamari commented 3 months ago

I tested the above fix and merged it in 1432bcc943d41736eca491ecec4eb9a6304dab36.