Make suppression explanation optional

[colinhacks]

I couldn't find any existing discussion of this, but apologies if I missed something.

I'm in the process of switching Zod's codebase over Biome and absolutely loving it. The only thing that stuck in my craw is the fact that suppression messages are required when using // biome-ignore.

For library authors specifically, there's always weird code we need to write for arcane reasons (the example below is used to implement z.instanceof). Even in application development more broadly there are a ton of cases where there isn't a good explanation to give.

It feels a bit paternalistic to require it, and in practice I'm guessing many people will just leave a trailing semi-colon or a filler message. Check out this GitHub code search query for validation of that: https://github.com/search?type=code&auto_enroll=true&q=%22%2F%2F+biome-ignore%22 It's ~40% of biome-ignore usages don't contain an informative message. Ultimately it's a DX paper cut but it triggers my OCD a bit.

Something to consider! Thanks for all the great work on Biome, it makes my life better every day.

[arendjr]

I think it’s probably a good thing the message is required. I will admit there have been times where I couldn’t come up with a good message and I filled in something silly like “It is what it is”. But every time I fill in something and usually it is useful.

The reason I like it being required is because it forces people to think. The exception is there for a reason, so there’s value in documenting it. Maybe the reason is obvious, silly, or pedantic at times, but I’d rather have those documented too than to miss out on a reason that should have been documented but wasn’t.

So there’s my crux: I think if we make reasons optional, people will less often stop to think about the reason and simply omit it because they can’t be bothered. You mention currently 40% of usages have a useless reason (I didn’t count to verify), but I think if we make them optional that percentage could easily increase to say 50% or 60%. Unfortunately we can’t really A/B test this, so it’s based on a gut feeling, but if true I think we would loose more value in the useful messages that will be omitted, than we would save by allowing people to not have to think too much.

---

All that said, we can probably improve the error message in the parser when a reason is missing.

[colinhacks]

Hey Arend, thanks for the response. I expected that answer, since this feels like a very intentional choice to require explanations.

You mention currently 40% of usages have a useless reason (I didn’t count to verify), but I think if we make them optional that percentage could easily increase to say 50% or 60%.

Honestly it would probably be more than 50-60% 😅 But I also think that's quite okay, since this is a matter of taste. It does feel unexpected for Biome to take such an opinionated stance here. The current behavior is known to be a "papercut" to (roughly) 40% of users. I'd venture to say a very small fraction of users would have the inverse reaction: being annoyed that Biome doesn't require an explanation. The right call depends on what metrics you're optimizing for.

Or perhaps the rightest answer is to do what Biome does with other "matters of taste" and make it a flag. 🤷‍♂️

[ematipico]

Thank you Colin for raising the argument.

There's no previous discussion because it's always been like this since the beginning, and now that adoption increases, I think it's totally normal to debate decision.

Arend explained quite well the reasons behind the decision.

An explanation can be useful for various reasons:

• the rule is buggy, and it's possible to add the URL to the issue
• there's an exception, the developer should document it and explain why it's safe to bypass the check

An explanation can become tedious for personal projects, where there's only one developer. However, an explanation can become very useful in codebases maintained by various developers: context, knowledge base and exceptions, aren't missed or forgotten.

I know requiring an explanation can be very controversial, depending on where a developer come from, however I believe that this, on the long run, can enforce a higher quality in the projects.

---

As a personal note, I have been pushing people and co-workers to add an explanation every time they turn off an eslint rule, use @ts-ignore or use @ts-expect-error. They honestly find it very usefuls because it helps them to review the PRs, and they started to do the same towards me and other contributors/co-workers.

[colinhacks]

Thanks Emanuele! I'll leave some final thoughts here.

Whether or not providing an explanation is a good practice is almost beside the point (of course it is!). The more relevant questions are:

1. does requiring explanations make code quality trend towards the better or towards the worse on average?
2. does requiring explanations make over developer experience better or worse on average?

I think neither of these are really answerable, but there is evidence that both DX and code quality suffer in a very sizable fraction of codebases. I think you'll agree that lots of nonsense or filler explanations is a bad code hygiene.

You guys have done a better job than me at listing scenarios where an explanation may not be useful to the developer: personal projects, self-evident explanations, etc. The "personal project" argument is particularly compelling. The vast majority of codebases are only ever touched by one person, so Biome should perhaps provide some affordance for that scenario, or even prioritize it.

I know requiring an explanation can be very controversial, depending on where a developer come from, however I believe that this, on the long run, can enforce a higher quality in the projects.

I don't know how these decisions get made, but for anything controversial like this, would adding a flag be an appropriate solution?

[ematipico]

The harm to maintainability doesn't come from the filler explanations - it comes from frustrated developers opting to disable the rule entirely.

@morgante How so? From my point of view, if the user is forced to suppress a rule multiple times in the same file/files, maybe that rule isn't needed for those particular file/files and should be turned off.

but there is evidence that both DX and code quality suffer in a very sizable fraction of codebases

@colinhacks Personal experience, as I shared before. It's impossible to give evidence of said fact, and it's impossible to proof the contrary. In the end, it all boils down to our experiences and where we come from.

I feel strongly about the explanation bit, however, I am willing to compromise and see if we come to an understanding or a solution that makes sense. What if Biome emits a diagnostic (info or hint) every time there's no explanation for a lint rule?

[arendjr]

I agree a (recommended) lint rule that reports on missing explanations might be a good middle ground. We wouldn’t need any new options and we still have (by default) the behavior we desire, and those that don’t like it can still disable it.

[colinhacks]

I'd love that. Any mechanism for opting out of required explanations would make me happy, I'll defer to you on the details. 👍

[morgante]

@morgante How so? From my point of view, if the user is forced to suppress a rule multiple times in the same file/files, maybe that rule isn't needed for those particular file/files and should be turned off.

The problem is more often that's a particular pattern of usage that is a false positive / worth ignoring, but then many other cases where it's not.

Anyways, I think the idea of just making this itself a lint rule is a good compromise.

[ematipico]

I'm not sure having a lint rule for that is the right solution. Suppression comments span across tools AND languages, and lint rules work per language.

The quickest win is via analyzer, but not via the linting rules.

[morgante]

I would be in favor of making this optional. I actually originally required comments for all suppression comments in Grit and relaxed it after feedback from customers (even though theoretically GritQL makes it easy to custom exclusions).

Every lint rule has a cost in DX and people will only continue using rules that have a benefit which outweighs the cost. Requiring an exaplanation for every ignore (especially since many codebases end up with common ignore patterns for specific rules) increases that cost.

The end result can easily be worse code since developers opt to remove the rule entirely instead of adding lots of filler ignore lines.

[ematipico]

As I said before, the explanation inside suppression comments can be good and bad at the same time.

While I follow your reasoning, it completely ignores the fact that a code base might be owned by a team of people, and that's where the value of explanation comes into play. In my opinion, the small cost of DX that you explained is completely overshadowed by the value that this explanation gives to the developers and teammates after you. A piece of code is read more than it is written, after all. Also, in case some code changes, the explanation can give important insights.

[morgante]

While I follow your reasoning, it completely ignores the fact that a code base might be owned by a team of people, and that's where the value of explanation comes into play.

I'm not sure where I said a codebase isn't owned by a team? I'm talking entirely about teams here, so I don't know why you're putting words in my mouth.

[ematipico]

You didn't. I said it might :) I am not putting any words in your mouth.

[morgante]

Ok, but I'm not "ignoring the fact that a code base might be owned by a team" — my entire perspective is grounded in team codebases, where there is still a cost/benefit tradeoff for any tool.