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

Don't suggest -Wno-deferred-out-of-scope-variables as a possible action. #4440

Closed
jeukshi opened this issue Oct 28, 2024 · 6 comments · Fixed by #4441
Closed

Don't suggest -Wno-deferred-out-of-scope-variables as a possible action. #4440

jeukshi opened this issue Oct 28, 2024 · 6 comments · Fixed by #4441

Comments

@jeukshi
Copy link
Contributor

jeukshi commented Oct 28, 2024

Is your enhancement request related to a problem? Please describe.

With code

f = let x = Doesn'tExist
     in undefined

when I execute code action on Doesn'tExist, HLS (haskell-language-server version: 2.9.0.0 (GHC: 9.6.6)) adds {-# OPTIONS_GHC -Wno-deferred-out-of-scope-variables #-} on the top of my file. It does this, I believe, because this is the only code action available. I don't think this is the solution that user wants in most cases. At least in Emacs it is easy to miss that HLS did that (if only one action is available, it is executed), leading to confusion on as to why, suddenly, HLS "doesn't work". It works, of course, because with that flag there is no longer a compile time error.

Describe the solution you'd like

Don't suggest {-# OPTIONS_GHC -Wno-deferred-out-of-scope-variables #-}.

Describe alternatives you've considered

Restarting HLS... a lot :) I don't think there is an alternative.

@fendor
Copy link
Collaborator

fendor commented Oct 29, 2024

Hi, thank you for your bug report!

Does emacs run the code action for a particular location when there is only one code action? That feels like rather bad behaviour to me. It can easily happen that the only available code action is not something you want to execute, imo. Are you sure that's emacs behviour?

Why do you need to restart HLS to undo the code action? Shouldn't undo make HLS display the error again?

@jeukshi
Copy link
Contributor Author

jeukshi commented Oct 29, 2024

Are you sure that's emacs behviour?

Doom Emacs, at least, yes. It is... questionable UX, I agree.

If it is not a constructor, like 'func = doesn'tExist', then I get 3 actions I can choose:

  • Define doesn'tExist :: _
  • Add argument 'doesn'tExist' to a function
  • Disable "deferred-out-of-scope-variables"

Why do you need to restart HLS to undo the code action?

I know it should be an error there, but there is none reported by HLS (it got deferred by the flag and I didn't notice). So I try to get it back. Imagine copy-pasting some code, running add import on a bunch of variables and this guy gets inserted along the way, on the top of the file. I spend some time figuring it out, twice now.

But ignoring Emacs behavior, let me elaborate.

Is Disable "deferred-out-of-scope-variables" really an action that HLS should offer? It seems to me this flag is "know what you are doing"/"you should know when you need it" type of deal. Rather niche at that. Not to mention, it does not apply to a variable, but a whole file. And, worse, it doesn't even fix the issue, it just changes Haskell into Python. All this in the name of helping with... a typo. I would rather have no action here and fix my error by hand, properly.

@fendor
Copy link
Collaborator

fendor commented Oct 29, 2024

Related issue #2032, seems like we have some prior art of special casing some warnings. See PR #2061 for the fix.
I feel like this approach of blacklisting flags is not the most maintainable, but I agree this flag is extremely rarely what you want.

Fixes welcome!

Either way, I think doom emacs's behaviour is broken, too.

@jeukshi
Copy link
Contributor Author

jeukshi commented Oct 29, 2024

Seeing that PR I think you already did 90% of the work by just finding it! I'll then do my part and nuke that thing.

As for Emacs, well, it's Emacs, can be configured, probably.

jeukshi added a commit to jeukshi/haskell-language-server that referenced this issue Oct 29, 2024
jeukshi added a commit to jeukshi/haskell-language-server that referenced this issue Oct 29, 2024
Fixes haskell#4440

Fixes test for disabling deferred-type-errors.
@Bodigrim
Copy link
Contributor

Bodigrim commented Nov 2, 2024

Could HLS set -Wno-deferred-out-of-scope-variables under the hood to make further progress with compilation and report more diagnostics?

@fendor
Copy link
Collaborator

fendor commented Nov 3, 2024

@fendor fendor closed this as completed in 96bea00 Nov 3, 2024
soulomoon pushed a commit to soulomoon/haskell-language-server that referenced this issue Nov 4, 2024
Fixes haskell#4440

Fixes test for disabling deferred-type-errors.
fendor pushed a commit to fendor/haskell-language-server that referenced this issue Dec 23, 2024
Fixes haskell#4440

Fixes test for disabling deferred-type-errors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants