Fix code action for adding missing constraints to type signatures

[jhrcek]

I'm trying to fix this issue, which is quite relevant for our codebase at work: we have many type signatures like this, for which the code action "Add CONSTRAINT to the context of the type signature for FUNCTION" doesn't work.

someFunction
    :: ( WithDb env m
       , WithError m
       , Has PieceOfConfig env
       )
    => Id User
    -> Id ProjectInfo
    -> m ProjectInfo
someFunction userId projectId = do
...

I added 2 failing tests illustrating both issues. The current implementation seems quite brittle (based on parsing stuff with regexes), I'm trying to figure out how to make it more robust..

[alanz]

I'm trying to figure out how to make it more robust..

Use the AST :)

[jhrcek]

Ok, I tried to use the AST and made the tests pass. It'll still need some work and I'll have to clean this up a lot.
I'll get back to this.

[pepeiborra]

Thank you for your contribution!
Apologies for the long delay in reviewing this, change looks fine to me with a minor change request.

[pepeiborra]

Not every type with a context is rooted in an HsQualTy constructor. Did you consider using splitLHsQualTy instead ?

[jhrcek]

Good point, I wasn't aware a function like that existed.
I just tried it but it makes the test case where there's no initial context to start with fail.

Looking at how splitLHsQualTy is implemented it's clear why: It uses noLHsContext = noLoc [] in the case where I use SrcSpan of the start of the type signature. So in that respect my "manual" pattern match seems better.

On the other hand splitLHsQualTy has an additional case HsParTy which I have no idea what kind of syntax it represents. I'll try to find out and tweak my implementation based on that (Ideally adding a test case which triggers that case).

[jhrcek]

I didn't find an example syntax which would trigger that HsParTy case, but I reimplemented my attempt like this, which made the tests pass

          srcSpanToRange $ case splitLHsQualTy locatedType of
            (L contextSrcSpan _ , _) ->
              if isGoodSrcSpan contextSrcSpan
                then contextSrcSpan -- The type signature has explicit context
                else -- No explicit context, return SrcSpan at the start of type sig where we can write context
                     let start = srcSpanStart $ getLoc locatedType in mkSrcSpan start start

BUT it seems to be noticably slower:

compared to my previous impl:

Which implementation would you suggest using?

[jhrcek]

Nevermind, the speed difference seems to have been fluke. On repeated reruns both versions seems to have comparable performance. So I'm going with your suggestion of using the ghc helper function.