Config for public API changes

[camsteffen]

Sometimes we turn off lints in cases where the suggestion would change the public API of a crate. This is a reasonable default, but I believe this should be an opt-out configuration option for all Clippy lints. Oftentimes, a crate has a public API but it is still "private" in the sense that it is only used within a single project (e.g. clippy_utils), and so API changes are not as much of a concern.

I'm against silencing lints on public items. The default should be to lint everywhere. Silencing should only be done in exceptional cases and there should be an easy way to turn it back on. I actually would prefer a lint to be moved to restriction entirely than crippled in that way.

[camsteffen]

I'm against silencing lints on public items.

Just to be clear, we are not silencing lints on public items altogether, but silencing lints that would suggest changing the public API in a non-backward-compatible way (i.e. changing a function signature).

The default should be to lint everywhere.

That seems more preferable to me as well, by the principle of least surprise.

Just to be clear, we are not silencing lints on public items altogether, but silencing lints that would suggest changing the public API in a non-backward-compatible way (i.e. changing a function signature).

Yes. Aware of that.

[camsteffen]

Config name idea: suppress_public_api_changes

[matthiaskrgr]

I think it makes sense to not lint on public items by default.
Some people are already annoyed by false positives in clippy.
If someone renames half their public api to get a crate "clippy clean" and 20 other crates have to update their code afterwards to match the renamed items, this is definitely something I would want to avoid.

Having an opt-in warning that you can enable explicitly when you want to break your api anyway (for example when go from 1.0 to 2.0) sounds fine imo.

[llogiq]

Yeah, we added the exported check to many lints for exactly that reason: Clippy should not break people's code. Having a flag to allow linting those items for the outlined reasons is a good idea, though (when we decided to not lint those cases, we didn't have any config).

[camsteffen]

Discussed this in the Clippy meeting and decided to not warn by default. Though there could be exceptions if the lint is severe, such as a security issue.

We decided on the name avoid_breaking_exported_api.

Also, this should be advertised on the Readme in some fashion since this is a config option with relatively large implications and new projects will likely want to override the default. But we also would like to have some sort of Configuration Guide instead.

[Manishearth]

A thing I mentioned there that I want to make sure is mentioned here: this shouldn't actually change our calculus of whether we break public APIs. Sometimes a lint is more severe and needs to be able to suggest breaking changes, even if the result is just that the lint is silenced.

[Manishearth]

Put differently, "we shouldn't use the existence of this config as a reason to avoid writing lints that always suggest breaking changes".

So like, I think we have some lints currently that do suggest breaking changes to pub and should, we shouldn't change that.

[llogiq]

Yeah, we should also probably document this facet of the respective lints.