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

Ignoring False positives #146

Open
NorfairKing opened this issue Apr 7, 2024 · 5 comments
Open

Ignoring False positives #146

NorfairKing opened this issue Apr 7, 2024 · 5 comments

Comments

@NorfairKing
Copy link
Contributor

It would be really nice to be able to run weeder in CI.
Unfortunately it still shows many false positives, such as the Paths_ modules and TH-spliced functions.
So I propose to have a part of the config file called expected-weeds that lets you describe false positives that weeder should ignore.
It should also error when one of the expected weeds is not found, because otherwise weeds will grow in that section of the configuration file.

@ocharles
Copy link
Owner

ocharles commented Apr 7, 2024

I think you might be asking for a combination of #58, and a way to say that some rules must match - is that correct?

@NorfairKing
Copy link
Contributor Author

@ocharles yes that sounds right.

@ocharles
Copy link
Owner

ocharles commented Apr 8, 2024

Ok. I think that for TH you probably want those definitions as roots, not ignored weeds. For Paths_ that is outside your control (it's generated code), so ignoring those would be more correct. Currently we have:

roots = [
  # All `main :: IO()` functions
  '^Main.main',

  # Automatically generated and outside our control
  '^Paths_.*',
]

In our config, which you might want to copy.

@NorfairKing
Copy link
Contributor Author

Ok. I think that for TH you probably want those definitions as roots, not ignored weeds.

Alright, but then I still have a need for unused root patterns to be an error.

@ocharles
Copy link
Owner

ocharles commented Apr 8, 2024

Yea, for sure - I've also wanted weeder to be self weeding there! Maybe a nice little fix for ZuriHac 2024 🙂

mergify bot pushed a commit to swarm-game/swarm that referenced this issue Jul 17, 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](https://github.com/swarm-game/swarm/pull/2044/files#diff-73f8a010e9b95aa9fe944c316576b12c2ec961272d428e38721112ecb1c1a98bR227).
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

No branches or pull requests

2 participants