feat: custom explanations with --suppress

[anthonyshew]

Summary

#4008 introduces a --suppress flag, but it would add ignore comments using the default explanation (<explanation>). Since --suppress bulk ignores existing diagnostics, you'd have to go back through your changes to give a more worthwhile explanation (or accept <explanation> being in your codebase). Instead, this PR sets a more informative default message of "Ignored using --suppress".

Additionally, you can now add a custom explanation with --reason. This flag can only be used when you use --suppress.

Test Plan

Updated tests to use this default message. I've also manually tested the binary against a few of my repositories.

[anthonyshew]

I felt like it helped to add "Bulk" here to portray that you'd want to use this for migration purposes. Can take it off if desired.

[anthonyshew]

I've verified that this works by writing a few more tests and running it against a few repositories I have. I'm admittedly not 100% certain the way that I've written it is the best way to go about it. Ema let me know to go through AnalyzerOptions, but not sure about the rest of what I have.

[ematipico]

Overall it looks good. We should avoid .clone, and rely on references where is possible.

[anthonyshew]

I was using String so I could avoid playing "The Lifetime Game" while I was trying to get the codepaths right. 😄

Now that I'm doing the lifetime-ing, the error you see on the build is something I honestly have no idea how to contend with. It might take me awhile to sort this out. Also noteworthy that there's a bunch of changes due to putting lifetimes in a bunch of places, so possibly worth another more general look over things.

[ematipico]

Ok, then let's keep String inside AnalyzerOptions, having a lifetime there doesn't make sense.

Push the change and then I'll have a look if you still have lifetime issues

[ematipico]

@anthonyshew I optimised some code here 3e9a369 (#4343), the was some unneeded allocations

[anthonyshew]

Thanks! I had just caught onto that while I was looking for a few more places to put string slices. You beat me to it. 🤝

It seems like we're in a good spot now on String vs. &str. bpaf needs a String so it feels like it makes sense to let it keep being String in the CLI crate. It's only gets passed through a couple layers to get to AnalyzerOptions after that? We definitely can bring it to a &str and back to String. It felt a bit silly when I started doing it so asking first.

After it makes it through the options, it's a &str now in the file handlers, suppression actions, etc.