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 the ability to ignore weeds #58

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add the ability to ignore weeds #58

wants to merge 2 commits into from

Conversation

ocharles
Copy link
Owner

Fixes #56

src/Weeder/Main.hs Outdated Show resolved Hide resolved

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@ocharles
Copy link
Owner Author

ocharles commented Jan 7, 2021

This is currently in limbo pending support from users. I thought I needed this, turns out I don't. But maybe others do?

@EncodePanda
Copy link

I've "just" started using weeder for my personal projects, so I don't know yet :)

@shapr
Copy link

shapr commented Oct 21, 2021

Work codebase is 500kloc and we have several example projects that aren't roots and I would like to ignore.
I've set them as roots, but I could see the benefit of ignoring Example.hs and that sort of thing!

@anka-213
Copy link

Yes, this feature would be useful.

@philderbeast
Copy link
Contributor

For haskell/cabal, it might help there being able to ignore cabal-testsuite:

$ weeder --version
weeder version 2.7.0
hie version 9063
$ weeder --write-default-config
Did not find config: wrote default config to ./weeder.toml
incompatible hie file: /.../cabal-testsuite/PackageTests/CopyHie/setup.dist/work/dist/build/extra-compilation-artifacts/hie/HieLocal.hie
    this version of weeder was compiled with GHC version 9063
    the hie files in this project were generated with GHC version 9047
    weeder must be built with the same GHC version as the project it is used on

@NorfairKing
Copy link
Contributor

NorfairKing commented Apr 7, 2024

@ocharles I definitely need this for false positives such as TH splices.

In fact, the last three times I tried using weeder, I decided not to because I couldn't ignore false-positives :P
Now I figured out that you can work around this by adding false positives as roots, but then those grow weeds as well, so requiring the ignores to match something is necessary too.

@sellout
Copy link

sellout commented Oct 6, 2024

I’ve run into a similar use case for ignoring instances.

I have

root-instances = [
    { class = '''^Data\.Foldable\.Foldable$''' },
    { class = '''^Data\.Traversable\.Traversable$''' },
    { class = '''^GHC\.Base\.Functor$''' },
    { class = '''^GHC\.Classes\.Eq$''' },
    { class = '''^GHC\.Classes\.Ord$''' },
    { class = '''^GHC\.Generics\.Generic$''' },
    { class = '''^GHC\.Generics\.Generic1$''' },
    { class = '''^GHC\.Read\.Read$''' },
    { class = '''^GHC\.Show\.Show$''' },
]

which generally works well. But if I have something like

data UnusedType = UnusedConstructor
  deriving (Eq, Ord, Read, Show)

then UnusedType isn’t reported as a weed, because it has instances that are treated as roots.

I think that type-class-roots should also ignore instead of treating them as roots. Or maybe it becomes type-class-weeds = "complain"|"ignore"|"root" or something.

With the current impl (at least 2.8.0), the following configuration

type-class-roots = true
unused-types = true

doesn’t actually warn about many unused types, because they tend to have deriving (Eq, Ord) or something on them.

My workaround is to run Weeder, make sure it’s clean, then remove root-instances from weeder.toml (or set type-class-roots = false), and run weeder | grep --invert-match '\(Instance\)'. This shows me unused types that were hidden by the instances being treated as roots.

However, ignoring instances also causes any functions that exist only to be called by those instances to now be labeled as weeds, but I think this is a minor issue.

Similarly, to “ignore” tests and benchmarks, I do the following

stack clean
stack build --test --bench --haddock --no-run-benchmarks --no-run-tests
weeder
# ensure everything is clean
stack clean
stack build
weeder
# output now contains types and terms in the “core” code that are only used by tests or benchmarks.

I partition these results into two groups – one is dead code that is tested, and so I remove the code & corresponding tests. The other group is testing/benchmarking code that should be moved to the test modules instead of in the “core” code.

I think there may be ways to identify these more subtle cases (e.g., explicitly calling out tests that refer to otherwise dead code), but having an ignore mechanism would be a great first step.

But, per my examples above, just having an ignore field that parallels roots isn’t enough. For any individual entry (including root-instances and root-modules), we should be able to indicate whether it’s an ignore or a root … or add ignore-instances and ignore-modules in addition to the ignore list.

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.

Add the ability to ignore weeds without creating roots
7 participants