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

Unrequired language pragmas when using GHC2021 #1386

Open
dbeacham opened this issue Jun 6, 2022 · 10 comments · May be fixed by #1464
Open

Unrequired language pragmas when using GHC2021 #1386

dbeacham opened this issue Jun 6, 2022 · 10 comments · May be fixed by #1464

Comments

@dbeacham
Copy link

dbeacham commented Jun 6, 2022

If I have a file with a flag turned on by GHC2021, e.g. {-# LANGUAGE TypeApplications #-} and run

> hlint -XGHC2021 fileWithTypeApplicationOn.hs

should I expect it to pick up the language pragma as unrequired or duplicate?

I'm not currently seeing any warnings but I'm also not sure if that's expected behaviour.

@ndmitchell
Copy link
Owner

It's expected that you don't see any warnings. If you want to ban various extensions (either because they are turned on by default, or because you think they are bad ideas) see https://github.com/ndmitchell/hlint#restricting-items. The extensions passed on the command line are more about getting the code to parse properly, rather than informing HLint of the environment the code is run in.

@ocharles
Copy link
Contributor

While I understand what's happening, I share @dbeacham's confusion. I thought using -XGHC2021 would extend the rules to know that 2021-default extensions don't need to be explicitly enabled.

@ndmitchell
Copy link
Owner

It certainly could. Would -XRankNTypes also give warnings if you had rank N types explicitly enabled? Would that be annoying to some people?

@amesgen
Copy link
Contributor

amesgen commented Mar 2, 2023

I am also interested in this, motivated by wanting to automatically remove GHC2021-implied extensions in a project with lots of per-file LANGUAGE pragmas that will soon-ish require GHC >=9.2 and can hence switch to GHC2021.

I took a stab at implementing this in #1464; let me know if this looks somewhat reasonable.

Would -XRankNTypes also give warnings if you had rank N types explicitly enabled?

For now, I did not do that as I did not immediately see a strong use case; but it should not be difficult to add if it is desired of course.

@alaendle
Copy link

alaendle commented Mar 9, 2023

@amesgen - I had the same requirement - switching to GHC2021 and remove the old language pragmas. But that's really easy to do by just using a .hlint.yaml that disables all extensions and only explicitly allow extensions that are not part of GHC2021. Also this allows you to prove that no other/unwanted extensions live in your codebase.

@amesgen
Copy link
Contributor

amesgen commented Mar 9, 2023

Yeah, good point, #1464 is not strictly required for this use case. It seems to me to be more convenient not having to maintain the complement of GHC2021 in .hlint.yaml (around ~80 extensions ATM), but it is very low-maintenance, so not too bad.

@ocharles
Copy link
Contributor

ocharles commented Mar 9, 2023

Can someone share such a rule set?

@alaendle
Copy link

It certainly could. Would -XRankNTypes also give warnings if you had rank N types explicitly enabled? Would that be annoying to some people?

At least I would except such warnings; because we really want to highlight unnecessary language pragmas in the code files - so my expectations is that -X parameters usually indicated the default extensions activated in .cabal or .package files - so technically the pragmas in the codebase aren't necessary.

Can someone share such a rule set?

I jumped at the opportunity to start with an empty list of allowed extensions and just added the used extensions as now reported by hlint. This has two advantages from my perspective - you now have a documentation of the used extensions - and it also is a chance to get rid of unwanted pragmas - e.g. I spotted a really unnecessary {-# LANGUAGE NoMonomorphismRestriction #-} in my codebase. To give an example - here is my .hlint.yaml file:

# Disable all extensions by default
# And only add extensions that are needed and not part of GHC2021
- extensions:
  - default: false
  - name:
    - AllowAmbiguousTypes
    - DeriveAnyClass
    - DerivingStrategies
    - OverloadedLabels
    - OverloadedStrings
    - QuasiQuotes
    - RecordWildCards
    - TemplateHaskell
    - ViewPatterns

@ocharles
Copy link
Contributor

I think HLint does need to know about this, because while the above works it causes a warning about a "restricted" extension. This is confusing, because the extension isn't restricted at all - it's just redundant to explicitly enable it.

@michaelpj
Copy link

(I just switched HLS to use GHC2021, it was annoying)

I would really appreciate @amesgen 's change. I am used to hlint helpfully telling me about redundant extensions, so I was very much expecting it to help me out when I switched to default-language: GHC2021. I don't see why we wouldn't use that information since we have it!

I also don't know if hlint supports identifying extensions that are implied by default-extensions, but that would also be very useful. Perhaps that amounts to being sensitive to -X flags as @ndmitchell suggests here.

Maybe this would be annoying to some people, but I think the most common way people use hlint is with a tool like cabal, and as such it is IMO a much better UX if hlint appears to know about information in the cabal file as much as possible.

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

Successfully merging a pull request may close this issue.

6 participants