Setting up pedantic Clippy

[tombh]

I realised that setting up Clippy in VSCode is probably quite similar to my Neovim. Both editors use the rust-analyzer LSP to generate diagnostics. So my Clippy settings look like this:

settings = {
	["rust-analyzer"] = {
		checkOnSave = {
			overrideCommand = {
				"cargo",
				"clippy",
				"--message-format=json",
				"--all-targets",
				"--all-features",
				"--",
				"-W",
				"clippy::all",
				"-W",
				"clippy::pedantic",
				"-W",
				"clippy::restriction",
				"-W",
				"clippy::nursery",
				"-W",
				"clippy::cargo",
				"-W",
				"missing_docs",
			},
		},
	},
}

I think the VSCode seting might be something like:

"rust-analyzer.checkOnSave.overrideCommand": [
	"cargo",
	"clippy",
	"--message-format=json",
	"--all-targets",
	"--all-features",
	"--",
	"-W",
	"clippy::all",
	"-W",
	"clippy::pedantic",
	"-W",
	"clippy::restriction",
	"-W",
	"clippy::nursery",
	"-W",
	"clippy::cargo",
	"-W",
	"missing_docs"
]

I call this "pedantic" even though it contains a lot more than clippy::pedantic because it represents the most verbose form of feedback. Therefore, enabling this level implies at least a willingness, if not a requirement, to enable all other levels. It should be noted that it is the only level that can generate false-positives, namely, advice that isn't actually true or indeed even fixable, though that's rare.

Such strict settings are not seen as merely "training wheels". One of the most unique things about Rust is its static analysis, therefore what it does even before it runs any code. So even though it is technically accurate to call Clippy a linter, it doesn't tell the whole story: that what Rust most fundamentally is, is a sophisticated static analyser, an advanced "linter" if you will. Perhaps a more helpful way to look at Clippy, is just to consider it Rust itself.

These pedantic settings can be overwhelming, in an already overwhelming language. I would very much consider them a starting place, rather than a hard requirement. Generous usage of #[allow(...)] directives should certainly be used in any given project. My suggestion is that a developer starts from this strictest of places and allow(...) things as and when they arise and when they are properly understood.

[dgreisen]

I resolved the bulk of them. however I cannot figure out the last three:

    Checking stele v0.1.0 (C:\Users\dreisen\programming\rust\stelae)
 --> src\utils\mod.rs:1:1
  |
1 | /// The git module contains structs for interacting with git modules
  | ^
  |
  = note: `-D clippy::mod-module-files` implied by `-D warnings`
  = help: move `src\utils\mod.rs` to `src\utils.rs`

error: `mod.rs` files are not allowed, found `src\utils\git\mod.rs`
 --> src\utils\git\mod.rs:1:1
  |
1 | use git2::{Error, Repository};
  = help: move `src\utils\git\mod.rs` to `src\utils\git.rs`

 --> src\utils\git\mod.rs:2:5
  |
2 | use std::fmt;
  |     ^^^^^^^^
  |
  = note: `-D clippy::std-instead-of-alloc` implied by `-D warnings`
  = help: consider importing the item from `alloc`
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#std_instead_of_alloc

@tombh - when I switch to utils.rs/git.rs, I get an error saying to switch bag to mod. When I switch to alloc, it won't compile. I couldn't figure out how to allow either of these. Could you please let me know how to resolve these?

[tombh]

This is where we have to start to whitelist certain lints. So I think something like this at the very top of src/lib.rs will fix them:

#![allow(clippy::mod_module_files)]
#![allow(clippy::std_instead_of_alloc)]

Placing them in lib.rs applies them to the whole crate.

These mod.rs warnings are merely to encourage falling on one side of the mod.rs/self-named-mod.rs debate. These 2 lints are orthogonal: clippy::mod_module_files and clippy::self_named_module_files (BTW the site those links link to is great for getting more context on lints). I think the mod.rs convention is more common. So this is one of those cases where there was no way to solve it. Sorry if you struggled with it. That's one of the downsides to these "pedantic" Clippy settings.

I'm not certain but clippy::std_instead_of_alloc might be interesting to you if you want to reduce the amount of unnecessary external dependencies for security and preservation purposes. I think it's for projects like embedded devices where the absolute minimal amount of code is used, but to be honest I don't actually know. Hence just whitelisting it for now, using std is by far the most conventional version of Rust.

Other common whitelisted lints that I think might be relevant are:
clippy::implicit_return This is just simply idiomatic Rust now. Not allowing it would only be for projects migrated from C or something.
clippy::missing_inline_in_public_items: A compiler optimisation that this project is unlikely to benefit from.

[dgreisen]

I believe this is finally fully resolved. pedantic is used by default in vscode and ci.

[tombh]

I have some more feedback on this, but I think it'll be better in the review