parsed_string_literals: new lint

[samueltardieu]

This lint detects parsing of string literals into IP addresses when they are known correct.

changelog: [parsed_string_literals]: new lint

Inspired by this Zulip comment.

[rustbot]

r? @Jarcho

rustbot has assigned @Jarcho.
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

[Jarcho]

Is there any reason to limit linting this to just ip addresses? Any call to parse on a string literal has the same issue regardless of the type. The suggestion would have to be specialised per type so it could only exist for a subset of cases. Possibly a config to ignore specific types, but I'm not sure if this would be needed.

[samueltardieu]

I guess this could be extended, but at this stage I'm not sure we should make a generic lint for this. Specialized lints can be collectively renamed into a more generic one if we don't see one of them being disabled in isolation. I'd be more comfortable with a separate lint for the time being.

In any case, I'll suspend this PR until rust-lang/rust#140976 is merged and synced to Clippy (hopefully tomorrow).

@rustbot author

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[samueltardieu]

Rebased to take advantage of the new diagnostic items.

What about clippy::parsed_string_literals as a name?

[Jarcho]

That sounds like a good name.

[Jarcho]

FCP started here: https://rust-lang.zulipchat.com/#narrow/channel/257328-clippy/topic/FCP.3A.20parsed_string_literals/with/519705970

[samueltardieu]

Amended to avoid a useless string allocation during diagnostic production (and version number).

[samueltardieu]

Added numerical/boolean types as well.

[samueltardieu]

I've leveraged the existing clippy_utils::source::str_literal_to_char_literal to ensure that the original representation of a character literal is preserved when possible.

[samueltardieu]

Ping @Jarcho

[samueltardieu]

Ping @Jarcho

[samueltardieu]

Ping @Jarcho

[samueltardieu]

Rebased (first push), and updated the version to 1.92.0 (second push).

[Jarcho]

check_primitive and check_ipaddr are split in a weird spot. The check is basically a type dispatch, but the functions are split before checking the type's kind. e.g.

match ty.kind() {
  ty::UInt(_) => check_uint,
  ty::Int(_) => check_int,
  ty::Bool => check_bool,
  ty::Float(_) => check_float,
  ty::Adt(..) => check_ipaddr,
}

For integers and floats from_str accepts a leading '+' which would need to be removed from suggestions. For negative values you might need to add parenthesis as well (e.g. "-1".parse().unwrap().foo() would be (-1).foo()).

[samueltardieu]

I've reworked completely this lint, and reorganized it with submodules so that it is not only easier to read, but also it should be much easier to add new types beyond primitive types and ip addresses.

[Jarcho]

Why are you looking for the parent statement here? If the type isn't on the parse call itself then type inference will work just fine.

[samueltardieu]

Because I hadn't noticed that IpAddr could get direct conversion from [u8; 4] (IPv4) or [u16; 8] (IPv6), and thought I had to go through one intermediate value. I will fix this, this will be even better then.

[Jarcho]

A snippet isn't needed here and it's the slowest available option. The fastest thing to do is match the type down to a single segment path and compare the symbol.

[samueltardieu]

Done, and .with_source_text() is used whenever possible at lint emission type to limit the number of copies.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.