Add fn_to_numeric_cast_usize lint

[Coder-256]

changelog: Add fn_to_numeric_cast_usize lint

Fixes #5906.

In order to implement this, I had to make the design choice that casts from a function to usize should first go through a function pointer for clarity. For example:

fn fun() -> usize { 1 }
let a: usize = fun as fn() -> usize as usize;

Casts from a function pointer to a usize are still OK. Also, if you simply disable this lint, the old behavior should apply (except for error/help messages, as described below).

I also changed the error/help messages for "function as numeric" in order to more clearly explain that the function is being cast as a pointer, and suggest calling it then casting it, rather than casting a pointer to it as a usize (if I write foo as i32, I probably mean foo() as i32, not foo as usize).

Furthermore, I had to special-case the transmutes_expressible_as_ptr_casts lint in order to account for this change (recommending casting rather than calling due to the nature of transmute), and I am not 100% sure that I did it correctly. I based it on this method. This part might also need more tests.

Finally, this is my first contribution to Clippy, so please let me know if I missed anything.

---

EDIT: I've thought about this some more, and I think it would actually make more sense to remove the fn_to_numeric_cast_with_truncation lint entirely. I don't think the question of whether the cast would truncate is really relevant to this kind of lint; any cast to something other than usize is equally questionable IMO. Again, I think it's far more likely that the correct solution is to call the function and cast the result, rather than what the old help suggests: casting to a usize instead of whatever numeric were trying to cast to. I don't think I have ever needed to use a function pointer except for calling a C function.

Taking that into consideration, another solution could look something like:

Original	Lint	Help
func as i8	fn_to_numeric_cast	func() as i8, [func as fn() -> () as usize]
func as i128	fn_to_numeric_cast	func() as i128, [func as fn() -> () as usize]
func as usize	fn_to_numeric_cast_usize	func() as usize, func as fn() -> () as usize
fn_ptr as i8	fn_to_numeric_cast	fn_ptr() as i8, [fn_ptr as usize]
fn_ptr as i128	fn_to_numeric_cast	fn_ptr() as i128, [fn_ptr as usize]

The bracketed help messages are messages that I think would do more harm than good by being overly confusing (especially to new users) but should still be considered.

Casts that are allowed:

• fn_ptr as usize
• fn_ptr as usize as i8
• fn_ptr as usize as i128
• func as fn() -> ()
• func as fn() -> () as usize
• func as fn() -> () as usize as i8
• func as fn() -> () as usize as i128

I think I prefer this design better. You need to explicitly convert from a function to a function pointer, from a function pointer to a usize, and from a usize to another numeric. It's more verbose, but I think this way it is very hard to do the wrong thing by mistake. Plus, this way, the longer the cast, the less likely it is that you are doing the right thing! I'm not quite sure who makes these decisions though.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[Coder-256]

For the function item errors, I was also going to add something like "help: did you mean to cast to a function pointer?" but I'm not sure I know how to do that correctly. Should I just replace span_lint_and_sugg with span_lint_and_then, then cal diag.span_suggestion and diag.help?

[Coder-256]

Hmm, I had only guessed the 32-bit test output, and I just realized that I think I guessed wrong, but apparently CI doesn't test it(??), and the test failure is actually spurious(???).

Also, I implemented the alternate model in my fn-cast-usize-alternate branch, and I think the error messages are even better than before.

[flip1995]

to remove the fn_to_numeric_cast_with_truncation lint entirely

I disagree. I'm not sure why fn_to_numeric_cast_with_truncation and fn_to_numeric_cast are separate lints though.

Taking that into consideration, another solution could look something like:

The thing is, that Clippy suggestions should always keep the semantics of the original program, except we can be absolutely sure, that the written code is utterly wrong. We can't assume this in this case. What we can do is to suggest both, but the semantic preserving suggestion should always come first.

Should I just replace span_lint_and_sugg with span_lint_and_then, then cal diag.span_suggestion and diag.help?

Yes this is how that is done.

Hmm, I had only guessed the 32-bit test output, and I just realized that I think I guessed wrong, but apparently CI doesn't test it(??), and the test failure is actually spurious(???).

It will be tested by @bors before the PR is merged. We have an i686 (?) runner.

[flip1995]

This is unrelated?

I wouldn't do this, since the extra cast to fn() -> T is code bloat. If people prefer these explicit casts, they should enable the new lint (which should be pedantic).

[flip1995]

This should be pedantic or even restriction IMO, since this cast is usually fine.

[flip1995]

This repeats too much code.

match cast_from.kind {
    ty::FnDef(..) if cast_to.kind == ty::Uint(UintTy::Usize) => { /* lint FN_TO_NUMERIC_CAST_USIZE */ },
    ty::FnDef(..) | ty::FnPtr(_) => { /* keep old code */ },
    .. => {},
}

[flip1995]

As an additional test:

    // Casting to fn pointer and then to a `usize` is OK and should not warn
    let _ = foo as fn() -> String as usize;

[flip1995]

I don't think we should assume here, that a cast of the return value was meant. Also I think the casting function pointer ... message was clear enough that it talks about a function pointer. Also emphasizing with * or something else isn't used in error messages.

What we can do is add two suggestions, one that suggests to cast the pointer to usize and one to call the function and cast the resulting value to usize, iff it is a FnDef(..)

[flip1995]

r? @flip1995

[Coder-256]

This is waiting on discussion in #5921.

[bors]

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

[kraktus]

I think this can be closed as implemented by #7705