Fix ICE in sugg::DerefDelegate with (named) closures

[anall]

rustc comiler internals helpfully tell us how to fix the issue:

  to get the signature of a closure, use `substs.as_closure().sig()` not `fn_sig()`

Fixes ICE in #9041

This also makes this code in sugg::DerefDelegate match a different use typ.fn_sig(…) I found: in mixed_read_write_in_expression -- being strict on the value of typ.kind() will hopefully reduce any future possibility of ICE crashes in this area.

---

changelog: none

[rust-highfive]

r? @flip1995

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

[Jarcho]

You can use clippy_utils::ty::expr_sig here. It will also handle impl Fn and dyn Fn types. You would need to either write an iterator for it or access all the arguments by index.

[anall]

I do have that change prototyped out, but I am not sure how confident I feel in making it, as no tests seem to exercise the ExprKind::MethodCall match branch, which I also need to change for your suggested change.

No tests start failing if I comment out that match branch, allowing the catchall to return false

[anall]

Unless I am doing something wrong, expr_sig doesn't seem to handle impl Fn:

fn make_arg_no_deref_impl() -> impl Fn(&&u32) -> bool {
    move |x: &&u32| **x == 78
}

fn main() {
    let v = vec![3,2,1,0];

    let _ = v.iter().find(|x: &&u32| (make_arg_no_deref_impl())(x)).is_none();
}

This ends up with kind being Opaque, which clippy_utils::ty::fn_sig does not handle.
This change currently turns this from what is currently ICE into an invalid suggestion.

Attempting to add support for Opaque to fn_sig does make the search-is-some lint give a correct suggestion.

expr_sig also doesn't seem to handle Boxed dyn Fn.

fn make_arg_no_deref_dyn() -> Box<dyn Fn(&&u32) -> bool> {
    Box::new( move |x: &&u32| **x == 78 )
}

…
let _ = v.iter().find(|x: &&u32| (make_arg_no_deref_dyn())(x)).is_none();
…

This ends up in another incorrect suggestion [with ty.kind being Adt].
If I manually deref the box … (*make_arg_no_deref_dyn())(x) … the involved lint does now give a correct suggestion.

I'm not exactly sure how to handle Box

[Jarcho]

By impl Fn() I meant as a function argument. Making opaque types work is a bonus.

Box can be handled by recursing on the inner type in ty_sig. It doesn't handle the general problem of types implementing any of the Fn* traits, but there isn't a stable way to do that.

If you can't make some form work it's fine to leave it broken and open an issue for it. A bad suggestion is still an improvement over an ICE.

[Jarcho]

Does the change for Box work. It doesn't look like there's a test.

[anall]

Does the change for Box work. It doesn't look like there's a test.

This was an accidental/incomplete push, but the change does seem to work with the correct test.

[Jarcho]

Looks good. You can squash the last couple of commits.

[anall]

Squashed

[Jarcho]

Thank you.

@bors r+

[bors]

📌 Commit 782b484 has been approved by Jarcho

It is now in the queue for this repository.

[bors]

⌛ Testing commit 782b484 with merge 2058b92...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Jarcho
Pushing 2058b92 to master...

[flip1995]

Thanks for reviewing this @Jarcho!

[anall]

Looks like I forgot to flag this commit to close #9041 after being merged, and I can't seem to close that issue myself.