Restore ability to run source plugins #3309
Conversation
JakobBruenker
force-pushed
the
fix-source-plugins
branch
from
bd23c76 to
b2ae93a
Compare
Since ghc 9.0, plugins are stored in the HscEnv, not in the DynFlags. This caused HLS not to run source plugins anymore. This commit fixes that. Fixes haskell#3299.
JakobBruenker
force-pushed
the
fix-source-plugins
branch
from
b2ae93a to
41c2b77
Compare
|
Thank you! This is great stuff.
The right place for a test would be the ghcide test suite. You can take inspiration on the existing plugin tests, as well to reenable the ones that are disabled for GHC 9.x |
|
You can enable the test here: haskell-language-server/ghcide/test/exe/Main.hs Line 1190 in caa5ce5 Might be broken for windows. |
In 9.4, the ability for parser source plugins to access and manipulate non-fatal parse errors was added: https://gitlab.haskell.org/ghc/ghc/-/wikis/migration/9.4#parser-plugins-have-a-different-type HLS always threw an error in this situation without running the plugins though. This commit fixes that.
|
Before I address your comments: I noticed that the ability for parser plugins to change error messages, introduced in 9.4, wasn't supported by HLS. This new commit fixes that. I haven't had time yet to test it with every supported GHC version, however. I'm still seeing an |
|
I ran cabal test (using latest master) and they all passed. Can we get this merged? I'd really like to have this functionality back. edit: oops, I was running under GHC 9.0.2. :( |
|
@JakobBruenker Do you think you have time to push this over the finish line for #3409? If not, I can take a look at it. EDIT: sorry, couldn't stop myself 🙈 |
fendor
force-pushed
the
fix-source-plugins
branch
from
c564332 to
4a44857
Compare
| -- Implicit assumption: DynFlags in 'msrModSummary' are the same as | ||
| -- the DynFlags in 'msrHscEnv'. |
There was a problem hiding this comment.
This is inaccurate, later in the pipeline, dozens of functions modify the 'DynFlags' from 'msrModSummary'. This comment will be changed to basically telling users to prefer the DynFlags from msrModSummary instead of from this HscEnv. Does this seem sensible?
|
@fendor sorry, didn't see your comment yesterday. Sorry to have this let sit idle, I'm happy to see it's done! |
|
@JakobBruenker Dont be sorry, you did a valuable contribution, and I am genuinely thankful for that! I didn't give you any time to react, happy that you are not mad at me for taking over! |
Since ghc 9.0, plugins are stored in the HscEnv, not in the DynFlags. This caused HLS not to run source plugins anymore. This commit fixes that.
Fixes #3299.
It also fixes one of the two problems mentioned in #2779, which is that the plugin wasn't running in HLS. It does not fix the problem where HLS shows the wrong location for the error message described in that issue.
If I should add a test it would be helpful to know which test-data directory it should go into, and which files I have to look at to make the test run.