let_unit_with_type_underscore: make early-pass

[ada4a]

This kind of supersedes #15386 -- by making the lint early-pass, we get access to TyKind::Parens that surround the type ascription. And with that, we can return to the simpler calculation of span_to_remove.

The biggest hurdle was is_from_proc_macro -- calling that function required me to impl WithSearchPat for rustc_ast::Ty, i.e. ast_ty_search_pat, which I based on ty_search_pat. Since that's a larger change, I could extract it into a PR of its own.

changelog: none

[rustbot]

r? @samueltardieu

rustbot has assigned @samueltardieu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[samueltardieu]

I won't be able to properly review this PR in the next two weeks, I'll reroll the dice.
r? clippy

[Jarcho]

This doesn't work the local and type are in different contexts. The old one is the correct way to do this.

[ada4a]

That's a bit unfortunate, because removing span_to_remove was kind of the whole purpose of this PR.. I'm the one who added it in #15386

[ada4a]

This doesn't work [if] the local and type are in different contexts

Actually, we don't even lint if that's the case:

rust-clippy/clippy_lints/src/let_with_type_underscore.rs

Line 33 in aa8b09d

&& local.span.eq_ctxt(ty.span)

So I think this is fine after all?

Or do you think we can instead remove that check now? I tried doing that, then I needed to modfify span_to_remove to be ty.span.with_lo(local.pat.span.source_callsite().hi()), but then all of the following test cases passed at least:

macro_rules! foo {
    () => {
        (a)
    };
};
macro_rules! bar {
    () => {
        _
    };
};

let (a): bar!() = 0;
//~^ let_with_type_underscore
let foo!(): _ = 0;
//~^ let_with_type_underscore
let foo!(): bar!() = 0;
//~^ let_with_type_underscore

gives:

error: variable declared with type underscore
  --> tests/ui/let_with_type_underscore.rs:71:5
   |
LL |     let (a): bar!() = 0;
   |     ^^^^^^^^^^^^^^^^^^^^
   |
help: remove the explicit type `_` declaration
   |
LL -     let (a): bar!() = 0;
LL +     let (a) = 0;
   |

error: variable declared with type underscore
  --> tests/ui/let_with_type_underscore.rs:73:5
   |
LL |     let foo!(): _ = 0;
   |     ^^^^^^^^^^^^^^^^^^
   |
help: remove the explicit type `_` declaration
   |
LL -     let foo!(): _ = 0;
LL +     let foo!() = 0;
   |

error: variable declared with type underscore
  --> tests/ui/let_with_type_underscore.rs:75:5
   |
LL |     let foo!(): bar!() = 0;
   |     ^^^^^^^^^^^^^^^^^^^^^^^
   |
help: remove the explicit type `_` declaration
   |
LL -     let foo!(): bar!() = 0;
LL +     let foo!() = 0;
   |

But:

• the error message mentions _ instead of bar!()
• this could be seen as a feature addition

So I'd say leaving the eq_ctxt in and simplifying span_to_remove is good enough?

[Jarcho]

Sorry, I meant the pattern and type. e.g. let $name: _

[ada4a]

Then maybe that's what the check should've been?

[Jarcho]

This should be the last argument, or "(" if there are none. Closing parenthesis are stripped from the end before matching.

[Jarcho]

Should be "..".

[Jarcho]

This allow shouldn't be needed anymore.

[ada4a]

how so?..

[Jarcho]

I thought there was a PR that blocked the lint if there was a comment.

edit: I thought you had copied that from somewhere else in the file.

[ada4a]

Hm, I rebased onto latest master, and it still lints.. but now that I've already done the rebase, I guess I might just leave it in

[Jarcho]

This should match the path at the start and nothing at the end. The closing parenthesis can't be reliably detected.

[ada4a]

I knew that parentheses get trimmed, but I thought that'd be the case only for paired ones? Turning foo!(bar, baz) into foo!(bar, baz feels.. wrong. But sure

[Jarcho]

They're trimmed because the HIR tree doesn't have a separate node for parenthesis making ty and (ty) appear the same. The AST does have them as a node, but since the machinery is currently shared...

I'm working on getting rid of the need for is_from_proc_macro.

[ada4a]

oh, best of luck to you then!

[ada4a]

this should address all points except #15458 (comment)