Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

You're working from a false premise #1

Open
ssokolow opened this issue Nov 12, 2020 · 2 comments
Open

You're working from a false premise #1

ssokolow opened this issue Nov 12, 2020 · 2 comments

Comments

@ssokolow
Copy link

If this is used as described in your blog post, then you're operating under a false premise.

Clippy uses configuration files in the TOML format, which can have one of two names —.clippy or .clippy.toml. To obtain the data, we used the GitHub code search.

It is atypical to have a clippy configuration file because the configuration file is for configuring the lints, not enabling or disabling them, and most lints either have no configuration options or have good enough defaults.

In fact, you can't enable or disable lints in clippy.toml last I checked, and RFC 2476 was supposed to have clippy.toml made deprecated/unstable before it was distributed through rustup and rust-lang/rust-clippy#3013 tracks discussion of how to rectify that oversight in a non-breaking fashion.

doc-valid-idents is the only use I have for clippy.toml in any of my projects but, if you take a look at my rust-cli-boilerplate starter template, you'll see that I go out of my way to enable extra Clippy lints.

In fact, the doc_markdown lint (which, remember, is what consumes doc-valid-idents) is disabled by default.

I didn't check exhaustively, but I didn't see any evidence that you're parsing the Rust code in the repositories to check for allow/deny/forbid attributes on clippy::... warnings/errors and, without that, it's impossible to tell apart codebases which don't use Clippy from ones that do use it but leave the per-lint configuration keys at their defaults.

...not to mention that, if you're studying and giving advice on configuration changes away from default, it's much more meaningful to study which lints get enabled or disabled than configuration options that could even be dead weight if someone disabled the lint and didn't edit clippy.toml.

We can see that the most popular option is 10000 and also a lot of people set this option to 999999. The answer is that type complexity threshold 1000 is hard to exceed, and assigning this value to it effectively disables lint because it’s not likely that any of the types you’ll ever use will hit it.

...and I can't think of a reason to do that legitimately, because that's equivalent to setting a regular expression for valid identifiers to .* instead of disabling the lint properly. Definitely a code smell, since it could easily fool any new developer coming to the project who checks the list of enabled and disabled lints without checking clippy.toml.

@pavel-ignatovich
Copy link
Contributor

Thank you for your feedback!

It was our first attempt at research in this field, and we're delighted to see where we were not correct so that we can do it better.

I've checked the links you shared and agreed that approach based only on clippy.toml is far from exhausive. We're currently thinking about performing a more complex analysis based on inline configs, and we would be glad to receive recommendations from experienced users.

Do you think analyzing inline configurations would be enough, or should we also parse configs of CI systems like Travis CI to determine which arguments people use while running Clippy?

@ssokolow
Copy link
Author

Initially, I wouldn't have expected anything beyond inline configurations to be necessary but, after seeing how many people you found abusing cargo.toml to "disable" lints, I now expect there probably is a non-trivial subset of users who do something like invoking cargo clippy with -W/--warn, -A/--allow, -D/--deny, and -F/--forbid arguments via a justfile or Makefile or cargo-make definition or CI configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants