Rule: Boolean expression using in / !in for progression

[Marcono1234]

It appears boolean expressions such as the condition of an if statement can use the in / !in operator for progressions:

fun main(args: Array<String>) {
    val x = args[0].toInt()
    val start = args[1].toInt()
    val end = args[2].toInt()
    
    if (x in start..end step 2) {
        println("in")
    }
}

The byte code this generates (Kotlin 1.5.0, JVM target) is pretty inefficient:

43: new           #23                 // class kotlin/ranges/IntRange
46: dup
47: iload_2
48: iload_3
49: invokespecial #27                 // Method kotlin/ranges/IntRange."<init>":(II)V
52: checkcast     #29                 // class kotlin/ranges/IntProgression
55: iconst_2
56: invokestatic  #35                 // Method kotlin/ranges/RangesKt.step:(Lkotlin/ranges/IntProgression;I)Lkotlin/ranges/IntProgression;
59: checkcast     #37                 // class java/lang/Iterable
62: iload_1
63: invokestatic  #41                 // Method java/lang/Integer.valueOf:(I)Ljava/lang/Integer;
66: invokestatic  #47                 // Method kotlin/collections/CollectionsKt.contains:(Ljava/lang/Iterable;Ljava/lang/Object;)Z

So it apparently:

1. Creates an IntRange and then an IntProgression
2. Calls Collections.contains(Iterable, Object)

Therefore (if I understand it correctly), until the first matching element is found for every element in the progression it would wrap the primitive in an object and then compare it.

(Note that this is only for boolean expressions; for loops using progressions seem to generate efficient byte code.)

However, I am not sure how relevant a rule for this is because I don't think such code would be pretty common. That is why I have created a discussion for this for now; feedback is appreciated.

Maybe the bug here is also that the Kotlin compiler is not emitting optimized byte code, e.g. it could probably write:

if (x >= start && x <= end && (x - start % step == 0)) {
    // ...
}

(Though maybe since this usecase is so uncommon such feature was not added to the compiler.)

[cortinico]

Thanks for the proposal. I'm unsure if we want to start going in this direction and adding rules for missed compiler optimizations.

I would try to keep Detekt a higher level and inspect readability/code-quality issues or obvious performance problems.

IMHO for this specific rule using an IntProgression is actually way more readable than having to manually write the equivalent boolean condition.

[BraisGabin]

I think the same. But this could be an useful custom ruleset. One for people that needs really performant code. The problem of that code is that usually it goes agains readability, and that's the main focus in the core rules of detekt.

Anyway, did you open (or upvote) an issue in the kotlin compiler so they can fix this issue?

[Marcono1234]

I did not find an existing issue so I created KT-47025

[Marcono1234]

Feel free to consider this suggestion resolved. My bug report was a duplicate of KT-34617, and it appears their intention is to solve this issue (though it seems to be slightly tricky compatibility-wise).