"Formatting should be idempotent" test failure with ASCII style and HTTP headers

[sjakobi]

$ cabal test dhall:tasty --test-options "--quickcheck-tests 1000000 -p idempo"
...
    Formatting should be idempotent: FAIL (9.77s)
      *** Failed! Falsified (after 40534 tests and 9 shrinks):
      ASCII
      Header ""
      Embed (Import {importHashed = ImportHashed {hash = Nothing, importType = Remote (URL {scheme = HTTP, authority = "a", path = File {directory = Directory {components = []}, file = ""}, query = Nothing, headers = Just (Pi "_" (Var (V "" 0)) (Var (V "" 0)))})}, importMode = Code})
      "http://a/ using `` \8594 ``\n" /= "http://a/ using `` -> ``\n"
      Use --quickcheck-replay=23582 to reproduce.

[sjakobi]

I suspect that here are two problems at play here:

1. Headers are always formatted in Unicode style, even when the CharacterSet is supposed to be ASCII

   dhall-haskell/dhall/src/Dhall/Pretty/Internal.hs

   Lines 1095 to 1097 in 9d932f5

   	prettyImportExpression :: Pretty a => Expr Src a -> Doc Ann
   	prettyImportExpression (Embed a) =
   	Pretty.pretty a

   dhall-haskell/dhall/src/Dhall/Syntax.hs

   Lines 790 to 799 in 9d932f5

   	instance Pretty URL where
   	pretty (URL {..}) =
   	schemeDoc
   	<> "://"
   	<> Pretty.pretty authority
   	<> pathDoc
   	<> queryDoc
   	<> foldMap prettyHeaders headers
   	where
   	prettyHeaders h = " using " <> Pretty.pretty h

2. The header should be formatted as an import expression: in this case with parentheses.

[sjakobi]

Here's a version with ===:

    Formatting should be idempotent: FAIL (10.81s)
      *** Failed! Falsified (after 45416 tests and 8 shrinks):
      ASCII
      Header ""
      Embed (Import {importHashed = ImportHashed {hash = Nothing, importType = Remote (URL {scheme = HTTPS, authority = "a", path = File {directory = Directory {components = []}, file = ""}, query = Nothing, headers = Just (Equivalent (Var (V "" 0)) (Var (V "" 0)))})}, importMode = Code})
      "https://a/ using `` \8801 ``\n" /= "https://a/ using `` === ``\n"
      Use --quickcheck-replay=662378 to reproduce.

[Gabriella439]

This requires a change to the pretty-printing API to fix. The issue is that Dhall.Pretty.prettyCharacterSet can only format the import using the Pretty instance for Import, since the type of prettyCharacterSet is:

prettyCharacterSet :: Pretty a => CharacterSet -> Expr Src a -> Doc Ann

... and that Pretty instance doesn't permit customizing the CharacterSet

There are only three ways I can think of to fix this:

• Split prettyCharacterSet into two monomorphic functions with the following types:

  ??? :: CharacterSet -> Expr Src Import -> Doc Ann
  ??? :: CharacterSet -> Expr Src Void -> Doc Ann

• Change the Pretty constraint on prettyCharacterSet to a dhall-specific pretty-printing class

  e.g.:

  class PrettyDhall a where
    pretty :: CharacterSet -> a -> Doc Ann
  
  prettyCharacterSet :: PrettyDhall a => CharacterSet -> Expr Src a -> Doc Ann

• Replace the type-class constraint with an explicit function

  prettyCharacterSet :: (CharacterSet -> a -> Doc Ann) -> CharacterSet -> Expr Src a -> Doc Ann

[sjakobi]

@Gabriel439 Thanks for outlining our options! 👍

I was somewhat under the impression that HTTP headers were a rather unloved feature that might end up being removed rather sooner than later. If HTTP headers are removed, it's probably unnecessary to address this issue.

I might be misremembering the status of this feature though?!

[Gabriella439]

Yeah, I'd be fine with removing HTTP headers so long as we explained how to do the same thing using netrc files (although I haven't actually checked yet if netrc files cover the most common use cases)

[sjakobi]

Note: #1951 adds a workaround for this issue to the idempotenceTest which should be removed when this issue is fixed.