Why does the ThrowsCount rule ignore override fun?

[stellingsimon]

Thanks for this invaluable piece of software.

ThrowsCount reports this as a violation:

fun f1(x: Int) {
    when (x) {
        1 -> throw IOException()
        2 -> throw IOException()
        3 -> throw IOException()
    }
}

but not this:

override fun f3(x: Int) { // does not report overridden function
    when (x) {
        1 -> throw IOException()
        2 -> throw IOException()
        3 -> throw IOException()
    }
}

Looking at the source code of the ThrowsCount rule, it seems this behavior is intentional.
At least there's a test case asserting this exact behavior.
This behavior and the associated test have been there since the rule was added in 2017.

Still, I wonder: why? A function's body doesn't become more readable just because it's overriding some interface method.
Also, the test's name clearly does not express what it's body implements:

@Test
fun `reports violation by default`() {
    assertThat(subject.lint(code)).hasSize(1)
}

So my expectations are met by the test's name, but not the test's body and the current implementation. Which one needs to change? I'm willing to provide a PR once I know what is to be expected here.

[BraisGabin]

That's odd. Could you review the git history to see if that was added from the beginning of the rule or if it was added based on an issue request? To me this seems like an issue. I can't imagine any reason to treat the override functions in a different way on this rule.

If there is not issue we could removed. If there is, please link it and we can decide what to do. (probably add a configuration but let's first check it before take any decision)

[stellingsimon]

Thank's for the reply.

When ThrowsCount.kt was added in 648e5dc it already included the

if (!function.hasModifier(KtTokens.OVERRIDE_KEYWORD)) {

Its successor commit 259155b then "Added test case for overridden fun in ThrowsCount" with an explicit

override fun f3(x: Int) { // does not report overridden function

so it looks like an intentional design, but I can't find any hints as to what motivates this.

[BraisGabin]

@schalkms do you remember if there was a reason for this? Could we remove it or should we add it behind a configuration?

[schalkms]

No, I don’t remember exactly, unfortunately. We had an initiative to exclude overridden functions back then. However, I don’t see a reason here to keep this behavior. Thus, feel free to change this behavior.

[BraisGabin]

@stellingsimon then, if you want to go for it just remove the if and fix the tests to reflect this. And thanks for the issue and the research :)

[stellingsimon]

Thanks. I reported the issue in #6752 and a proposed fix in #6753.