Skip to content

Set up partial functions ratchet #2974

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 13 commits into from
Jun 24, 2022

Conversation

michaelpj
Copy link
Collaborator

@michaelpj michaelpj commented Jun 20, 2022

  • Move the ghcide .hlint.yaml to the top-level
  • Add checks for a number of common partial functions, add exceptions for all existing locations
  • Remove cruft from .hlint.yaml
  • Set the hlint job to fail if there are any errors, ensuring that more instances can't be added without adding exceptions in .hlint.yaml.

@hasufell
Copy link
Member

CI error:

  Error: hlint	./ghcide/src/Development/IDE/GHC/CoreFile.hs		200	33	error	Avoid restricted function	Error in isGhcideName in module Development.IDE.GHC.CoreFile: Avoid restricted function ▫︎ Found: "nameModule" ▫︎ Note: may break the code

@michaelpj michaelpj force-pushed the mpj/partial-functions-ratchet branch from ca9c443 to 46d8dc5 Compare June 20, 2022 12:37
@michaelpj
Copy link
Collaborator Author

Rebased and added exceptions for the new stuff, added unsafePerformIO and fold1 variants.

@michaelpj
Copy link
Collaborator Author

I am very confused about how the changes I've made can affect some test in the func test suite 🤔

@michaelpj
Copy link
Collaborator Author

I think it must be picking up the hlint config.

@michaelpj michaelpj force-pushed the mpj/partial-functions-ratchet branch from c424232 to 0b5c3ec Compare June 21, 2022 09:35
@michaelpj
Copy link
Collaborator Author

Yep, that was it.

@michaelpj michaelpj requested a review from eddiemundo as a code owner June 21, 2022 10:38
@michaelpj
Copy link
Collaborator Author

How can I be getting ghcide test failures?????

@hasufell
Copy link
Member

How can I be getting ghcide test failures?????

There can be many reasons on windows:

  1. flaky tests, because windows file locking is odd (we get spurious failures in GHC/cabal/ghcup there as well)
  2. github actions environments are in fact rolling-release... so a job that succeeded yesterday may now fail, e.g. due to updated msys2 or other stuff

@michaelpj michaelpj force-pushed the mpj/partial-functions-ratchet branch from ddb607d to 08771a1 Compare June 21, 2022 13:59
@michaelpj
Copy link
Collaborator Author

I rebased, let's see if it reoccurs.

@michaelpj
Copy link
Collaborator Author

And now it's happy 🤷

@michaelpj
Copy link
Collaborator Author

Anyone like to approve?

@michaelpj
Copy link
Collaborator Author

I'll leave it a day or two in case anyone else has opinions :)

@jhrcek
Copy link
Collaborator

jhrcek commented Jun 23, 2022

Love it. Now I really don't have any excuse for not starting to remove individual partial functions one by one 😄

@michaelpj michaelpj added the merge me Label to trigger pull request merge label Jun 24, 2022
@mergify mergify bot merged commit efcb8e2 into haskell:master Jun 24, 2022
hololeap pushed a commit to hololeap/haskell-language-server that referenced this pull request Aug 26, 2022
* HLint partial functions

* Delete commented out stuff

* Delete flag-based warnings, unclear they add much

* Delete DA-specific hints

* Delete extra cpp args

* Delete extension options relating to dead build system

* Don't bother trying to restrict CPP for now, it's everywhere

* Make the hlint job fail if there are any errors

* Fix for rebase, add unsafePerformIO and fold1 variants

* Add some more indexing functions

* Try turning off hlint on this test file

* Add .hlint.yaml file for hls-hlint-plugin

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants