Skip to content
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

Allow apply refactorings to an already parsed module #91

Closed
jneira opened this issue Nov 12, 2020 · 8 comments
Closed

Allow apply refactorings to an already parsed module #91

jneira opened this issue Nov 12, 2020 · 8 comments

Comments

@jneira
Copy link
Contributor

jneira commented Nov 12, 2020

Hi, we added recently hlint support in haskell-language-server, including the apply of refactorings usgins appy-refact, via applyRefactorings
The actual integration could be improved if the api would expose a function to apply them to an already parsed module.
Now we have to use a temporary file and ask apply-refact to parse it again:

res <- liftIO $ withSystemTempFile (takeFileName fp) $ \temp h -> do
            hClose h
            writeFileUTF8NoNewLineTranslation temp oldContent
            (Right <$> applyRefactorings Nothing commands temp) `catches`
                    [ Handler $ \e -> return (Left (show (e :: IOException)))
                    , Handler $ \e -> return (Left (show (e :: ErrorCall)))
                    ]

As we are using ghc-lib to parse for ghc < 8.10 and the real ghc for 8.10, following hlint, maybe only we can take advantage in the latter path, but i think it still will worth it.

Refact.Internal.runRefactoring is exposed but maybe a higher endpoint, closer to applyRefactorings would be handy for downstream packages.

For ghc < 8.10 we will continue to need delegate the parsing to apply-refact. In that case it would be handy be able to inform the parser option, to, for example, pass custom language extensions not included in the module as pragmas.
Let me know if you think that should be in another issue.

//cc @ndmitchell @alanz

@zliu41
Copy link
Collaborator

zliu41 commented Nov 12, 2020

Sounds good, happy to do this.
Does something like applyRefactorings, but replacing the FilePath argument with

(Language.Haskell.GHC.ExactPrint.Anns, Located (HsModule GhcPs))

work for you?

it would be handy be able to inform the parser option, to, for example, pass custom language extensions not included in the module as pragmas

Totally agree. applyRefactorings should support passing language extensions.

@jneira
Copy link
Contributor Author

jneira commented Nov 12, 2020

Many thanks for take care 😃

Does something like applyRefactorings, but replacing the FilePath argument with

Sounds good, we already have ParsedSource(==Located (HsModule GhcPs)) and only will need Language.Haskell.GHC.ExactPrint.Anns (i guess we can get it from ghc ApiAnns??)

@zliu41
Copy link
Collaborator

zliu41 commented Nov 13, 2020

Correct, Anns can be obtained from ApiAnns via postParseTransform.

@jneira
Copy link
Contributor Author

jneira commented Nov 17, 2020

For ghc < 8.10 we will continue to need delegate the parsing to apply-refact. In that case it would be handy be able to inform the parser option, to, for example, pass custom language extensions not included in the module as pragmas.

It is done in #98

@zliu41
Copy link
Collaborator

zliu41 commented Nov 18, 2020

@jneira It just occurred to me that for this to work, your Located (HsModule GhcPs) must be from GHC, not ghc-lib, since that's what ghc-exactprint requires. It seems you are using ghc-lib at least for GHC < 8.10, and if so, this is not going to work for you until ghc-exactprint is migrated to ghc-lib (alanz/ghc-exactprint#93).

I'll fix the other bugs you reported in the next few days but this is the best that can be done at the moment.

I did plan to work on alanz/ghc-exactprint#93 once I get more bandwidth but the pandemic has made it difficult.

@jneira
Copy link
Contributor Author

jneira commented Nov 19, 2020

thanks for your great work
I was assuming that i was going to use ParsedModule only for ghc-8.10, at least initially, so it would be great to have it, even without ghc-lib support

@zliu41
Copy link
Collaborator

zliu41 commented Nov 19, 2020

OK, sounds good. I'll put it in then.

@zliu41
Copy link
Collaborator

zliu41 commented Nov 20, 2020

Merged #100

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

No branches or pull requests

2 participants