Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix suggestAddTypeAnnotation regex #760

Merged
merged 8 commits into from
Jan 5, 2021

Conversation

kderme
Copy link
Contributor

@kderme kderme commented Dec 30, 2020

I had a code with some warnings

module Main where

main :: IO ()
main =
    let x = 3
    in return ()

The suggestion is

2020-12-30 06:33:26.391800777 [ThreadId 7] - <--2--{"result":[{"edit":{"changes":{"file:///home/kostas/programming/test-
hls/Main.hs":[{"range":{"start":{"line":4,"character":12},"end":{"line":4,"character":13}},"newText":"(3’ • In the expression: 3 In 
an equation for ‘x :: Integer)"}]}},"kind":"quickfix","diagnostics":[{"severity":2,"range":{"start":{"line":4,"character":12},"end":
{"line":4,"character":13}},"source":"typecheck","message":"• Defaulting the following constraint to type ‘Integer’\n    Num p0 
arising from the literal ‘3’\n• In the expression: 3\n  In an equation for ‘x’: x = 3\n  In the expression: let x = 3 in return 
()"}],"title":"Add type annotation ‘Integer’ to ‘3’ • In the expression: 3 In an equation for ‘x’"}],"jsonrpc":"2.0","id":556}

and it results in a wrong fix:

- let x = 3
+ let x = (3’ • In the expression: 3 In an equation for ‘x :: Integer)

From the warning

    - Defaulting the following constraint to type ‘Integer’
        Num p0 arising from the literal ‘3’
    - In the expression: 3
      In an equation for ‘x’: x = 3
      In the expression: let x = 3 in return ()

it seems the lit is parsed as 3’ • In the expression: 3 In an equation for ‘x instead of 3 since it is longer and still valid:

  • It is wrapped in ' '
  • There is a second In the expression after it.

Btw the approach here seems rather hacky. Are there any plans for ghc to give better support for fix sugestions?

tested with ghc-8.8.4

@kderme kderme changed the title fix-suggestAddTypeAnnotation regex fix suggestAddTypeAnnotation regex Dec 30, 2020
@kderme kderme changed the title fix suggestAddTypeAnnotation regex Fix suggestAddTypeAnnotation regex Dec 30, 2020
@pepeiborra
Copy link
Collaborator

There are plans to make GHC produce structured errors: https://gitlab.haskell.org/ghc/ghc/-/issues/18516

@pepeiborra
Copy link
Collaborator

How do we know that your fix works? A new test case in https://github.com/haskell/haskell-language-server/blob/master/ghcide/test/exe/Main.hs would be great.

@kderme kderme force-pushed the fix-suggestAddTypeAnnotation branch from d853733 to f5039f5 Compare December 31, 2020 14:20
Copy link
Collaborator

@pepeiborra pepeiborra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks!

@jneira jneira added the merge me Label to trigger pull request merge label Jan 4, 2021
@mergify mergify bot merged commit 93ab0ea into haskell:master Jan 5, 2021
pepeiborra added a commit to pepeiborra/ide that referenced this pull request Jan 9, 2021
Co-authored-by: Javier Neira <atreyu.bbb@gmail.com>
Co-authored-by: Pepe Iborra <pepeiborra@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants