Support bulk suppressions

[webpro]

This pull request introduces a bulk suppressions system in Knip.

The ignore* configuration options are a means to get rid of false positives (i.e. when Knip is wrong), not for actual issues you want to deal with later. Suppressions aims to fix this.

For instance, Knip doesn't read unused files at all, so they can't be suppressed other than using ignoreFiles (or ignore) patterns — even though they might not be false positives.

Large projects with lots of issues should benefit significantly from a more iterative and managed flow to deal with issues as reported by Knip. For instance, teams can tackle issues by issue type and monorepo workspaces (e.g. there's commands for "today we're going to fix unused exports in a particular workspace").

Please head over to https://feat-bulk-suppressions.knip.pages.dev/features/bulk-suppressions for initial documentation including examples and possible workflows.

This pull request has been opened to discuss feedback. Feel free to express ideas, questions, concerns, etc.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/knip@1532

npm i https://pkg.pr.new/@knip/language-server@1532

npm i https://pkg.pr.new/@knip/mcp@1532

commit: fe84779

[jfmengels]

A lot of the decisions made here are the same I chose for elm-review, so I feel like this will be really nice 👍
Especially the automatic pruning of unnecessary suppressions.

What I'm not sure from the docs whether one can go and edit it manually (to tackle the next removal for instance) or if that would cause problems for Knip.

knip --check-suppressions

Does this only check suppressions are up to date, or does it also run knip? In other words, should people run knip ; knip --check-suppressions or just knip --check-suppressions? I find it a bit unclear in the docs.

Expiry

One feature I'm worried about (that I also considered but ended up rejecting) is the expiry feature. I believe that this can be useful, but it also means that at some point, builds will simply start failing after the expiry date has been reached. Here are two scenarios off the top of my head:

• You're working on a large team, and suddenly the build fails for everyone (including for PRs that don't touch the code looked at by Knip). This could create some chaos, or if that happens at inopportune moments a painful deployment process.
• Tests for older tags/versions suddenly start failing also. This means that rollbacks or making a new patch release for an older version can be more painful.

In order to be more helpful, I'm guessing some kind of message saying "Warning: Some suppressed errors will be reported in 5 days!" could be useful in order to not be a total surprise. But:

• Having new logs appear conditionally is not always welcome by users/systems
• It might be lost in large test suite logs (or in CI, who looks at them when it's green?)
• It might spook users who don't know Knip well who will raise alarms to the rest of the company (and the managers!)

I believe these are pain points that make the feature potentially more painful than helpful, hence why I decided against it.

If you decide to keep this feature: I don't know if you indicate why something was unsuppressed, but it's probably good to indicate in this case that the given date was reached.

Great work for the rest, this is a really nice feature! ❤️

[webpro]

Thanks a lot, those are all great points. Let me address a few right away:

• The JSON file can be edited manually. Extra props will be preserved.
• The --check-suppressions runs along with Knip to verify actual status quo, so requires just the single run to serve its purpose.
• Adding in the until prop for expiry is deliberate and I feel like this feature is only relevant for power users that consciously take the consequences upon them (and their team) if they use both the --suppress-all and --suppress-until flags. That said, additional affordances and output like a timely warning prior to expiry might be helpful indeed.

I'm going to let it sink in for a bit more and update docs and perhaps implementation as well accordingly. Thanks!

[sazzer]

One concern I have with this is that it requires all of the suppressions to be done in the same file, which can be painful for merging. Being able to do exactly this but in JSDoc form (or similar) would then mean that merges are much cleaner and the suppressions are kept with the context of what's being suppressed.

[webpro]

That's what the non-standard arbitrary tags are for: https://knip.dev/reference/jsdoc-tsdoc-tags#tags (as mentioned here: https://feat-bulk-suppressions.knip.pages.dev/features/bulk-suppressions#suppressions-vs-jsdoc-tags).

[sazzer]

That's what the non-standard arbitrary tags are for: https://knip.dev/reference/jsdoc-tsdoc-tags#tags (as mentioned here: https://feat-bulk-suppressions.knip.pages.dev/features/bulk-suppressions#suppressions-vs-jsdoc-tags).

But these don't support Knip warning / erroring if they're no longer necessary, do they?

If they do then that's exactly what I'm looking for :)

Cheers

[webpro]

But these don't support Knip warning / erroring if they're no longer necessary, do they?

They do! Was poorly documented, just extended https://knip.dev/reference/jsdoc-tsdoc-tags#tags