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

#600 Code action to ignore hlint hints module wide #2458

Merged
merged 11 commits into from
Dec 10, 2021

Conversation

eddiemundo
Copy link
Collaborator

@eddiemundo eddiemundo commented Dec 9, 2021

ignore-hlint-hint-module-wide-demo

It will also insert a -Wno-unrecognized-pragmas options pragma only if that dynflag isn't already set.

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

Amazing work, including sensible new test cases and nice refactorings, many many thanks

@jneira
Copy link
Member

jneira commented Dec 9, 2021

There is some problem with the pragmas placement in ubuntu and ghc-8.8.4:

 FAIL (1.88s)
      Test output was different from 'test/testdata/UnrecognizedPragmasOn.expected.hs'. Output of ["git","-c","core.fileMode=false","diff","--no-index","--text","--exit-code","test/testdata/UnrecognizedPragmasOn.expected.hs","/tmp/UnrecognizedPragmasOn.expected34086-3.actual"]:
      diff --git a/test/testdata/UnrecognizedPragmasOn.expected.hs b/tmp/UnrecognizedPragmasOn.expected34086-3.actual
      index 564503c..e65d316 100644
      --- a/test/testdata/UnrecognizedPragmasOn.expected.hs
      +++ b/tmp/UnrecognizedPragmasOn.expected34086-3.actual
      @@ -1,5 +1,5 @@
      -{-# OPTIONS_GHC -Wunrecognised-pragmas #-}
       {-# OPTIONS_GHC -Wno-unrecognised-pragmas #-}
       {-# HLINT ignore "Eta reduce" #-}
      +{-# OPTIONS_GHC -Wunrecognised-pragmas #-}
       module UnrecognizedPragmasOn where
       foo x = id x

@eddiemundo
Copy link
Collaborator Author

I think I've fixed the tests, but it's weird.

The issue was me not using the right dynflags in some cpp for the pragmas parser initialization, but that should (and does) fail the Pragmas plugin tests for 8.8.4. Yet I never saw any 8.8.4 test failures from the CI in the PR where I extracted the pragma parsing stuff into its own module. This means that the CI didn't run the Pragmas plugin tests for that PR?

Either that or somehow default dynflags have changed since then, and the Pragma plugin tests were passing back then but not now.

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

Really a great work, happy to see a contibution to my beloved hlint plugin
looking forward to your next contributions

@jneira jneira added the merge me Label to trigger pull request merge label Dec 10, 2021
@mergify mergify bot merged commit 0f49c0e into haskell:master Dec 10, 2021
@eddiemundo eddiemundo deleted the ignore-hlint-hint branch December 11, 2021 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(Opinion) hlint should not be enabled by default
2 participants