as_conversions triggers on derived code

[RReverser]

Summary

I like using as_conversions as a catch-all guard to stop myself from reaching for it in my own code when better solutions are available. However, it triggers on the generated derive code too, e.g. when using serde-repr.

Worse, it can't even be disabled by putting #[allow(clippy::as_conversions)] on the offending type, as it doesn't propagate to the derive-generated code, so the only option is to opt-out at the module level.

Lint Name

as_conversions

Reproducer

I tried this code:

#![warn(clippy::as_conversions)]

use serde_repr::{Deserialize_repr, Serialize_repr};

#[derive(Serialize_repr, Deserialize_repr)]
#[repr(i32)]
pub enum State {
    Error = -1,
    Idle = 0,
    Waiting = 1,
    Done = 2,
}

I saw this happen:

using a potentially dangerous silent `as` conversion
consider using a safe wrapper for this conversion
for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#as_conversions

I expected to see this happen:

(no warning, as the as conversion is used internally by an external crate)

Version

rustc 1.64.0 (a55dd71d5 2022-09-19)
binary: rustc
commit-hash: a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52
commit-date: 2022-09-19
host: x86_64-pc-windows-msvc
release: 1.64.0
LLVM version: 14.0.6

Additional Labels

@rustbot label +T-macros

[kraktus]

root issue: https://github.com/dtolnay/serde-repr/blob/78e7199f79bc0ad441fcd8aaa1b2d8a41f3672c7/src/lib.rs#L58

I'm not sure why this is not considered as in_external_macro, I assume because it reuses root spans.

[RReverser]

root issue: dtolnay/serde-repr@78e7199/src/lib.rs#L58

I'm not sure why this is not considered as in_external_macro, I assume because it reuses root spans.

The as itself doesn't come from the source though, I guess clippy should be looking at the keyword's span but doesn't currently.

[RReverser]

Yeah I'm not familiar with clippy code, but looking at this it seems that it only check entire expression's span, which essentially means "from the first token to the last token":

rust-clippy/clippy_lints/src/as_conversions.rs

Line 50 in b72e451

if in_external_macro(cx.sess(), expr.span) {

However, in case like serde-repr's (#ident::#variant as #repr), first and last tokens both have spans in the user code, so the span is assumed to be entirely in the user's code too, despite the most important part - as keyword in the middle - coming from an external macro.

Beyond those guesses, I'm not even sure what the fix would look like / how to check for the keyword's span specifically.

[Jarcho]

This might need is_from_proc_macro. The span for e as ty should use the context from as not from the expression or the type.

[lochetti]

@rustbot claim

[lochetti]

@Jarcho to use is_from_proc_macro we would have to change it from EarlyLintPass to LateLintPass am I correct?

[Jarcho]

You can also implement WithSearchPat for all the needed ast types.