False positive on kotlin:S6514 (Delegator pattern should use "by" clause)

Products: SonarQube 10.2 and SonarLint (IntelliJ) 9.0.0.75308
Language: Kotlin

Hi everyone,

I believe I’ve come across a false positive related to the rule kotlin:S6514 (Delegator pattern should use “by” clause). Here is a code example to demonstrate the issue:

fun interface Policy { fun doSomething() }

class FirstPolicy : Policy {
    override fun doSomething() { println("first") }
}

class SecondPolicy : Policy {
    override fun doSomething() { println("second") }
}

class DynamicPolicy: Policy {
    private var policy: Policy = FirstPolicy()

    fun useSecondPolicy() {
        policy = SecondPolicy()
    }

    override fun doSomething() {
        policy.doSomething()
    }
}

fun main() {
    val dynamicPolicy = DynamicPolicy()
    dynamicPolicy.doSomething()
    dynamicPolicy.useSecondPolicy()
    dynamicPolicy.doSomething()
}

This code triggers the issue “Replace with interface delegation using ‘by’ in the class header.” at line 20.

The “fix” for this issue, as suggested in the rule description, is to use interface delegation:

class DynamicPolicy(private var policy: Policy = FirstPolicy()): Policy by policy {
    fun useSecondPolicy() {
        policy = SecondPolicy()
    }
}

However, despite the code compiling without errors, it doesn’t function correctly due to the way the Kotlin compiler handles interface delegation. This can be verified by running the main method with both the old and new classes.

IntelliJ IDEA’s Kotlin analyzer issues a warning for the new class: “Delegating to ‘var’ property does not take its changes into account”.

It appears that SonarKotlin shouldn’t raise this issue for “var” properties.

1 Like

Hello @felipebz, thank you for reporting such an interesting use case! It was really interesting for me to discover the details behind Kotlin Delegation Pattern :slight_smile:

You can track the progress of the resolution on SONARKT-359.

A small note from my side: I would avoid having a delegate that changes at runtime if possible. An alternative could be having a second abstraction that supports two delegates. Anyway, the most correct implementation depends on the use case.

Cheers,

Angelo

1 Like

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.