Overlapping [From,To]JSON instances with aeson-1.5.3.0

[sjakobi]

hnix currently fails to compile with aeson-1.5.3.0:

``` src/Nix/Expr/Types.hs:546:10: error: • Overlapping instances for ToJSON NExpr arising from a use of ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoJSON’ Matching instances: instance ToJSON1 f => ToJSON (Fix f) -- Defined in ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON’ instance ToJSON NExpr -- Defined at src/Nix/Expr/Types.hs:546:10 • In the expression: aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoJSON @(NExpr) In an equation for ‘toJSON’: toJSON = aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoJSON @(NExpr) In the instance declaration for ‘ToJSON NExpr’ | 546 | instance ToJSON NExpr | ^^^^^^^^^^^^ src/Nix/Expr/Types.hs:546:10: error: • Overlapping instances for ToJSON NExpr arising from a use of ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoEncoding’ Matching instances: instance ToJSON1 f => ToJSON (Fix f) -- Defined in ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON’ instance ToJSON NExpr -- Defined at src/Nix/Expr/Types.hs:546:10 • In the expression: aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoEncoding @(NExpr) In an equation for ‘toEncoding’: toEncoding = aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoEncoding @(NExpr) In the instance declaration for ‘ToJSON NExpr’ | 546 | instance ToJSON NExpr | ^^^^^^^^^^^^ src/Nix/Expr/Types.hs:546:10: error: • Overlapping instances for ToJSON NExpr arising from a use of ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoJSONList’ Matching instances: instance ToJSON1 f => ToJSON (Fix f) -- Defined in ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON’ instance ToJSON NExpr -- Defined at src/Nix/Expr/Types.hs:546:10 • In the expression: aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoJSONList @(NExpr) In an equation for ‘toJSONList’: toJSONList = aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoJSONList @(NExpr) In the instance declaration for ‘ToJSON NExpr’ | 546 | instance ToJSON NExpr | ^^^^^^^^^^^^ src/Nix/Expr/Types.hs:546:10: error: • Overlapping instances for ToJSON NExpr arising from a use of ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoEncodingList’ Matching instances: instance ToJSON1 f => ToJSON (Fix f) -- Defined in ‘aeson-1.5.3.0:Data.Aeson.Types.ToJSON’ instance ToJSON NExpr -- Defined at src/Nix/Expr/Types.hs:546:10 • In the expression: aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoEncodingList @(NExpr) In an equation for ‘toEncodingList’: toEncodingList = aeson-1.5.3.0:Data.Aeson.Types.ToJSON.$dmtoEncodingList @(NExpr) In the instance declaration for ‘ToJSON NExpr’ | 546 | instance ToJSON NExpr | ^^^^^^^^^^^^ src/Nix/Expr/Types.hs:561:10: error: • Overlapping instances for FromJSON NExpr arising from a use of ‘aeson-1.5.3.0:Data.Aeson.Types.FromJSON.$dmparseJSON’ Matching instances: instance FromJSON1 f => FromJSON (Fix f) -- Defined in ‘aeson-1.5.3.0:Data.Aeson.Types.FromJSON’ instance FromJSON NExpr -- Defined at src/Nix/Expr/Types.hs:561:10 • In the expression: aeson-1.5.3.0:Data.Aeson.Types.FromJSON.$dmparseJSON @(NExpr) In an equation for ‘parseJSON’: parseJSON = aeson-1.5.3.0:Data.Aeson.Types.FromJSON.$dmparseJSON @(NExpr) In the instance declaration for ‘FromJSON NExpr’ | 561 | instance FromJSON NExpr | ^^^^^^^^^^^^^^ src/Nix/Expr/Types.hs:561:10: error: • Overlapping instances for FromJSON NExpr arising from a use of ‘aeson-1.5.3.0:Data.Aeson.Types.FromJSON.$dmparseJSONList’ Matching instances: instance FromJSON1 f => FromJSON (Fix f) -- Defined in ‘aeson-1.5.3.0:Data.Aeson.Types.FromJSON’ instance FromJSON NExpr -- Defined at src/Nix/Expr/Types.hs:561:10 • In the expression: aeson-1.5.3.0:Data.Aeson.Types.FromJSON.$dmparseJSONList @(NExpr) In an equation for ‘parseJSONList’: parseJSONList = aeson-1.5.3.0:Data.Aeson.Types.FromJSON.$dmparseJSONList @(NExpr) In the instance declaration for ‘FromJSON NExpr’ | 561 | instance FromJSON NExpr | ^^^^^^^^^^^^^^ ```

The problem is that our instances for NExpr overlap with the new instances for Fix added in aeson-1.5.3.0

https://github.com/haskell-nix/hnix/blob/1a2d627a5807cde657ab1277234e9521008e8114/src/Nix/Expr/Types.hs#L533-L561

[sjakobi]

While looking into this I noticed that the current instances (added in https://github.com/haskell-nix/hnix/commit/146780c024adb3c9ec3d2122863a90e5fa5d276a) behave somewhat weirdly:

> :set -XOverloadedStrings 
> toJSON ("bla" :: NExpr)
Object (fromList [("unFix",Object (fromList [("tag",String "NSym"),("contents",String "bla")]))])

Is the unFix key an intended part of the output, and should we try preserving it?

Also, what's the purpose of the instances for NExpr? What are they being used for?

In any case I haven't figured out yet how to make use of the new instance ToJSON1 f => ToJSON (Fix f). With the following patch, I get a compile failure:

--- a/src/Nix/Expr/Types.hs
+++ b/src/Nix/Expr/Types.hs
@@ -508,10 +508,11 @@ $(deriveShow1 ''Binding)
 $(deriveShow1 ''Antiquoted)
 $(deriveShow2 ''Antiquoted)

---x $(deriveJSON1 defaultOptions ''NExprF)
+$(deriveJSON1 defaultOptions ''NExprF)
+$(deriveJSON1 defaultOptions ''NKeyName)
 $(deriveJSON1 defaultOptions ''NString)
 $(deriveJSON1 defaultOptions ''Params)
---x $(deriveJSON1 defaultOptions ''Binding)
+$(deriveJSON1 defaultOptions ''Binding)
 $(deriveJSON1 defaultOptions ''Antiquoted)
 $(deriveJSON2 defaultOptions ''Antiquoted)

@@ -543,7 +544,9 @@ instance ToJSON NUnaryOp
 instance ToJSON NBinaryOp
 instance ToJSON NRecordType
 instance ToJSON a => ToJSON (NExprF a)
+#if !MIN_VERSION_aeson(1,5,3)
 instance ToJSON NExpr
+#endif

 instance (FromJSON v, FromJSON a) => FromJSON (Antiquoted v a)
 instance FromJSON a => FromJSON (NString a)
@@ -558,7 +561,9 @@ instance FromJSON NUnaryOp
 instance FromJSON NBinaryOp
 instance FromJSON NRecordType
 instance FromJSON a => FromJSON (NExprF a)
+#if !MIN_VERSION_aeson(1,5,3)
 instance FromJSON NExpr
+#endif

 $(makeTraversals ''NExprF)
 $(makeTraversals ''Binding)

src/Nix/Expr/Types.hs:1:1: error:
    Exception when trying to run compile-time code:
      Constructor ‘DynamicKey‘ must only use its last 1 type variable(s) within the last 1 argument(s) of a data type

@jwiegley Any thoughts / advice?

[jwiegley]

@sjakobi unFix is an internal detail, and shouldn't be output by the encoding. I think it was simply never used, and so I didn't notice that this sort of scaffolding was leaking into the data representation.

[sjakobi]

I think it was simply never used

Since this issue is blocking the next release (#689) and I don't currently have the bandwidth to look into the issue more deeply, I suggest that we remove NExpr's [From,To]JSON instances for now.

Or does anyone else currently have the time to fix the instances properly?

[Anton-Latukha]

I ramp-up Haskell dev. Interested in moderate interesting tasks. Please, accept the contributed Haddock structure and docs.

Removing something - can do 8) Looking into fixing it, well... if you can not fix it right ahead - I probably neither. I remember looking into {From,To}JSON system and being puzzled about the use and ends of it, and there is related clojurians-org Report, essentially that he tried it and subsystem not worked fully anyway: https://github.com/haskell-nix/hnix/issues/539.

At some point in the future when would finish covering more system-practical Haskell stuff, should learn/practice Recursion Schemes if only just for the project's sake and deeper understanding.

Since code is done and mostly working, I am up for a piece-meal solution, dodge aeson, even simply rename our instance names (if they do not work - they probably should be explicitly properly marked as experimental anyway), etc, that instead of demolishing we can preserve current situation to solve it later, and others would be drawn to solve it.

Since our breadth of use is small, we can freely do any maneuvers: https://packdeps.haskellers.com/reverse/hnix.

[sjakobi]

Let's remove the problematic instances then. I'd appreciate a PR from you, @Anton-Latukha!

[sjakobi]

Please, accept the contributed Haddock structure and docs.

Why do you need to bring this up here, @Anton-Latukha? In case you still didn't get it, I'm concerned that you will spread your incomprehensible prose through the codebase that will stump everyone else until someone else takes the time to clean it up. If you don't accept reviews of your documentation changes to the codebase, I'm not interested in collaborating with you. Discussing this b***t with you is driving me nuts and makes me consider leaving this project.

[Anton-Latukha]

Well, sorry, since we just still not agreed on anything, so I ask in advance. Let us move this discussion theme to the #693.

[sjakobi]

I have made Hackage revisions for 0.8.0, 0.9.0 and 0.9.1 to constrain them to aeson < 1.5.3.

[Anton-Latukha]

I arrived at the same situation and solution.

This instance/data type situation showed me that I try to find a shortcut it feels like putting a cart before the horse. I am just not prepared for the question. I close: https://github.com/haskell-nix/hnix/issues/690 by doing what you did here. I go about it into detail in the chat https://gitter.im/haskell-nix/.

[sjakobi]

@Anton-Latukha, do you have a plan for achieving compatibility with aeson >= 1.5.3?

How do you feel about simply removing the problematic instances? Since they are apparently unused, I think that wouldn't be much of a loss.

[Anton-Latukha]

I've mainly tried to comply with aeson, I understood the "let's remove the problematic instances" in terms of removing local instances for using the upstream ones.

So that is what I tried.

And if I to remove all of the {To,From}JSON* instances - I would need to do a bunch of preparation work, going through our sources where those instances used, and look at reverse deps, I already looked at Profpatsch yarn2nix http://hackage.haskell.org/package/yarn2nix, he does not seem to be using instances, but also, well, there were people that tried to use it in the past.

We indeed can drop {To,From}JSON support, but that isomorphism seems theoretically basic and logical to have, if HNix wants to WebAssembly or if people would integrate HNix with web languages/technologies.

We should drop it if:
  | We care on drop to an important level -> but even then - I would ask one more person's opinion, since it seems important enough.
  | Drop it basing on our decision alone -> one of us should be able to resurrect it easily OR be sure that it is easy to him to write it anew. Again, since this isomorphism is an important one.

Since we two seem to be puzzled a bit, I am hesitant to remove the functionality that we even can't understand to fix, not even write one.

[sjakobi]

@Anton-Latukha, with "problematic instances", I mean just the ToJSON and FromJSON instances for NExpr – the other ones don't appear to be causing problems.

[Anton-Latukha]

Well, then - it is what I did, and I arrived at the same situation as you in https://github.com/haskell-nix/hnix/issues/690#issuecomment-671895749.

And I decided to go and to learn more, I felt uneasy, I do not want to learn aeson on the hunch and to write code here on the hunch, I better address my shortcomings.

That is my inference result (conclusion) from me approaching this. I leave this to others and would say more only when I understand more of the whole process of it.

[jwiegley]

I'm not sure anyone is even using the JSON instances for NExpr, so dropping them should be fine.

[sjakobi]

I'm not sure anyone is even using the JSON instances for NExpr, so dropping them should be fine.

Actually, the ToJSON NExpr instance is used by the --json option:

main/Main.hs:127:30: error:
    • No instance for (aeson-1.5.3.0:Data.Aeson.Types.ToJSON.ToJSON1
                         NExprF)
        arising from a use of ‘A.encodeToLazyText’
    • In the second argument of ‘($)’, namely
        ‘A.encodeToLazyText (stripAnnotation expr)’
      In the second argument of ‘($)’, namely
        ‘TL.putStrLn $ A.encodeToLazyText (stripAnnotation expr)’
      In the expression:
        liftIO $ TL.putStrLn $ A.encodeToLazyText (stripAnnotation expr)
    |
127 |     = liftIO $ TL.putStrLn $ A.encodeToLazyText (stripAnnotation expr)
    |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

$ hnix --json --expr '-1'
{"unFix":{"tag":"NUnary","contents":["NNeg",{"unFix":{"tag":"NConstant","contents":{"tag":"NInt","contents":1}}}]}}

This functionality was added in https://github.com/haskell-nix/hnix/commit/8d25aa461dbfe267292d91c289375a7702e561b9.

The usefulness of this output is still not very obvious to me. I guess it could be used for debugging, but that functionality could be superseded by https://github.com/haskell-nix/hnix/issues/661.

Is it OK if we remove this codepath?

https://github.com/haskell-nix/hnix/blob/0449dc457018b25335046d42be84ca3460feec16/main/Main.hs#L126-L127

The combination of --json and --eval should of course be preserved – it looks completely reasonable to me:

$ hnix --eval --json --expr '1 + 1'
2