I’m playing with Konsist and trying to see if I ca...

# konsist

[Carter]

I’m playing with Konsist and trying to see if I can flag List constructor arguments and verify they are actually ImmutableList . One trouble I seem to have is that withRepresentedTypeOf doesn’t seem to match List, so I experimented matching the class name instead.

This kind of works but doesn’t seem quite like the right approach. Is there a better way to do this?

Konsist.scopeFromProject()
    .classes()
    // Skip serializable classes, because immutable collections aren't serializable by default
    .withoutSomeAnnotationsOf(kotlinx.serialization.Serializable::class)
    .flatMap { it.constructors }
    .flatMap { it.parameters }
    .filter {
        it.type.name.startsWith(List::class.java.simpleName)
    }
    .assert { it.representsTypeOf<ImmutableList<*>>() }

[igor.wojda]

1. I think your 1st filter should use endsWith / contains instead of startsWith

(see Debug Konsist Test to understand what going on)

2. Regarding the assertion My guess is that <*> inside does not behave as you expect - in kotlin * means any type, but Konsist is just comparing strings, so you will compare ImmutableList<SomeType> == ImmutableList<*> which will be false.

assert { it.type.name.startsWith("ImmutableList") }

Thats being said I am guessing here - please share your Kotlin code snippet you want to verify - best would be containing one ore more correct and incorrect types. This will help me to understand what exactly you want to test.

[Carter]

Basically, I want to consider this to be invalid

data class SomeClass(val: List<Elt>)

Preferring:

data class SomeClass(val: ImmutableList<Elt>)

[Carter]

This prevents a common bug I’ve seen in code where someone does:

val mutableList = mutableListOf(a, b, c)
val someClass = SomeClass(mutableList)
mutableList.clear()

Although Kotlin is better than Java with default immutability of the List interface, the fact the mutable implementations are subclasses means certain bugs can still persist

[Carter]

(This pattern also ends up being better for Compose UI performance as well).

[igor.wojda]

It is tricky to check generic types using representsType as they are erashed in runtime (type erasure), so type name will not match. In this case you have to relay on string comparision.

I think for you case all you need it to check is that the List type is not used for constructor parameters:

Konsist.scopeFromProject()
            .classes()
            // Skip serializable classes, because immutable collections aren't serializable by default
            .withoutSomeAnnotationsOf(kotlinx.serialization.Serializable::class)
            .flatMap { it.constructors }
            .flatMap { it.parameters }
            .assertNot {
                it.type.name.startsWith("List<") 
            }

[Carter]

Immutable list is a subtype of list, and I wasn’t exactly sure how it was doing the matching internally hence my original approach.

[Carter]

If someone does an explicit list implementation like ArrayList, LinkedList, or MutableList, then I think the string matching gets tricky.

It doesn’t have to be bulletproof though and I think the suggestion above is a good 80-20 solution that hits the main goal.

[igor.wojda]

Sure there is so many combinations. The more rules you write the more you realise various edge cases - my personal approach would be to start with this and improve these safe guards when you notice during PR review that devs are making certain mistakes

[igor.wojda]

e.g. for the above case you could consider this regex:

.assertNot {
    val regex = """.*List.*""".toRegex()
    it.type.hasNameMatching(regex)
}

In the future I am planning to introduce more advanced checks - these will allows to verify entire inheritance hierarchy (not only types from a single file). You would be able to express things like "is child of List class"

[Carter]

when you notice during PR review that devs are making certain mistakes

At the moment I’m protecting against myself 😂.