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

use weeder #2044

Merged
merged 7 commits into from
Jul 17, 2024
Merged

use weeder #2044

merged 7 commits into from
Jul 17, 2024

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Jul 15, 2024

Closes #2043

Local usage notes

I suspect that HLS in VS Code spontaneously pollutes the .hie directory, because after a few successful invocations, weeder started complaining with:

incompatible hie file: /home/kostmo/github/swarm/.hie/Swarm/Web/Auth.hie
    this version of weeder was compiled with GHC version 9064
    the hie files in this project were generated with GHC version 9048
    weeder must be built with the same GHC version as the project it is used on

Fixing false positives

Previously, for each library and executable, the HIE directory was always the same:

ghc-options:
    -hiedir=.hie

However, most of the executables have at least one module path that is the same relative to their hs-source-dirs base: namely, Main.hs. This resulted in all but one of the Main.hie files being overwritten in the end.

To avoid this, I have specified a different -hiedir for each library/executable/test that parallels its hs-source-dirs base. This way, so long as hs-source-dirs are unique across these targets, all of the *.hs files across the entire project will have unique *.hie paths.

Whitelisting exceptions

There are some known limitations with weeder regarding support for Template Haskell and type families. Exceptions are specified in the roots list in weeder.toml.

After removing a handful of dead functions myself, there are approx. 30 "true positive" weeds that I have listed explicitly in weeder.toml. Maintenance of this list of exceptions should eventually be easier with ocharles/weeder#146.

Integration with CI

I found a ready-made GitHub Action for weeder: https://github.com/freckle/weeder-action
I hacked support directly into the generated .github/workflows/haskell-ci.yml file.

Ideally, the generator would have an option for a weeder step. Indeed, there is an open issue for supporting weeder in haskell-ci: haskell-CI/haskell-ci#132

A separate but related functionality that would be nice in haskell-ci is to specify one of the GHC versions in the matrix to do additional validations/builds that would be redundant if performed on the other matrix entries. I suppose weeder is inexpensive enough to redo four times, but the weeder binary is not available for download for all GHC versions (e.g. ghc 9.8.2). Something like haddock we probably only need to build once.
I have hacked this in to the generated file for weeder with a simple if condition.

@kostmo kostmo force-pushed the development/configure-weeder branch from 41a00e9 to e554ab6 Compare July 15, 2024 06:55
@xsebek
Copy link
Member

xsebek commented Jul 15, 2024

I tried it too, but gave up after the false positives. I did not notice that running HLS in VSCode has an impact, maybe I should give it a second try. 🤔

weeder.toml Show resolved Hide resolved
@kostmo kostmo force-pushed the development/configure-weeder branch 7 times, most recently from 24340a7 to 57f9e49 Compare July 17, 2024 06:36
@kostmo kostmo requested a review from byorgey July 17, 2024 07:13
@kostmo kostmo force-pushed the development/configure-weeder branch from 6767735 to c4dd1f0 Compare July 17, 2024 07:16
@kostmo kostmo marked this pull request as ready for review July 17, 2024 07:19
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

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

Great work, thanks!

My concern with editing haskell-ci.yml manually like this is that these changes might be lost next time we automatically regenerate it. At the very least, we should add some clear comments to the top of the file explaining which parts need to be left alone (we already have such a comment, about the custom on.push and on.pull-request conditions).

.github/workflows/haskell-ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

Amazing, thanks @kostmo! 👍

weeder.toml Show resolved Hide resolved
@kostmo kostmo added merge me Trigger the merge process of the Pull request. and removed merge me Trigger the merge process of the Pull request. labels Jul 17, 2024
@kostmo kostmo force-pushed the development/configure-weeder branch from 23644c4 to 9902ee7 Compare July 17, 2024 21:43
@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Jul 17, 2024
@mergify mergify bot merged commit 35975bb into main Jul 17, 2024
12 checks passed
@mergify mergify bot deleted the development/configure-weeder branch July 17, 2024 22:01
@kostmo kostmo mentioned this pull request Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use weeder for dead code analysis
3 participants