Re-write shadow lints

[camsteffen]

changelog: Move shadow_unrelated to restriction
changelog: The shadow lints find a lot more shadows and are not limited to certain patterns

Drastically simplifies the implementation. Catches a lot more cases.

I removed the "initialization happens here" note. It is not helpful IMO.

Closes #318
Fixes #2890
Fixes #6563
Fixes #7588
Fixes #7620

[rust-highfive]

r? @llogiq

(rust-highfive has picked a reviewer for you, use r? to override)

[ghost]

About recategorizing:

I think we should have some kind of shadow lint on by default as it is a source of bugs.

I thought a while ago that we could have a more permissive shadow lint that only lints when the two bindings are unrelated and have the same type. It's more likely you'll get a compiler error than a bug if you confuse two bindings with different types. Maybe you can try this variant?

[llogiq]

I like both this simplification and @mikerite's idea of only linting on same-type shadows.

It still should be either a pedantic or restriction lint. And perhaps we could reinstate the shadow lint group that was lost when we created the other grouos.

[camsteffen]

I can imagine the same-type lint would be more accurate.

Not so much for Clippy since we keep descending the HIR and shadowing expr and pat for example (and I think that is fine), but that's more specific to Clippy.

Note there is yet another suggestion #3433 for a new shadow lint. I would call that one usage_after_shadowed. So we could potentially have five shadow lints altogether. Each one makes sense, but I'm concerned that we have a proliferation of heuristics for shadowing and they all get a new lint.

Here's another idea. We could have one lint called shadow with these config options:

shadow-allow-related: same | reuse | change-type | none
shadow-require-unused: true | false

same, reuse and none correspond to the current shadow lints. change-type is @mikerite's idea. shadow-require-unused is #3433 - require that the shadowed binding is not used after the shadow goes out of scope.

[llogiq]

The problem with this config usage is that it's one-size-fits all and there is no way to allow(shadow) partially.

[camsteffen]

Can you give an example? The config seems to give enough control to me. But maybe I need to explain it...

The shadow-allow-related config creates a sliding scale of permissiveness. You are forced to pick a point on that scale and I think that is good. "How much does the shadow have to be related to be allowed?" Each option listed is one step more permissive than the previous one and includes the previous options. So shadow-allow-related: reuse means allow(shadow_same, shadow_reuse) and warn(shadow_unrelated, shadow_unrelated_same_type) Without the config, you are able to have a setup that doesn't make sense. For example, it doesn't make sense to just have #![warn(shadow_reuse)] because an instance of either shadow_unrelated or shadow_unrelated_same_type is a "worse" shadow and it won't be linted.

After this PR, you should be able to #[allow] on a single statement that creates a shadow (but I'll need to test that).

[llogiq]

How do you configure the items where the lints apply or not?

[camsteffen]

You can do #[allow(clippy::shadow)] on a function or a statement. Is that not enough?

[llogiq]

But what if I want to allow only some variants in one part of the code and other variants elsewhere?

[camsteffen]

I guess that level of tweaking just doesn't seem valuable to me. But I understand it would be a slight loss of functionality and that could frustrate users.

[bors]

☔ The latest upstream changes (presumably #7604) made this pull request unmergeable. Please resolve the merge conflicts.

[llogiq]

@camsteffen any news on this PR? What's your plan going forward?

[camsteffen]

@llogiq thanks for the ping. This has been in the back of my mind. If it's okay with you, I would like to make the shadow lints restriction (just move shadow_unrelated) and leave a possible allow-by-default lint to a future enhancement.

[camsteffen]

@llogiq @rustbot ready

[bors]

☔ The latest upstream changes (presumably #7673) made this pull request unmergeable. Please resolve the merge conflicts.

[llogiq]

r=me after rebase

[camsteffen]

@bors r=llogic

[bors]

📌 Commit ac5b02d has been approved by llogic

[bors]

⌛ Testing commit ac5b02d with merge 75167bb...

[bors]

💔 Test failed - checks-action_test

[camsteffen]

@bors r=llogic

[bors]

📌 Commit a17359c has been approved by llogic

[bors]

⌛ Testing commit a17359c with merge f8303ad...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogic
Pushing f8303ad to master...