[darcs-devel] [patch1850] introduce TestOnly class

[Ben Franksen]

Ben Franksen <ben.franksen at online.de> added the comment:

(I was a bit astonished that defining a nullary type class requires MultiParamTypeClasses, though there is a certain logic to that...)

My only comment wrt the general idea is that I see no way to use this technique to flag export of data constructors: it works only if we add dedicated functions that aren't used internally. This can become a bit heavy-weight especially for GADTs.

Here is a completely different idea: we add WARNING pragmas. I just tested this for RepoPatchV3: In src/Darcs/Patch/V3/Core.hs I added 

{-# WARNING Prim, Conflictor, Rotcilfnoc "Data constructors of RepoPatchV3 are only exported for testing" #-}

Then I get warnings like this:

harness/Darcs/Test/Patch/Properties/RepoPatchV3.hs:49:28: warning: [-Wdeprecations]
    In the use of data constructor ‘Conflictor’
    (imported from Darcs.Patch.V3.Core):
    "Data constructors of RepoPatchV3 are only exported for testing"
   |
49 | prop_consistentConflictor (Conflictor _ x p)
   |                            ^^^^^^^^^^

If I add

  ghc-options:      -Wno-warnings-deprecations

in darcs.cabal in the test-suite section, then all is fine.

The same trick could be used for PrimPatchId instead of adding unsafePrimPatchId. We could streamline the warning message to e.g. "TEST CODE ONLY", since ghc already gives us the source location and highlights the identifier for us.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1850>
__________________________________