Default lints should correspond to rustc

[dhardy]

Summary

I would expect that rust-analyzer, with default settings, would report the same lints (warnings, errors, etc.) as rustc

Details

Currently this is not the case; for example I also see messages about these:

• inactive-code — this may have a special purpose in some editors? In Kate it's more annoying than useful, but that may be just Kate.
• remove-unnecessary-else — nearest equivalent is Clippy's redundant_else?

The default enabled lints also do not correspond to Clippy's default lints (there is a note in the manual about enabling Clippy by default).

Motivation

I would (naively) expect that a project which is clean (reports no lints) under cargo check is also clean under rust-analyzer, and vice-versa. This makes it easier for developers using different tools to collaborate on a project.

Further motivation: the rust-analyzer project should probably not be responsible for choosing which lints are enabled by default (especially when both rustc and clippy already do this).

[dhardy]

Possibly related:

• configure rust-analyzer lints with Cargo [lints] table #16483
• Rust analyzer does not respects project-level clippy directives? #16419

[flodiebold]

These lints are all "WeakWarning" level, which VSCode shows in a very inconspicuous way, just some dots to give a hint you can do something there; the inactive-code is also marked as unused, which makes it be shown still differently, just darkening the code a bit:

In general I agree that for warnings/errors we emit that emulate corresponding rustc/clippy ones we should honor their lint settings, and we aren't quite there yet. These hints are a different category though.

[dhardy]

The specification doesn't list a "WeakWarning" level. I think non-standard extensions should be either disabled by default or gated to editors known to support them?

Or is the LSP standard too dated to be useful now (but surely it should also correspond to what VSCode supports since they're both MS products)?

(This should probably be forked to a new issue.)

---

That very inconspicuous hint sounds fine for inactive-code. Given that this is not a critique of the code but merely a notification of the current configuration, it does make sense that this is part of rust-analyzer and not rustc or Clippy.

The remove-unnecessary-else lint though, I still don't think should be here: it's a pedantic style lint, and thus falls into Clippy's remit.

[flodiebold]

Sorry, we apparently call "WeakWarning" what's called "Hint" on the LSP level. It's not a non-standard extension.

[flodiebold]

I don't know. I think these hints are more intended as "there's a useful thing to do here", not necessarily that there's something wrong. And you might not be using Clippy at all, so I don't think we should only go by what's enabled for Clippy. OTOH we should probably honor if a similar lint is explicitly disabled for Clippy.

[dhardy]

"there's a useful thing to do here"

This is very opinionated (the "useful" part). Your example above (where the else block is empty) is suspicious, and Clippy reports needless_else. cargo check doesn't report this, presumably because it ignores comments and there could be a comment there (which is enough to suppress the Clippy warning). My motivating example had code in the else block, at which point removal of the else is a stylistic change:

More generally,

• rustc mostly tries not to be opinionated beyond concerns of correctness and safety (with some exceptions, e.g. CamelCase names).
• Clippy is intentionally an opinionated tool (concerning style, potentially-confusing patterns and more). Some projects use it, some use it with customized lints, some don't use it at all.
• rust-analyzer, as a tool intended for usage everywhere, should (in my opinion) be minimally opinionated to minimize the difference in experience between those who use it and those who don't.

There is a counter-argument that certain things should be hinted by rustc if there were a minimally-conspicious way of doing so, and thus rust-analyzer can pick up these missing hints. If that is what you are going for, then these hints should be minimally opinionated.

[flodiebold]

Why would we want to "minimize the difference in experience between those who use [rust-analyzer] and those who don't"? You can always turn off any of these diagnostics using the rust-analyzer.diagnostics.disabled setting, so you can configure the experience to your personal liking.

[dhardy]

Sorry, I mean to minimize the difference in reported lints, so that usage of rust-analyzer is an individual decision, not a project-level decision.

[shssoichiro]

You can always turn off any of these diagnostics using the rust-analyzer.diagnostics.disabled setting, so you can configure the experience to your personal liking.

It seems strange to me that you would need to do this extra step. I think most users who are using rust-analyzer in VS Code would expect it to report the same things as running cargo check or cargo clippy. I thought my lint overrides in my Cargo.toml were not configured correctly until I noticed that the overrides applied in cargo check but not in my IDE. This is, in my opinion, a very unexpected and undesirable behavior.