Skip to content

Commit

Permalink
Add rule to ban extension functions on nullable receivers (#26)
Browse files Browse the repository at this point in the history
  • Loading branch information
mgulbronson authored Mar 6, 2024
1 parent c860491 commit 8ec1632
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 0 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ FaireRuleSet:
active: true
GetOrDefaultShouldBeReplacedWithGetOrElse:
active: true
NoExtensionFunctionOnNullableReceiver:
active: true
NoNonPrivateGlobalVariables:
active: true
NoNullableLambdaWithDefaultNull:
Expand Down
2 changes: 2 additions & 0 deletions detekt.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ FaireRuleSet:
active: true
GetOrDefaultShouldBeReplacedWithGetOrElse:
active: true
NoExtensionFunctionOnNullableReceiver:
active: true
NoNonPrivateGlobalVariables:
active: true
NoNullableLambdaWithDefaultNull:
Expand Down
2 changes: 2 additions & 0 deletions src/main/kotlin/com/faire/detekt/FaireRulesProvider.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.faire.detekt.rules.DoNotUsePropertyAccessInAssert
import com.faire.detekt.rules.DoNotUseSingleOnFilter
import com.faire.detekt.rules.DoNotUseSizePropertyInAssert
import com.faire.detekt.rules.GetOrDefaultShouldBeReplacedWithGetOrElse
import com.faire.detekt.rules.NoExtensionFunctionOnNullableReceiver
import com.faire.detekt.rules.NoNonPrivateGlobalVariables
import com.faire.detekt.rules.NoNullableLambdaWithDefaultNull
import com.faire.detekt.rules.NoPairWithAmbiguousTypes
Expand Down Expand Up @@ -43,6 +44,7 @@ internal class FaireRulesProvider : RuleSetProvider {
DoNotUseSingleOnFilter(config),
DoNotUseSizePropertyInAssert(config),
GetOrDefaultShouldBeReplacedWithGetOrElse(config),
NoExtensionFunctionOnNullableReceiver(config),
NoNonPrivateGlobalVariables(config),
NoNullableLambdaWithDefaultNull(config),
NoPairWithAmbiguousTypes(config),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package com.faire.detekt.rules

import io.gitlab.arturbosch.detekt.api.CodeSmell
import io.gitlab.arturbosch.detekt.api.Config
import io.gitlab.arturbosch.detekt.api.Debt
import io.gitlab.arturbosch.detekt.api.Entity
import io.gitlab.arturbosch.detekt.api.Issue
import io.gitlab.arturbosch.detekt.api.Rule
import io.gitlab.arturbosch.detekt.api.Severity
import org.jetbrains.kotlin.psi.KtNamedFunction
import org.jetbrains.kotlin.psi.psiUtil.isExtensionDeclaration

/**
* This rule reports extension functions on nullable types.
* Exceptions are made for functions that return non-nullable types since they do not introduce nullability.
*
* The reason for this rule is that extension functions on nullable types can obscure the nullability of the receiver.
*
* ```
* // Bad
* fun String?.foo(): String? = this
*
* nonNullString.foo() // Return value is nullable which needs to be handled downstream
*
* // Good
* fun String.foo(): String = this
*
* nonNullString.foo() // Return value stays non-null
* nullableString?.foo() // Use null-safe operator to handle nullable receiver
*
* // Okay — function returns non-nullable type, essentially converting a nullable into a non-nullable type
* fun String?.emptyIfNull(): String = this ?: ""
* ```
*/
internal class NoExtensionFunctionOnNullableReceiver(config: Config = Config.empty) : Rule(config) {
override val issue: Issue = Issue(
id = javaClass.simpleName,
severity = Severity.Warning,
description = "This rule reports extension functions on nullable types.",
debt = Debt.FIVE_MINS,
)

override fun visitNamedFunction(function: KtNamedFunction) {
super.visitNamedFunction(function)

if (!function.isExtensionDeclaration()) return
if (function.receiverTypeReference?.text?.endsWith("?") != true) return
if (function.typeReference?.text?.endsWith("?") != true) return

report(
CodeSmell(
issue = issue,
entity = Entity.from(function),
message = "No extension functions on nullable types",
),
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package com.faire.detekt.rules

import io.gitlab.arturbosch.detekt.test.assertThat
import io.gitlab.arturbosch.detekt.test.compileAndLint
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test

internal class NoExtensionFunctionOnNullableReceiverTest {
private lateinit var rule: NoExtensionFunctionOnNullableReceiver

@BeforeEach
fun setup() {
rule = NoExtensionFunctionOnNullableReceiver()
}

@Test
fun `flag non-private extension function on nullable type`() {
val findings = rule.compileAndLint(
"""
fun String?.foo(): String? = this
""".trimIndent(),
)
assertThat(findings).hasSize(1)
}

@Test
fun `do not flag extension function on non-nullable type`() {
val findings = rule.compileAndLint(
"""
fun String.foo(): String? = this
""".trimIndent(),
)
assertThat(findings).isEmpty()
}

@Test
fun `do not flag extension function that returns non-null type`() {
val findings = rule.compileAndLint(
"""
fun String?.foo(): String = this ?: ""
""".trimIndent(),
)
assertThat(findings).isEmpty()
}

@Test
fun `do not flag if manually suppressed`() {
val findings = rule.compileAndLint(
"""
@Suppress("NoExtensionFunctionOnNullableReceiver")
fun String?.foo(): String? = this
""".trimIndent(),
)
assertThat(findings).isEmpty()
}
}

0 comments on commit 8ec1632

Please sign in to comment.