Add informative message to UselessCallOnNotNull report (#2920)

* Add informative message to UselessCallOnNotNull report * Add test cases for UselessCallOnNotNull error messages
detekt · Aug 4, 2020 · 4d0d8c9 · 4d0d8c9
1 parent 8e6679e
commit 4d0d8c9
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 14 deletions.
diff --git a/...les-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNull.kt b/...les-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNull.kt
@@ -56,17 +56,25 @@ class UselessCallOnNotNull(config: Config = Config.empty) : Rule(config) {
         FqName("kotlin.text.isNullOrBlank") to Conversion("isBlank")
     )
 
+    @Suppress("ReturnCount")
     override fun visitQualifiedExpression(expression: KtQualifiedExpression) {
         super.visitQualifiedExpression(expression)
         if (bindingContext == BindingContext.EMPTY) return
 
         val resolvedCall = expression.getResolvedCall(bindingContext) ?: return
-        if (uselessFqNames.contains(resolvedCall.resultingDescriptor.fqNameOrNull())) {
-            val safeExpression = expression as? KtSafeQualifiedExpression
-            val notNullType = expression.receiverExpression.getType(bindingContext)?.isNullable() == false
-            if (notNullType || safeExpression != null) {
-                report(CodeSmell(issue, Entity.from(expression), ""))
+        val fqName = resolvedCall.resultingDescriptor.fqNameOrNull() ?: return
+        val conversion = uselessFqNames[fqName] ?: return
+
+        val safeExpression = expression as? KtSafeQualifiedExpression
+        val notNullType = expression.receiverExpression.getType(bindingContext)?.isNullable() == false
+        if (notNullType || safeExpression != null) {
+            val shortName = fqName.shortName().asString()
+            val message = if (conversion.replacementName == null) {
+                "Remove redundant call to $shortName"
+            } else {
+                "Replace $shortName with ${conversion.replacementName}"
             }
+            report(CodeSmell(issue, Entity.from(expression), message))
         }
     }
 

diff --git a/...style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNullSpec.kt b/...style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UselessCallOnNotNullSpec.kt
@@ -16,23 +16,31 @@ object UselessCallOnNotNullSpec : Spek({
     describe("UselessCallOnNotNull rule") {
         it("reports when calling orEmpty on a list") {
             val code = """val testList = listOf("string").orEmpty()"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Remove redundant call to orEmpty")
         }
 
         it("reports when calling orEmpty on a list with a safe call") {
             val code = """val testList = listOf("string")?.orEmpty()"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Remove redundant call to orEmpty")
         }
 
         it("reports when calling orEmpty on a list in a chain") {
             val code = """val testList = listOf("string").orEmpty().map { }"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Remove redundant call to orEmpty")
         }
 
         it("reports when calling orEmpty on a list with a platform type") {
             // System.getenv().keys.toList() will be of type List<String!>.
             val code = """val testSequence = System.getenv().keys.toList().orEmpty()"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Remove redundant call to orEmpty")
         }
 
         it("does not report when calling orEmpty on a nullable list") {
@@ -45,7 +53,9 @@ object UselessCallOnNotNullSpec : Spek({
 
         it("reports when calling isNullOrBlank on a string with a safe call") {
             val code = """val testString = ""?.isNullOrBlank()"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Replace isNullOrBlank with isBlank")
         }
 
         it("does not report when calling isNullOrBlank on a nullable string") {
@@ -58,7 +68,9 @@ object UselessCallOnNotNullSpec : Spek({
 
         it("reports when calling isNullOrEmpty on a string") {
             val code = """val testString = "".isNullOrEmpty()"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Replace isNullOrEmpty with isEmpty")
         }
 
         it("reports when calling isNullOrEmpty on a string with a safe call") {
@@ -76,7 +88,9 @@ object UselessCallOnNotNullSpec : Spek({
 
         it("reports when calling orEmpty on a string") {
             val code = """val testString = "".orEmpty()"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Remove redundant call to orEmpty")
         }
 
         it("does not report when calling orEmpty on a nullable string") {
@@ -89,7 +103,9 @@ object UselessCallOnNotNullSpec : Spek({
 
         it("reports when calling orEmpty on a sequence") {
             val code = """val testSequence = listOf(1).asSequence().orEmpty()"""
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Remove redundant call to orEmpty")
         }
 
         it("does not report when calling orEmpty on a nullable sequence") {
@@ -107,7 +123,9 @@ object UselessCallOnNotNullSpec : Spek({
                 val noList = "str".orEmpty()
                 val list = listOf(1, 2, 3).orEmpty()
             """
-            assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)
+            val findings = subject.compileAndLintWithContext(env, code)
+            assertThat(findings).hasSize(1)
+            assertThat(findings[0].message).isEqualTo("Remove redundant call to orEmpty")
         }
     }
 })