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

add hlint rule for NonEmpty.unzip #1500

Merged
merged 1 commit into from
Jan 14, 2024
Merged

add hlint rule for NonEmpty.unzip #1500

merged 1 commit into from
Jan 14, 2024

Conversation

mixphix
Copy link

@mixphix mixphix commented Apr 24, 2023

For CLC ticket #86.

@mixphix
Copy link
Author

mixphix commented May 3, 2023

I wasn't sure if this was the best category to put it in. Let me know!

@ndmitchell
Copy link
Owner

Thanks for the hint - this should go in the base group, as we want it on by default.

@ndmitchell
Copy link
Owner

I'll move it

@ndmitchell ndmitchell merged commit c17deb5 into ndmitchell:master Jan 14, 2024
ndmitchell added a commit that referenced this pull request Jan 14, 2024
@ndmitchell
Copy link
Owner

You are of course right this is a generalisation hint, and if we weren't deprecating NonEmpty.unzip it would be exactly the right place for it to go. But given it is, we want it on by default. Moved in 6f84e5c

@snowleopard
Copy link

I just had to silence this warning because it was firing here. I'm not sure why it fires there: as far as I can tell, this unzip refers to the monomorphic one for lists, not to Data.List.NonEmpty.unzip as the HLint rule says. Maybe HLint can't really tell where that unzip is coming from? But then maybe the suggestion should be worded differently.

Note that I have import Data.List.NonEmpty (NonEmpty (..), nonEmpty, toList, reverse) at the top of the module, so I shouldn't have Data.List.NonEmpty.unzip in scope.

Not a big deal (I just disabled the warning) but something felt fishy, hence my report.

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 this pull request may close these issues.

3 participants