Skip to content

Add change type signature quick fix #2436

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jneira opened this issue Dec 3, 2021 · 18 comments
Closed

Add change type signature quick fix #2436

jneira opened this issue Dec 3, 2021 · 18 comments
Labels

Comments

@jneira
Copy link
Member

jneira commented Dec 3, 2021

When you change the body of a function and the type signature does not match you get the obvious compiler error:

F.e. for

myfunc :: String
myfunc = forM

you get

• Couldn't match type ‘t0 a0 -> (a0 -> m0 b0) -> m0 (t0 b0)’
                 with ‘[Char]’
  Expected type: String
    Actual type: t0 a0 -> (a0 -> m0 b0) -> m0 (t0 b0)
• Probable cause: ‘forM’ is applied to too few arguments
  In the expression: forM
  In an equation for ‘myfunc’: myfunc = forM

It would be nice to have a quick fix (better than a code lens imo) which changes the type signature to match the body new type, something like Change type signature to: (Traversable t, Monad m) =>t a -> (a -> m b) -> m (t b)

That type is already shown on hover.

Additional context

  • We already have a code lens to add the code signature, so this feature could reuse a lot from that one
  • That feature is common in other ides like eclipse or intellij
@ncfavier
Copy link

ncfavier commented Dec 7, 2021

Would love to see this as well: code lenses are distracting.

@michaelpj
Copy link
Collaborator

Note that there's already a code action for adding a missing signature, if you don't like the lens.

@ncfavier
Copy link

ncfavier commented Dec 8, 2021

I couldn't find it. In Visual Studio Code, when clicking on the light bulb that appears next to a top-level definition, I only see retrie's fold/unfold actions.

@michaelpj
Copy link
Collaborator

Works for me! Unless lsp-mode is doing something weird and listing code lenses as code actions also?

@jneira
Copy link
Member Author

jneira commented Dec 8, 2021

Works for me! Unless lsp-mode is doing something weird and listing code lenses as code actions also?

I think that is the case, no code action in vscode for me neither

imagen

@jneira
Copy link
Member Author

jneira commented Dec 8, 2021

I would say code lenses for missing type signature is one of the more appropiate uses of lenses. It displays just the code which will be added if you click it, no replaces or subsitutions and helps you even if you dont add them.
But i agree even for missing type signatures it can be invasive and should have an alternative in the server itself.

@michaelpj what do you think to move upstream the code lenses as code actions thing of emacs lsp mode. Unconditionally or only if user disable the code lenses explicitly.

@michaelpj
Copy link
Collaborator

@michaelpj
Copy link
Collaborator

Hmm, maybe that's only used to apply the edit, ignore me.

@michaelpj
Copy link
Collaborator

I can't find where lsp-mode does this, but I guess it must be.

@pepeiborra
Copy link
Collaborator

HLS code actions are driven by warnings. If you don't see the "add missing signature" code action, enable -Wmissing-signatures in your module or project

Screen.Recording.2021-12-08.at.14.48.38.mov

@michaelpj
Copy link
Collaborator

Ah, that might well be it. There's definitely a code action: I just checked and it's part of a code action response from the server.

@michaelpj
Copy link
Collaborator

It's a bit odd that you can get one and not the other...

@ncfavier
Copy link

ncfavier commented Dec 8, 2021

That works, but I don't want to enable that warning for normal compilation, and I don't want the yellow squiggles. The fold/unfold actions don't rely on warnings, so surely there's a way!

@pepeiborra
Copy link
Collaborator

Tying code actions to warnings allows users to select which quick fixes they care about. So it's not a technical limitation, it's a design decision.

@jneira
Copy link
Member Author

jneira commented Dec 8, 2021

Well, change the type signature, the topic of the issue, will be tied to a compiler error, so it will be always available ;-)

It's a bit odd that you can get one and not the other...

Agree both should be in sync

@drsooch
Copy link
Collaborator

drsooch commented Jan 20, 2022

If there isn't any movement on this, I'll take a look.

@drsooch
Copy link
Collaborator

drsooch commented Jan 24, 2022

There are a couple questions I ran into at the start of thinking about how this would work:

  • How do we determine when to change a signature? For example, if I define a function Int -> Int -> Int and use it in a context where it should be Integer -> Integer -> Integer. Which usage site should we aim to change? Should we try to change it all?
    ex:
foo :: Int -> Int -> Int
foo x y = x + y

bar :: Integer -> Integer
bar x = foo x 20
  • Is it possible to grab the actual type rather than the expected type (outside of a regex match)? Error messages from GHC won't always provide the full type signature that is expected. And we would have to guess what the type signature would look like.
    ex:
foo :: Int -> Int
foo x = x + ord c

The error message looks something like:

• Couldn't match expected type ‘Int’ with actual type ‘Char -> Int’
    • The equation(s) for ‘foo’ have two arguments,
      but its type ‘Int -> Int’ has only one
  |
4 | foo x c = x + ord c

I'm not well-versed enough in the GHC API to know if it's possible to "solve" the expected signature? Naively, I could replace the first Int with Char -> Int, but I can imagine as parameter lists grow this may become exceedingly complex.

@jneira
Copy link
Member Author

jneira commented Jan 25, 2022

interesting questions, maybe we could try first the simple way and no handle changes in number of params which could be an improvement in a 2 iteration

and change only the function target of the ghc error, if that breaks other function a bunch of errors will be spawned and the user will decide if going back or forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants