Create Rule: MapGetWithNotNullAssertionOperator

detekt-cli/src/main/resources/default-detekt-config.yml

@@ -413,6 +413,8 @@ potential-bugs:
     excludes: "**/test/**,**/androidTest/**,**/*.Test.kt,**/*.Spec.kt,**/*.Spek.kt"
     excludeAnnotatedProperties: ""
     ignoreOnClassesPattern: ""
+  MapGetWithNotNullAssert:
+    active: false
   MissingWhenCase:
     active: true
   RedundantElseInWhen:

detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MapGetWithNotNullAssert.kt

@@ -0,0 +1,70 @@
+package io.gitlab.arturbosch.detekt.rules.bugs
+
+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.lexer.KtTokens
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.psi.KtPostfixExpression
+import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
+import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe
+
+/**
+ * Reports calls of the map access methods `map[]` or `map.get()` with a not-null assertion operator `!!`.
+ * This may result in a NullPointerException.
+ * Preferred access methods are `map[]` without `!!`, `map.getValue()`, `map.getOrDefault()` or `map.getOrElse()`.
+ *
+ * Based on an IntelliJ IDEA inspection MapGetWithNotNullAssertionOperatorInspection.
+ *
+ * <noncompliant>
+ *  val map = emptyMap<String, String>()
+ *  map["key"]!!
+ *
+ *  val map = emptyMap<String, String>()
+ *  map.get("key")!!
+ * </noncompliant>
+ *
+ * <compliant>
+ * val map = emptyMap<String, String>()
+ * map["key"]
+ *
+ * val map = emptyMap<String, String>()
+ * map.getValue("key")
+ *
+ * val map = emptyMap<String, String>()
+ * map.getOrDefault("key", "")
+ *
+ * val map = emptyMap<String, String>()
+ * map.getOrElse("key", { "" })
+ * </compliant>
+ */
+class MapGetWithNotNullAssert(config: Config) : Rule(config) {
+
+    override val issue: Issue =
+        Issue(
+            "MapGetWithNotNullAssert",
+            Severity.CodeSmell,
+            "map.get() with not-null assertion operator (!!) can result in a NullPointerException. " +
+                    "Consider usage of map.getValue(), map.getOrDefault() or map.getOrElse() instead",
+            Debt.FIVE_MINS
+        )
+
+    override fun visitPostfixExpression(expression: KtPostfixExpression) {
+        if (expression.isMapGet() && expression.operationToken == KtTokens.EXCLEXCL) {
+            report(CodeSmell(issue, Entity.from(expression), "map.get() with not-null assertion operator (!!)"))
+        }
+        super.visitPostfixExpression(expression)
+    }
+
+    private fun KtPostfixExpression.isMapGet(): Boolean {
+        return this
+            .baseExpression
+            .getResolvedCall(bindingContext)
+            ?.resultingDescriptor
+            ?.fqNameSafe == FqName("kotlin.collections.Map.get")
+    }
+}

detekt-rules/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/providers/PotentialBugProvider.kt

@@ -13,6 +13,7 @@ import io.gitlab.arturbosch.detekt.rules.bugs.InvalidRange
 import io.gitlab.arturbosch.detekt.rules.bugs.IteratorHasNextCallsNextMethod
 import io.gitlab.arturbosch.detekt.rules.bugs.IteratorNotThrowingNoSuchElementException
 import io.gitlab.arturbosch.detekt.rules.bugs.LateinitUsage
+import io.gitlab.arturbosch.detekt.rules.bugs.MapGetWithNotNullAssert
 import io.gitlab.arturbosch.detekt.rules.bugs.MissingWhenCase
 import io.gitlab.arturbosch.detekt.rules.bugs.RedundantElseInWhen
 import io.gitlab.arturbosch.detekt.rules.bugs.UnconditionalJumpStatementInLoop
@@ -46,6 +47,7 @@ class PotentialBugProvider : RuleSetProvider {
                 WrongEqualsTypeParameter(config),
                 ExplicitGarbageCollectionCall(config),
                 LateinitUsage(config),
+                MapGetWithNotNullAssert(config),
                 MissingWhenCase(config),
                 RedundantElseInWhen(config),
                 UnconditionalJumpStatementInLoop(config),

detekt-rules/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/bugs/MapGetWithNotNullAssertSpec.kt

@@ -0,0 +1,76 @@
+package io.gitlab.arturbosch.detekt.rules.bugs
+
+import io.gitlab.arturbosch.detekt.api.Config
+import io.gitlab.arturbosch.detekt.test.KtTestCompiler
+import io.gitlab.arturbosch.detekt.test.compileAndLintWithContext
+import org.assertj.core.api.Assertions.assertThat
+import org.spekframework.spek2.Spek
+import org.spekframework.spek2.style.specification.describe
+
+class MapGetWithNotNullAssertSpec : Spek({
+    val subject by memoized { MapGetWithNotNullAssert(Config.empty) }
+
+    val wrapper by memoized(
+        factory = { KtTestCompiler.createEnvironment() },
+        destructor = { it.dispose() }
+    )
+
+    describe("check for MapGetWithNotNullAssert") {
+
+        it("reports map[] with not null assertion") {
+            val code = """
+                fun f() {
+                    val map = emptyMap<Any, Any>()
+                    val value = map["key"]!!
+                }"""
+            assertThat(subject.compileAndLintWithContext(wrapper.env, code)).hasSize(1)
+        }
+
+        it("reports map.get() with not null assertion") {
+            val code = """
+                fun f() {
+                    val map = emptyMap<Any, Any>()
+                    val value = map.get("key")!!
+                }"""
+            assertThat(subject.compileAndLintWithContext(wrapper.env, code)).hasSize(1)
+        }
+
+        it("does not report map[] call without not-null assert") {
+            val code = """
+                fun f() {
+                    val map = emptyMap<String, String>()
+                    map["key"]
+                }"""
+            assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
+        }
+
+        it("does not report map.getValue() call") {
+            val code = """
+                fun f() {
+                    val map = emptyMap<String, String>()
+                    map.getValue("key")
+                }"""
+            assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
+        }
+
+        it("does not report map.getOrDefault() call") {
+            val code = """
+                fun f() {
+                    val map = emptyMap<String, String>()
+                    map.getOrDefault("key", "")
+                }"""
+            assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
+        }
+
+        it("does not report map.getOrElse() call") {
+            val code = """
+                fun f() {
+                    val map = emptyMap<String, String>()
+                    map.getOrElse("key", { "" })
+                }"""
+            assertThat(subject.compileAndLintWithContext(wrapper.env, code)).isEmpty()
+        }
+
+    }
+
+})

docs/pages/documentation/potential-bugs.md

@@ -303,6 +303,44 @@ class Foo {
 }
 ```
 
+### MapGetWithNotNullAssert
+
+Reports calls of the map access methods `map[]` or `map.get()` with a not-null assertion operator `!!`.
+This may result in a NullPointerException.
+Preferred access methods are `map[]` without `!!`, `map.getValue()`, `map.getOrDefault()` or `map.getOrElse()`.
+
+Based on an IntelliJ IDEA inspection MapGetWithNotNullAssertionOperatorInspection.
+
+**Severity**: CodeSmell
+
+**Debt**: 5min
+
+#### Noncompliant Code:
+
+```kotlin
+val map = emptyMap<String, String>()
+map["key"]!!
+
+val map = emptyMap<String, String>()
+map.get("key")!!
+```
+
+#### Compliant Code:
+
+```kotlin
+val map = emptyMap<String, String>()
+map["key"]
+
+val map = emptyMap<String, String>()
+map.getValue("key")
+
+val map = emptyMap<String, String>()
+map.getOrDefault("key", "")
+
+val map = emptyMap<String, String>()
+map.getOrElse("key", { "" })
+```
+
 ### MissingWhenCase
 
 Turn on this rule to flag `when` expressions that do not check that all cases are covered when the subject is an enum