Typeclass constraint insert suggestions should not be fully-qualified as the syntax is not valid.

[theophile-scrive]

Hi! When suggesting the addition of a missing typeclass constraint in a function signature, which has not yet been brought into scope, HLS will suggest its fully-qualified name (Module.Module.Class m), which is not valid syntax in the absence of a corresponding import line, which 1) is not handled automatically by HLS, and 2) not desired, as fully-qualified constraints are noisy and hamper legibility.

I would suggest to trim the name of the constraint, and let the import plugin sort out its provenance, or better, leave the user decide how it should be qualified. But inserting a fully-qualified name which then has to be edited is not a smooth experience.

[fendor]

Thank you for the bug report!

I agree, the current status quo is an unsatisfactory experience.
This may be caused by the pretty printing logic to automatically qualify Names, so perhaps it is as simple as changing how we print the Name?

[theophile-scrive]

@fendor Is this something that can be done by a relative newcomer to the codebase? I don't mind submitting a PR but the exact place eludes me at the moment. :)

[fendor]

I think it may be possible, but when it comes to GHC and HLS, all sorts of unexpected consequences could come up if we change the printing :) I can have a quick look to give some pointers, but I can't promise outright that it is easy to do.

[theophile-scrive]

Got it, thank you!

[fendor]

It looks like we get the constraint from the error message, for example:

src/GHC/Debug/Brick/Render/Histogram.hs:30:54: error: [GHC-39999]
    • No instance for ‘ghc-internal-9.1003.0:GHC.Internal.Real.Integral
                         a’
        arising from a use of ‘fromIntegral’
      Possible fix:
        add (ghc-internal-9.1003.0:GHC.Internal.Real.Integral
               a) to the context of
          the type signature for:
            histogram :: forall a. a -> [GHC.Debug.Types.Size] -> Widget Name
    • In the second argument of ‘(/)’, namely ‘fromIntegral boxes’
      In the third argument of ‘ceiling’, namely
        ‘(100 / fromIntegral boxes)’
      In the first argument of ‘fromIntegral’, namely
        ‘(ceiling @Double @Int (100 / fromIntegral boxes))’
   |
30 |     step = fromIntegral (ceiling @Double @Int (100 / fromIntegral boxes))
   |                                                      ^^^^^^^^^^^^

As such, we could try parsing the error message to strip the unit and module prefix... Or perhaps there is a GHC option to print the name differently? How does the diagnostic look like using a structured diagnostics 🤔

[fendor]

It looks like we might be able to get the Name from the structured error message, perhaps CannotResolveInstance.

The idea would be to do something similar to #4620, and then pattern match on the correct constructor, extract the PredType via errorItemPred. I am not sure what the way would be to get the name unqualified from the CannotResolveInstance thing, though :)
Finding the correct ppr* variation is a slight challenge.

So, summary, in the hls-refactor-plugin, thread the active diagnostics to suggestFunctionConstraint in plugins/hls-refactor-plugin/src/Development/IDE/Plugin/CodeAction.hs, pattern match on the context using appropriate pattern matchers, e.g. see ghcide/src/Development/IDE/Types/Diagnostics.hs for our compatability module, and then pretty print it to our hearts desire :) Doable, but I wouldn't call it easy.