repeat_once triggers when the parameter is a constant

[frantisekhanzlikbl]

Lint name: repeat_once

I tried this code:

fn main() {
    const REPEAT_COUNT: usize = 1;
    
    "x".repeat(REPEAT_COUNT);
}

I expected to see this happen: the lint is not triggered because if I used a constant as a parameter I probably intend to allow changing the constant value to something else in the future.

Instead, this happened: the lint triggers

Meta

• cargo clippy -V: clippy 0.1.54 (1c6868a 2021-05-27
• rustc -Vv:

  rustc 1.54.0-nightly (1c6868aa2 2021-05-27)
  binary: rustc
  commit-hash: 1c6868aa21981b37cbd3fc95828ee3b0ac22d494
  commit-date: 2021-05-27
  host: x86_64-unknown-linux-gnu
  release: 1.54.0-nightly
  LLVM version: 12.0.1
  

[xFrednet]

@rustbot claim

[xFrednet]

Hey @giraffate and @flip1995 the lint implementation contains a deliberate test that resolves the value of constants, like in this case. You two discussed the check here. Would you also consider this to be a false positive, or working as intended? 🙃

[giraffate]

Yes, that was the intention. I thought that warning is valid in many cases at that time.

because if I used a constant as a parameter I probably intend to allow changing the constant value to something else in the future.

However, if this is a common case, it's a good idea to change that behavior.

[frantisekhanzlikbl]

I'd like to note that I don't feel too strongly about this - I can always just disable the lint. I just wanted to report it since it felt a little surprising.

[flip1995]

because if I used a constant as a parameter I probably intend to allow changing the constant value to something else in the future.

I mean, if you change this constant the lint will no longer trigger. But as long as the constant is 1 the lint will tell you that it is 1. So I think the lint is right here. Even if in this case it will rather remind you to change the constant than that the repeat(CONST) call is wrong. I see value in that too.

So I would keep the current behavior. Especially because code like

"x".repeat(NUMBER + const_fn() * ANOTHER_CONST);

may result in an unintentional "x".repeat(1). In this case, the lint is really valuable.

[xFrednet]

Thank you for the feedback. I'm still undecided on this one, constants are usually used for readability and to make parts kind of configurable. This ticket basically addresses the second reason of making things configurable.

Due to the previous comments, I would suggest keeping the current behavior and adding a note about this to the lint documentation. Would that work for everyone?

[flip1995]

usually used for readability

Yes, and on the other hand they may hide the real value of the constant. Linting also on constants is therefore generally a good idea IMO.

would suggest keeping the current behavior and adding a note about this to the lint documentation

SGTM

[frantisekhanzlikbl]

I like the point raised by @flip1995 in #7306 (comment), so I agree that this behaviour is correct.