Try   HackMD

Summary

The current clippy.toml file is currently unstable. However, this file has been the de-facto way to configure Clippy
for a long time. This RFC proposes a stabilization strategy for this file, so that Clippy users can better rely on it.

Motivation

Many projects, including Rust-for-Linux, are using the clippy.toml file to configure Clippy. Until now, we didn't
provide any guarantees about this configuration file, so that we can be most flexible when maintaining it. This comes at
the cost of the Clippy users. By stabilizing the existence of the clippy.toml file, and a minimal subset of its
configurations, we can achieve a middle ground: some guarantees for the users, while allowing us flexibility to change
non-stabilized parts.

Guide-level explanation

Kinda included in the reference-level-explanation. Should this become a real RFC, I'll clean that up.

Reference-level explanation

Stabilize the file

The first step is to stabilize the existence of this configuration file. So Clippy will guarantee that it can be
configured with a clippy.toml or a .clippy.toml file.

This is also a good time to revisit the file discovery. Currently, it is discovered through the CLIPPY_CONF_DIR env
variable, the CARGO_MANIFEST_DIR env variable, or the current path in that order. The last part could be adapted to
what rustfmt does:

  • Search every parent directory
  • Look in the $HOME dir or in the .config/clippy (Windows/Mac?) dir

This also means, that the CLIPPY_CONF_DIR env variable needs to be stabilized in Clippy.

Stabilize the content

At the time of writing, Clippy can be configured with over 80 different configuration values, that either configure a
single lint or apply to multiple lints. This RFC doesn't propose the stabilization of any of those. Instead, a process
is introduced on how those configuration values can be stabilized.

To stabilize a configuration value, a proposal needs to be made on the #clippy Zulip
channel. The stabilization of this configuration value
needs to be accepted by the Clippy maintainers and there can't be a veto from a maintainer.

When considering the stabilization of the value, certain criteria should be considered:

  • How long does this configuration value already exist (unchanged)?
  • How much is it used in practice? Is it "battle tested"?
  • Is it tied to a specific lint? If so, how stable and long lived is the lint itself?
  • How confident are we that the configuration can stay unchanged for a long time?

For stable configuration values, Clippy will provide the same loose stability guarantees as for lints: If they are
deprecated or renamed, a warning is emitted, but Clippy continues working.

Examples for good candidates for stabilization would be msrv and avoid-breaking-exported-api.

Error handling

Currently Clippy is really sensitive, when it sees something unexpected in the config file:

  1. If a deprecated config is used, a warning is emitted
  2. If an unknown config is used, an error is emitted
  3. If a config is duplicated, an error is emitted
  4. If a deprecated config and it's replacement is used, an error is emitted
  5. If the toml file can't be parsed for some reason, an error is emitted

Those warnings and errors are emitted through the
DiagCtxtHandle, not through
the lint infrastructure. That means there is no way to silence those warnings or errors.

Especially 2.-4. are a problem for crates that want to use multiple Rust versions with the same configuration file. This
RFC proposes to change the error handling to:

To provide flexibility for users to handle config parsing errors, the errors and warnings emitted during parsing will be
replaced with lints. This can be done by passing on the TryConf, instead of the Conf to clippy_lints. The
ConfError struct might need to be extended to an enum, that can be easily mapped to the corresponding lint for this
error by clippy_lints.

Unknown Configs

One use case presented by R4L is using the same clippy.toml file for multiple Rust versions. Currently, this would
mean that only configuration options can be used, that already existed in the MSRV. If a new config option should then
be used, that's only available (and relevant) for a newer toolchain, older Clippy versions wouldn't accept this config
file.

Being able to allow unknown configs would automatically be possible by converting the errors to deny lints. However,
this would mean that also typos won't get detected. To address this, a new error class is introduced that checks the
Levensthein distance of the specified config option and the available ones. If that distance is small (TBD), it is
classified as a typo and handled by a different lint, than the "unknown config" case. This can use the same
infrastructure as the unknown-lints lint in rustc.

Deprecated Configs

Currently, if a config is renamed and both the new and the old name is used (case 4.), an error is emitted. This should
be demoted to a warn-by-default lint. If this happens, a warning should be emitted and only the config specified in the
new name should be considered.

By then disabling this lint and the unknown config lint, the same file with multiple versions of a renamed config can be
used and will work with all future rust versions: In versions before the config was renamed, the old name and config is
used and the new one gets ignored as an unknown config. In newer versions, the old name and config will be ignored

Parsing errors

If parsing fails for a known config, the default value or the first valid config value will be used. Parsing failures
will trigger a deny-by-default lint though.

This behavior, together with disabling the duplicated configs lint, should allow users to provide configurations that
are valid in different Clippy versions.

Examples

The following examples illustrate the behavior of Clippy, assuming the lint for parsing errors and for duplicated
configs are disabled:

msrv = true

true is not a valid type for the msrv configuration option, so the default value None is used for this
configuration.

msrv = true
msrv = "1.54.0"

Same as above, but a valid configuration is given the second time the msrv is configured, so 1.54.0 will be used for
this configuration.

msrv = "1.54.0"
msrv = true

The first configuration of the msrv is valid, so the second one will be ignored and 1.54.0 will be used for this
configuration.

msrv = "1.54.0"
msrv = "1.63.0"

Both configurations are valid for this configuration option. In this case, only the first one will be used all following
configurations ignored, so that 1.54.0 will be used for this configuration.

Drawbacks

???

Rationale and alternatives

  • Instead of converting the errors and warnings to lints, we could a immediately stable config option, that disables
    config parsing errors. The downside of this approach would be, that this option either disables all or no configs, or
    we need to add a configuration option per possible error case. The lint infrastructure already provides this
    functionality.

Prior art

  • Rustfmt: rustfmt.toml
  • rust-analyzer: rust-project.toml

Unresolved questions

  • Should a new lint group (named config) be introduced for the config lints?
    • In Clippy, groups set the lint level, but for config parsing, we might want different lint levels, depending on
      the severity of the parsing error.

Future possibilities

???

Last changed by 
 
Philipp Krones
0
51
Published on HackMD