unnecessary call to min function

[FelixMaetzler]

What it does

A few days ago i opened #11901
You could do this for the min() method too.
So 0.min(<anything>) is always zero if <anything> is an unsigned integer.
Also this method is symmetric, that means same applies to <anything>.min(0).

You could go even further and generalize this to T::MIN.min(<anything>) (where T is any type of integer like i.e. i64) is always T::MIN.
And you could do this with the upper bound too: T::MAX.min(<anything>) is for any integer type T the same as <anything>.

Advantage

• Remove of unnecessary function call
• Reduction of Code Complexity

Drawbacks

No response

Example

// example with zero and unsigned Type
let a = 0.min(42_usize);

// example with MIN and signed Type
let a = 42.min(i64::MIN);

// example with MAX and signed Type
let a = i32::MAX.min(9);

Could be written as:

// example with zero and unsigned Type
let a = 0;

// example with MIN and signed Type
let a = i64::MIN;

// example with MAX and signed Type
let a = 9;

[FelixMaetzler]

I have never done a contribution to such a project, so i think this is a perfect opportunity for that.
I would like to implement this lint (or at least try it).

[FelixMaetzler]

@rustbot claim

[Centri3]

This is a good idea. You'll want to match on an Expr method call, the receiver is the 0 and the argument is the 42_usize. It'd be easiest to just match on the function name itself rather than using diagnostic items or similar since we'd need one for every type.

Getting the value of the constant is tricky, versus just matching on the name. I'd go with the former for the same reason as matching on the name. See the manual_is_finite and manual_is_infinite lint combo for an example on how to do this: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/manual_float_methods.rs#L55, most notably, use the Constant struct in clippy_utils.

Not sure how you'd easily check if it's gt/lt if it's a float.

I think we could extend this to min on two constants, not just 0, correct? (Note that the logic for 0, T::MIN, etc. would ideally be different - anything is unnecessary there, so we don't need to parse the argument which can't be done in some cases and thus wouldn't lint normally)

The same applies to the max lint as well, they should share the same code ideally

[FelixMaetzler]

This is a good idea. You'll want to match on an Expr method call, the receiver is the 0 and the argument is the 42_usize. It'd be easiest to just match on the function name itself rather than using diagnostic items or similar since we'd need one for every type.

Yes i thought the same.
I would add my 'min' method in this match: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/methods/mod.rs#L4098 and just calling my own check function.

and here is a bit of pseudo code of what i would do in my check function:

// i have something like this:
let _ = a.min(b);

// Check function:
let ty = cx.typeck_results().expr_ty(expr);
match ty {
    types_i_want_to_support => {},
    _ => return,
}
if a == 0 | T::MIN | T::MAX {
    lint()
}
if b == 0 | T::MIN | T::MAX {
    lint()
}

Technically i don't want to check if it is a 0, because this is just a "special case" of T::MIN. In other words: If 0 is not T::MIN i don't want to lint.
But i don't know right now if and how this is possible. I have to look into that.

I think we could extend this to min on two constants, not just 0, correct? (Note that the logic for 0, T::MIN, etc. would ideally be different - anything is unnecessary there, so we don't need to parse the argument which can't be done in some cases and thus wouldn't lint normally)

I think i don't quite understand what you're saying.
The two constants are T::MIN and T::MAX (with 0 as "special case" for T::MIN)?
Or do you mean constant expressions (things with the const keyword) that evaluate to that?

Getting the value of the constant is tricky, versus just matching on the name. I'd go with the former for the same reason as matching on the name. See the manual_is_finite and manual_is_infinite lint combo for an example on how to do this: https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/manual_float_methods.rs#L55, most notably, use the Constant struct in clippy_utils.

I will look into that. Thank you!

The same applies to the max lint as well, they should share the same code ideally

That's a good point. The logic for checking on these special values could been shared.

[Centri3]

I think i don't quite understand what you're saying.

It's 4am. On the first section I mean that, rather than only linting when the receiver is T::MIN, we always lint it if they're both constants and recv < arg and they're both constants (or literals).

The second section is how constant turns MIR (external)/HIR (same crate) into, well, a Constant. It doesn't look into any (many?) function calls, so for example, 0u32.min(a()) wouldn't lint since it (a()) wouldn't evaluate to a constant. This isn't good, so if it's T::MIN (and T::MAX for max), the lint shouldn't check the argument at all. If it's not T::MIN it falls back to checking the argument.

[FelixMaetzler]

It's 4am.

My bad. I'm from europe and i didn't think about other timezones.

On the first section I mean that, rather than only linting when the receiver is T::MIN, we always lint it if they're both constants and recv < arg and they're both constants (or literals).

Of course. Thats a good point. I didnt think of that at all. That happens if you are so focused on a detail, that you don't see the bigger picture.

The second section is how constant turns MIR (external)/HIR (same crate) into, well, a Constant.

I definitely have to look into what different Types there are and what i can do with it.

It doesn't look into any (many?) function calls, so for example, 0u32.min(a()) wouldn't lint since it (a()) wouldn't evaluate to a constant. This isn't good, so if it's T::MIN (and T::MAX for max), the lint shouldn't check the argument at all. If it's not T::MIN it falls back to checking the argument.

I think it does work. Because i see the 0u32 and then i can lint without the need of checking a().
This should be the first if in my pseudo code from above.
I can't simplify it with a constant (because then i need to know what a() evaluates to like you said) but i can simplify 0u32.min(a()) to just a().

[Centri3]

My bad.

Was just explaining why it was worded improperly 😅 You're fine :)

[FelixMaetzler]

So i have implemented the first draft. Because i'm new to colloboration on github, what is the next step?
How can i show my code so that other people can have a look at it and give feedback and check it for bugs etc?

[y21]

Closing as this was implemented in #12368